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@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/