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

[Allan McRae]

On 10/06/13 16:33, Patrick Steinhardt wrote:
> 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.
> 

I'll pull into master sometime after the 4.1.2 release.  It will not be
part of a 4.1.x release.

Allan