[pacman-dev] [PATCH] Readd some symlink checking in file conflict code

Nagy Gabor ngaba at bibl.u-szeged.hu
Wed Jul 23 07:58:29 EDT 2008


> Well, I am unable to understand that code, but maybe Nagy did, since
> he was able to provide a (fileconflict005 ?) pactest which fails after
> this patch :
> http://archlinux.org/pipermail/pacman-dev/2008-July/012522.html
> That pactest also triggers this "%s is a symlink to a dir in previous
> package\n" message which looks totally bogus.

OK. That ugly (not portable?) inode hack catches this case
_accidentally_ which can be replaced by a much "cheaper" test, if
needed.
So what is happening here? See commit
584ffa6aef13d0933ad4930ab9cb70d3af2977ff.
First of all, that part checks whether the _local file_ is a symlink
pointing to a to-be-removed file (dir?). I don't see why this case
should be handled at all. Clearly, this is not that case, we have a
local conflicting _directory_, not symlink, but the code believes that
it is that case.
To be short: S_ISLNK(lsbuf.st_mode) check is missing from the removed
code-part. This leads to the false decision which is good from
fileconflict004.py viewpoint.
See the decision part:
if(!_alpm_lstat(str, &pkgbuf) && lsbuf.st_ino == pkgbuf.st_ino)...
/* lsbuf represents the conflicting file, pkgbuf loops over the 'files'
of local package */
Since there is no symlink here, _alpm_lstat is equivalent with stat
(the code _assumed_ symlink here). So this part will simply detect that
our conflicting directory belongs to the local package 'files' list.
(Which doesn't mean that it will be removed, see fileconflict005.py).

So if you want to restore the old behaviour, this code-part can be
replaced with an easy "find directory-name in localpkg's filelist"
search (maybe tuned with realpath).

> "/path/to/pacman/root/test is a symlink to a dir in previous package"
> 
> I don't see how this reflects the situation of "fileconflict005" at
> all. In the new package, test is a symlink to a new directory test2.
> and test2 did not exist in any previous package.
> However, test existed in previous packages, as a directory.
> 
> Anyway, here are the different ways I propose to deal with this
> issue : 1) choose that fileconflict003 is more important than 004,
> and only supports that situation (current 3.2 code)
> 2) choose that 004 is more important than 003, so slightly edit the
> code to get back to this old behavior (of 3.0?)
> 3) we decide that we need to support both, in that case we go the way
> that Nagy suggested in the beginning :
> http://archlinux.org/pipermail/pacman-dev/2008-July/012470.html
> 

I am considering a "cheaper" solution now, only check the old version
of package (not the whole target list). But that cannot

My opinion about fileconflict check, that it is really fast, but it has
some bugs (most important: trans001.py). I think we should decide if we
want to implement FS#8585 *soon* or not, that would solve most of these
issues. If we want, I don't think we should waste too much effort to
this to-be-removed fileconflict check part.

Bye




More information about the pacman-dev mailing list