[pacman-dev] [PATCH] Install unchanged backup files to get correct timestamps.

[Patrick Steinhardt]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://mailman.archlinux.org/pipermail/pacman-dev/attachments/20130610/646960a1/attachment.asc>