[pacman-dev] backup handling ugly mess

[Nagy Gabor]

Hi!

First of all, as I see, you simply forgot about the fact that we have clear
definition about modified ["important"] and not modified ["can be reinstalled
easily, user did not work on this file at all, so there is nothing to lose"]
backup files in our manual (using md5sum). This is the reason for most of your
"grey" areas. [EDIT: I noticed Xavier's 5th point now.]
I also prefer the configfile terminology here, because that is more suggestive. 

> On Dec 6, 2007 1:56 AM, Xavier <shiningxc at gmail.com> wrote:
> > On Wed, Dec 05, 2007 at 08:25:54PM -0600, Dan McGee wrote:
> > > OK, so I think I understand the problem here, and it makes sense. It
> > > seems like manipulating backup arrays and merging them is probably a
> > > bad idea.
> > >
> > > Let me get this straight- your patch doesn't actually solve these
> > > problems, just makes the existing code work as we thought it would?
> > >
> >
> > Exactly.
> >
> > > Let's start by laying out a set of rules that pacman should follow,
> > > then seeing if our pactests represent these cases. I'm only going to
> > > cover upgrades, because removals are straightforward (always create
> > > pacsave files of those in the backup array).

I'm not sure. IMHO unmodified files can be removed.

> > > 1. If the file is in the old backup array, and is then removed from
> > > the backup array AND package, we should move it to pacsave.
> > > (upgrade024?)

This means that the new package renamed its config file, the solution also
depends on whether the old file was modified (.pacsave) or not ("overwrite" is
remove here).

> > ok.
> >
> > > 2. If the file is in the old backup array, and is then removed from
> > > the backup array BUT stays in the package, we have a gray area. We
> > > need to choose- either install the new file as pacnew and leave the
> > > old file (which would then be overwritten on the next upgrade because
> > > it is no longer in the backup array, so probably not the best
> > > solution), or do as upgrade025 does- move the existing file to pacsave
> > > and install the new file.
> >

This is probably a packaging error: the old package said that this is a config
file, the new package says that this is not a config file... strange. Since even
the simple removal of the file would induce a .pacsave, I vote a .pacsave here
too, if the file was modified.

> > yes, updrade025 looks alright.
> >
> > > 3. If the file is in the old backup array, and stays in the backup
> > > array BUT is removed from the package (bash), we should probably move
> > > (or should we copy and leave the original?) the file to pacsave. This
> > > is a gray area.
> >
> > This is a really odd case, but well, I would say just move it to pacsave,
> > because otherwise, it just let a normal file on the filesystem, without a
> > .pac* suffix, not owned by any packages.
> > So when you install a package that adds this file again, it'll conflict.

This is not necessarily an odd case.
Imagine a package, where the config file is born after package install (by a
graphical utility, or by post_update script...) <- we can confuse pacman easily
here [we _must_ assume (and DEFINE) that the post_install script doesn't modify
backup files, otherwise .pacnew would be useless]
If the old configfile was modified (we can determine this from the old package),
we can also keep that file or we can remove.

> >
> > That is upgrade026.
> >
> > >
> > > Note that we haven't even looked at the new backup array yet. I think
> > > it may have been a bit naive to introduce that without much testing...
> > > 4. If the file exists in the old package BUT is not in the backup
> > > array, but is in the backup array of the new package, we should either
> > > install the new file as pacnew OR move existing file to pacsave and
> > > install new file in original location. Not sure here.
> > >
> >

Imho we can simply remove the old file here.

> > (upgrade023)
> > I think this should be handled just like the common case :
> >
> > 5. If the file exists in both old package and new package, and both backup
> > arrays, then we let the md5sum logic in add.c decide if a .pacnew needs to
> be
> > extracted or not.

Hmm. This is not implemented yet?;-) [see EDIT;-]

> > > Comments please, especially if I missed hard cases. Once we get this
> > > hammered out this needs to be documented somewhere.
> > >
> >
> > There are probably other weird cases, but if all the above are handled
> > correctly, it should be good.
> 
> So given all this- do you want to go ahead and try to patch it so we
> get the expected behavior? I think your patch is a step in the right
> direction (although we may end up changing the way old & new backup
> arrays are combined because of some of the above). The biggest
> weirdness is our backup logic has to be split in two places- the
> remove and add code- and they can't really talk to each other easier
> unless we make much bigger changes. I'd prefer to have a small patch
> now and maybe overhaul this later once we get a release out the door.
> 
> -Dan

Poor Xavier... What about you and Aaron? Patches are welcome.

Bye


----------------------------------------------------
SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu
This mail sent through IMP: http://horde.org/imp/