[pacman-dev] [PATCH v2] Enable inverted patterns in NoExtract and NoUpgrade.

Patrick Steinhardt steinhardt.ptk at gmail.com
Tue Jun 18 10:51:40 EDT 2013


First thanks for your feedback.

On Tue, Jun 18, 2013 at 09:22:54AM -0400, Andrew Gregory wrote:
> On 06/18/13 at 09:14am, Patrick Steinhardt wrote:
> > It is now possible to invert patterns in NoExtract and NoUpgrade.
> > This feature allows users to whitelist certain files that were
> > previously blacklisted by another entry.
> > ---
> >  doc/pacman.conf.5.txt | 10 ++++++++--
> >  lib/libalpm/add.c     |  4 ++--
> >  lib/libalpm/util.c    | 32 ++++++++++++++++++++++++++++++++
> >  lib/libalpm/util.h    |  1 +
> >  4 files changed, 43 insertions(+), 4 deletions(-)
> > 
> 
> Looks good.  Fairly minor comments below.
> 
> > diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt
> > index 83a399e..e87f4c2 100644
> > --- a/doc/pacman.conf.5.txt
> > +++ b/doc/pacman.conf.5.txt
> > @@ -128,7 +128,10 @@ Options
> >  	'.pacnew' extension.
> >  	These files refer to files in the package archive, so do not include the
> >  	leading slash (the RootDir) when specifying them. Shell-style glob patterns
> > -	are allowed.
> > +	are allowed. It is possible to invert matches by prepending a file with
> > +	an exclamation mark. Inverted files will result in previously blacklisted
> > +	files to be whitelisted again. Subsequent matches will override previous
> > +	ones.
> 
> s/to be/being/
> 
> Need to mention that a leading literal ! or \ needs to be escaped.

I'll change those.

> 
> >  
> >  *NoExtract =* file ...::
> >  	All files listed with a `NoExtract` directive will never be extracted from
> > @@ -138,7 +141,10 @@ Options
> >  	from the 'apache' package.
> >  	These files refer to files in the package archive, so do not include the
> >  	leading slash (the RootDir) when specifying them. Shell-style glob patterns
> > -	are allowed.
> > +	are allowed. It is possible to invert matches by prepending a file with
> > +	an exclamation mark. Inverted files will result in previously blacklisted
> > +	files to be whitelisted again. Subsequent matches will override previous
> > +	ones.
> 
> Same.
> 
> >  
> >  *CleanMethod =* KeepInstalled &| KeepCurrent::
> >  	If set to `KeepInstalled` (the default), the '-Sc' operation will clean
> > diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c
> > index c20e7c6..6e12f56 100644
> > --- a/lib/libalpm/add.c
> > +++ b/lib/libalpm/add.c
> > @@ -183,7 +183,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
> >  	}
> >  
> >  	/* if a file is in NoExtract then we never extract it */
> > -	if(alpm_list_find(handle->noextract, entryname, _alpm_fnmatch)) {
> > +	if(_alpm_fnmatch_patterns(handle->noextract, entryname) == 0) {
> >  		_alpm_log(handle, ALPM_LOG_DEBUG, "%s is in NoExtract,"
> >  				" skipping extraction of %s\n",
> >  				entryname, filename);
> > @@ -245,7 +245,7 @@ static int extract_single_file(alpm_handle_t *handle, struct archive *archive,
> >  		} else {
> >  			/* case 3: */
> >  			/* if file is in NoUpgrade, don't touch it */
> > -			if(alpm_list_find(handle->noupgrade, entryname, _alpm_fnmatch)) {
> > +			if(_alpm_fnmatch_patterns(handle->noupgrade, entryname) == 0) {
> >  				notouch = 1;
> >  			} else {
> >  				alpm_backup_t *backup;
> > diff --git a/lib/libalpm/util.c b/lib/libalpm/util.c
> > index f84fb66..1382889 100644
> > --- a/lib/libalpm/util.c
> > +++ b/lib/libalpm/util.c
> > @@ -1249,6 +1249,38 @@ int _alpm_access(alpm_handle_t *handle, const char *dir, const char *file, int a
> >  	return ret;
> >  }
> >  
> > +/** Checks whether a string matches at least one shell wildcard pattern.
> > + * Checks for matches with fnmatch. Matches are inverted by prepending
> > + * patterns with an exclamation mark. Preceding exclamation marks may be
> > + * escaped. Subsequent matches override previous ones.
> > + * @param patterns patterns to match against
> > + * @param string string to check against pattern
> > + * @return 0 if string matches pattern, negative if they don't match and
> > + * positive if the last match was inverted
> > + */
> > +int _alpm_fnmatch_patterns(alpm_list_t *patterns, const char *string)
> > +{
> > +	int result = -1;
> > +	alpm_list_t *i;
> > +	char *pattern;
> > +	short inverted;
> 
> pattern and inverted should be moved to inside the loop.

I'd normally agree with that, but I adapted to the code style in
util.c where variables used only inside a loop are still declared
outside of it.

> 
> > +
> > +	for(i = patterns; i; i = i->next) {
> > +		pattern = i->data;
> > +
> > +		inverted = pattern[0] == '!';
> > +		if(inverted || pattern[0] == '\\') {
> > +			pattern++;
> > +		}
> > +
> > +		if(_alpm_fnmatch(pattern, string) == 0) {
> > +			result = inverted;
> 
> If we just loop through the list backwards (or reverse the list) we
> could return after the first match instead of saving the result and
> continuing the loop.

Indeed, you're right with that one. Even though I didn't think
about it and it certainly is a good optimisation I don't want to
include it as there are some problems with that.

First option would be to reverse the list. It might either be
reversed on NoExtract parsing, resulting in conventions that may
introduce bugs more easily, or on each call of
_alpm_fnmatch_patterns which would certainly kill all benefit by
reverse-iterating.

Second option is using the prev links of alpm_list, zut by
default, they've got an infinite loop when using the prev links.
alpm_list_reverse handles this by backing up head's prev and
restoring it later. But that's not a nice solution which should
be used here, I guess.

So if I didn't forget an easier method I'm against that
optimisation. I guess it won't matter much anyway, as NoExtract
or NoUpgrade lists likely will not be that huge anyway.

> 
> > +		}
> > +	}
> > +
> > +	return result;
> > +}
> > +
> >  /** Checks whether a string matches a shell wildcard pattern.
> >   * Wrapper around fnmatch.
> >   * @param pattern pattern to match against
> > diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h
> > index 56031f3..24b7c22 100644
> > --- a/lib/libalpm/util.h
> > +++ b/lib/libalpm/util.h
> > @@ -140,6 +140,7 @@ alpm_time_t _alpm_parsedate(const char *line);
> >  int _alpm_raw_cmp(const char *first, const char *second);
> >  int _alpm_raw_ncmp(const char *first, const char *second, size_t max);
> >  int _alpm_access(alpm_handle_t *handle, const char *dir, const char *file, int amode);
> > +int _alpm_fnmatch_patterns(alpm_list_t *patterns, const char *string);
> >  int _alpm_fnmatch(const void *pattern, const void *string);
> >  
> >  #ifndef HAVE_STRSEP
> > -- 
> > 1.8.3.1
> > 
> 
> apg
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://mailman.archlinux.org/pipermail/pacman-dev/attachments/20130618/eebc03be/attachment.asc>


More information about the pacman-dev mailing list