On Wed, Jun 05, 2013 at 02:52:53PM +1000, Allan McRae wrote:
On 04/06/13 21:10, Ross Lagerwall wrote:
On Tue, Jun 04, 2013 at 11:20:14AM +0200, Patrick Steinhardt wrote:
if(hash_local && hash_pkg && strcmp(hash_local, hash_pkg) == 0) { - /* local and new files are the same, no sense in installing the file - * over itself, regardless of what the original file was */ - _alpm_log(handle, ALPM_LOG_DEBUG, - "action: leaving existing file in place\n"); - unlink(checkfile); + /* local and new files are the same, updating anyway to get + * correct timestamps */ + _alpm_log(handle, ALPM_LOG_DEBUG, "action: installing new file: %s\n",
Perhaps this should be changed to something like: "action: updating existing file's timestamps\n" to differentiate it from when the file is actually new.
I like "installing new file" because it is exactly what we say we will do in the pacman(8) man page. Also, the situations are easily distinguished by the previous debug output.
+ entryname_orig); + if(try_rename(handle, checkfile, filename)) { + errors++; + }
Can we not just ignore it if it fails since it is non-fatal?
No. If something fails here, we are in a very bad situation for the rest of the transaction. I want that error to propagate so that we abort the transaction after this package is finished.
(Aside) What I have just noticed, is we do not print any actual errors during this whole section. So the transaction will stop with errors occurred during extraction but give no indication what actually happened. Another patch....
} else if(hash_orig && hash_pkg && strcmp(hash_orig, hash_pkg) == 0) { /* original and new files are the same, leave the local version alone, * including any user changes */
So... with my two comments above, I think the original patch is fine. I will pull that in the next few days unless there is objections to my above reasoning.
Are you going to pull this patch for pacman 4.1.2? Just curious as it is not included in the master branch yet. Patrick