[pacman-dev] [PATCH] remove: use strcmp for files in skip_remove

[Andrew Gregory]

On 03/03/15 at 04:10pm, Allan McRae wrote:
> On 30/11/14 09:02, Andrew Gregory wrote:
> > Paths from noupgrade, the transaction skip_remove, and package backup
> > lists were combined into a single list matched using fnmatch causing
> > paths with glob characters to match unrelated files.
> > 
> > Signed-off-by: Andrew Gregory <andrew.gregory.8 at gmail.com>
> > ---
> >  lib/libalpm/remove.c                             | 82 ++++++++++--------------
> >  test/pacman/tests/skip-remove-with-glob-chars.py | 19 ++++++
> >  2 files changed, 52 insertions(+), 49 deletions(-)
> >  create mode 100644 test/pacman/tests/skip-remove-with-glob-chars.py

<snip>

> > @@ -564,6 +546,24 @@ static int unlink_file(alpm_handle_t *handle, alpm_pkg_t *oldpkg,
> >  }
> >  
> >  /**
> > + * @brief Check if a package file should be removed.
> > + *
> > + * @param handle the context handle
> > + * @param newpkg the package replacing the current owner of \a path
> > + * @param path file to be removed
> > + *
> > + * @return 1 if the file should be skipped, 0 if it should be removed
> > + */
> > +static int _should_skip_file(alpm_handle_t *handle,
> 
> Remove underscore at start of name

Updated on my bugfix branch.

> > +		alpm_pkg_t *newpkg, const char *path)
> > +{
> > +	return _alpm_fnmatch_patterns(handle->noupgrade, path) == 0
> > +		|| alpm_list_find_str(handle->trans->skip_remove, path)
> 
> 3af0268f -> _alpm_fnmatch_patterns instead of alpm_list_find_str
> 
> Hrm...  If I make this change tests fail all over the place.  I have not
> investigated further.

This is correct as-is.  skip_remove should be searched using find_str
because it contains paths, not globs.

> > +		|| (newpkg && _alpm_needbackup(path, newpkg)
> > +				&& alpm_filelist_contains(alpm_pkg_get_files(newpkg), path));
> > +}
> > +
> > +/**
> >   * @brief Remove a package's files, optionally skipping its replacement's
> >   * files.
> >   *

<snip>

> > diff --git a/test/pacman/tests/skip-remove-with-glob-chars.py b/test/pacman/tests/skip-remove-with-glob-chars.py
> > new file mode 100644
> > index 0000000..039e150
> > --- /dev/null
> > +++ b/test/pacman/tests/skip-remove-with-glob-chars.py
> > @@ -0,0 +1,19 @@
> > +self.description = "transferred file with glob characters that match a removed file"
> > +
> > +lp = pmpkg("foo")
> > +lp.files = ["foo/b*r", "foo/bar"]
> > +self.addpkg2db("local", lp)
> > +
> > +sp1 = pmpkg("foo", "1.0-2")
> > +self.addpkg(sp1)
> > +
> > +sp2 = pmpkg("bar", "1.0-2")
> > +sp2.files = ["foo/b*r"]
> > +self.addpkg(sp2)
> > +
> > +self.args = "-U %s %s" % (sp1.filename(), sp2.filename())
> > +
> > +self.addrule("PKG_VERSION=foo|1.0-2")
> > +self.addrule("PKG_VERSION=bar|1.0-2")
> > +self.addrule("FILE_EXIST=foo/b*r")
> > +self.addrule("!FILE_EXIST=foo/bar")
> > 
> Needs added to TESTS.

Updated on my bugfix branch.

apg