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
pgpl6BH3dJU16.pgp
Description: PGP signature
