On 06/18/13 at 04:51pm, Patrick Steinhardt wrote:
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.
alpm_list_previous will stop at the beginning of the list.
+ } + } + + 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