[pacman-dev] [PATCH 0/5] Properly detect conflicts with directory symlinks

newer
[pacman-dev] [PATCH] PKGBUILD.vim:...

older
[pacman-dev] [PATCH] Mark already...

Allan McRae

29 Jul 2012 29 Jul '12

6:18 a.m.

This series of patches make pacman perform conflict checking with all directory paths resovled. This means that inter-package conflicts such as /lib/foo and /usr/lib/foo when /lib -> usr/lib are detected. Similarly, if a package moves a file from /lib/foo to /usr/lib/foo when /lib -> usr/lib (i.e. the file is effectively the same), this is no longer detected as a conflict with the filesystem. A big thanks to Andrew for fixing my initial attempt at the resolving of the filelist and making my idea for efficiently storing paths that are unchanged by resolving a reality. Allan McRae (3): Add resolved_path to alpm_filelist_t Resolve file paths during inter-package conflict check Avoid upgrade conflict with unchanged effective path Andrew Gregory (2): Add _alpm_filelist_resolve _alpm_filelist_resolve: use original filenames where possible lib/libalpm/alpm.h | 1 + lib/libalpm/conflict.c | 6 ++ lib/libalpm/filelist.c | 174 ++++++++++++++++++++++++++++++++--- lib/libalpm/filelist.h | 2 + lib/libalpm/package.c | 22 ++++- test/pacman/tests/fileconflict001.py | 2 - test/pacman/tests/fileconflict013.py | 2 - test/pacman/tests/fileconflict016.py | 2 - test/pacman/tests/fileconflict017.py | 26 ++++++ 9 files changed, 217 insertions(+), 20 deletions(-) create mode 100644 test/pacman/tests/fileconflict017.py -- 1.7.11.3

Show replies by date

Allan McRae

29 Jul 29 Jul

6:18 a.m.

New subject: [pacman-dev] [PATCH 1/5] Add resolved_path to alpm_filelist_t

Add an array to hold the resolved paths of the files in alpm_filelist_t. When the file name and its resolved file name are identical, the pointer to the original file name is used to avoid duplicate memory allocation. Signed-off-by: Allan McRae --- lib/libalpm/alpm.h | 1 + lib/libalpm/package.c | 11 ++++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 9aa6ac2..c341b17 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -213,6 +213,7 @@ typedef struct _alpm_file_t { typedef struct _alpm_filelist_t { size_t count; alpm_file_t *files; + char **resolved_path; } alpm_filelist_t; /** Local package or package file backup entry */ diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index a4c3309..ab84329 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -540,6 +540,9 @@ int _alpm_pkg_dup(alpm_pkg_t *pkg, alpm_pkg_t **new_ptr) } } newpkg->files.count = pkg->files.count; + /* deliberately do not copy resolved_path as this is only used + * during conflict checking and the sorting of list does not readily + * allow keeping its efficient memory usage when copying */ } /* internal */ @@ -590,9 +593,15 @@ void _alpm_pkg_free(alpm_pkg_t *pkg) if(pkg->files.count) { size_t i; for(i = 0; i < pkg->files.count; i++) { - free(pkg->files.files[i].name); + FREE(pkg->files.files[i].name); } free(pkg->files.files); + if(pkg->files.resolved_path) { + for(i = 0; i < pkg->files.count; i++) { + free(pkg->files.resolved_path[i]); + } + free(pkg->files.resolved_path); + } } alpm_list_free_inner(pkg->backup, (alpm_list_fn_free)_alpm_backup_free); alpm_list_free(pkg->backup); -- 1.7.11.3

Allan McRae

6:18 a.m.

New subject: [pacman-dev] [PATCH 2/5] Add _alpm_filelist_resolve

From: Andrew Gregory The _alpm_filelist_resolve function takes a filelist and creates a list with any symlinks in directory paths resolved. Signed-off-by: Allan McRae --- lib/libalpm/filelist.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/filelist.h | 2 + 2 files changed, 158 insertions(+) diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index 1928056..d32a3e5 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -17,10 +17,166 @@ * along with this program. If not, see http://www.gnu.org/licenses/. */ +#include #include +#include /* libalpm */ #include "filelist.h" +#include "util.h" + +/** Helper function for comparing strings when sorting */ +static int _strcmp(const void *s1, const void *s2) +{ + return strcmp(*(char **)s1, *(char **)s2); +} + +/** + * @brief Resolves a symlink and its children. + * + * @attention Pre-condition: files must be sorted! + * + * @param files filelist to resolve + * @param i index in files to start processing + * @param path absolute path for the symlink being resolved + * @param root_len length of the root portion of path + * @param resolving is file \i in \files a symlink that needs to be resolved + * + * @return the index of the last file resolved + */ +size_t _alpm_filelist_resolve_link( + alpm_filelist_t *files, size_t i, char *path, size_t root_len, int resolving) +{ + struct stat sbuf; + const char *causal_dir; /* symlink being resolved */ + char *filename_r; /* resolved filename */ + size_t causal_dir_len = 0, causal_dir_r_len = 0; + + if(resolving) { + /* deal with the symlink being resolved */ + filename_r = malloc(PATH_MAX); + causal_dir = files->files[i].name; + causal_dir_len = strlen(causal_dir); + if(realpath(path, filename_r) == NULL) { + files->resolved_path[i] = strdup(causal_dir); + return i; + } + causal_dir_r_len = strlen(filename_r + root_len) + 1; + if(causal_dir_r_len >= PATH_MAX) { + files->resolved_path[i] = strdup(causal_dir); + return i; + } + /* remove root_r from filename_r */ + memmove(filename_r, filename_r + root_len, causal_dir_r_len + 1); + strcpy(filename_r + causal_dir_r_len - 1, "/"); + files->resolved_path[i] = strdup(filename_r); + i++; + } + + for(; i < files->count && (!resolving || + strncmp(files->files[i].name, causal_dir, causal_dir_len) == 0); i++) { + char *filename = files->files[i].name; + size_t f_len = strlen(filename); + + if(resolving) { + if(f_len + causal_dir_r_len - causal_dir_len > PATH_MAX) { + files->resolved_path[i] = strdup(filename); + continue; + } + + strcpy(filename_r + causal_dir_r_len, filename + causal_dir_len); + + if(root_len + causal_dir_r_len + f_len - causal_dir_len > PATH_MAX) { + /* absolute path is too long */ + files->resolved_path[i] = strdup(filename_r); + continue; + } + } else { + filename_r = filename; + } + + /* deal with files and paths too long to resolve*/ + if(filename[f_len-1] != '/') { + files->resolved_path[i] = strdup(filename_r); + continue; + } + + /* construct absolute path and stat() */ + strcpy(path + root_len, filename_r); + int exists = !_alpm_lstat(path, &sbuf); + + /* deal with symlinks */ + if(exists && S_ISLNK(sbuf.st_mode)) { + i = _alpm_filelist_resolve_link(files, i, path, root_len, 1); + continue; + } + + /* deal with normal directories */ + files->resolved_path[i] = strdup(filename_r); + + /* deal with children of non-existent directories to reduce lstat() calls */ + if (!exists) { + char *f; + i++; + while(i < files->count && strncmp(files->files[i].name, filename, f_len) == 0) { + f = files->files[i].name; + if(resolving && strlen(f + causal_dir_len) + causal_dir_r_len <= PATH_MAX) { + strcpy(filename_r + causal_dir_r_len, f + causal_dir_len); + files->resolved_path[i] = strdup(filename_r); + } else { + files->resolved_path[i] = strdup(f); + } + i++; + } + i--; + } + } + + if(resolving) { + free(filename_r); + } + + return i-1; +} + +/** + * @brief Takes a file list and resolves all directory paths according to filesystem + * + * @attention Pre-condition: files must be sorted! + * + * @note A symlink and directory at the same path in two difference packages + * causes a conflict so the filepath can not change as packages get installed + * + * @param handle the context handle + * @param files list of files to resolve + */ +void _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files) +{ + char path[PATH_MAX]; + size_t root_len; + + if(!files || files->resolved_path) { + return; + } + + MALLOC(files->resolved_path, files->count * sizeof(char*), return); + memset(files->resolved_path, 0, files->count); + + /* not much point in going on if we can't even resolve root */ + if(realpath(handle->root, path) == NULL){ + return; + } + root_len = strlen(path) + 1; + if(root_len >= PATH_MAX) { + return; + } + strcpy(path + root_len - 1, "/"); + + _alpm_filelist_resolve_link(files, 0, path, root_len, 0); + + qsort(files->resolved_path, files->count, sizeof(char *), _strcmp); +} + /* Returns the difference of the provided two lists of files. * Pre-condition: both lists are sorted! diff --git a/lib/libalpm/filelist.h b/lib/libalpm/filelist.h index 2d5cbc0..3152b9d 100644 --- a/lib/libalpm/filelist.h +++ b/lib/libalpm/filelist.h @@ -21,6 +21,8 @@ #include "alpm.h" +size_t _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t i, char *path, size_t root_len, int resolving); +void _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files); alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA, alpm_filelist_t *filesB); -- 1.7.11.3

Dave Reisner

1:50 p.m.

New subject: [pacman-dev] [PATCH 2/5] Add _alpm_filelist_resolve

On Sun, Jul 29, 2012 at 04:18:32PM +1000, Allan McRae wrote:

...

From: Andrew Gregory

The _alpm_filelist_resolve function takes a filelist and creates a list with any symlinks in directory paths resolved.

Signed-off-by: Allan McRae --- lib/libalpm/filelist.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/filelist.h | 2 + 2 files changed, 158 insertions(+)

diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index 1928056..d32a3e5 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -17,10 +17,166 @@ * along with this program. If not, see http://www.gnu.org/licenses/. */

+#include #include +#include

/* libalpm */ #include "filelist.h" +#include "util.h" + +/** Helper function for comparing strings when sorting */ +static int _strcmp(const void *s1, const void *s2)

I'm worried about symbol clashes here, since technically '_' isn't our namespace. Please pick a name similar to what we use elsewhere for comparison callbacks, such as _alpm_str_cmp or _alpm_files_cmp.

...

+{ + return strcmp(*(char **)s1, *(char **)s2); +} + +/** + * @brief Resolves a symlink and its children. + * + * @attention Pre-condition: files must be sorted! + * + * @param files filelist to resolve + * @param i index in files to start processing + * @param path absolute path for the symlink being resolved + * @param root_len length of the root portion of path + * @param resolving is file \i in \files a symlink that needs to be resolved + * + * @return the index of the last file resolved + */ +size_t _alpm_filelist_resolve_link( + alpm_filelist_t *files, size_t i, char *path, size_t root_len, int resolving) +{ + struct stat sbuf; + const char *causal_dir; /* symlink being resolved */ + char *filename_r; /* resolved filename */ + size_t causal_dir_len = 0, causal_dir_r_len = 0; + + if(resolving) { + /* deal with the symlink being resolved */ + filename_r = malloc(PATH_MAX);

This seems very prone to leakage. Why not just allocate it on the stack?

...

+ causal_dir = files->files[i].name; + causal_dir_len = strlen(causal_dir); + if(realpath(path, filename_r) == NULL) { + files->resolved_path[i] = strdup(causal_dir);

STRDUP macro?

...

+ return i; + } + causal_dir_r_len = strlen(filename_r + root_len) + 1; + if(causal_dir_r_len >= PATH_MAX) { + files->resolved_path[i] = strdup(causal_dir);

STRDUP...

...

+ return i; + } + /* remove root_r from filename_r */ + memmove(filename_r, filename_r + root_len, causal_dir_r_len + 1); + strcpy(filename_r + causal_dir_r_len - 1, "/"); + files->resolved_path[i] = strdup(filename_r);

STRDUP... I'm done pointing these out, but there's more.

...

+ i++; + } + + for(; i < files->count && (!resolving || + strncmp(files->files[i].name, causal_dir, causal_dir_len) == 0); i++) {

This seems really ugly. Can't the second half of the exit condition be moved into the body of the loop?

...

+ char *filename = files->files[i].name; + size_t f_len = strlen(filename); + + if(resolving) { + if(f_len + causal_dir_r_len - causal_dir_len > PATH_MAX) { + files->resolved_path[i] = strdup(filename); + continue; + } + + strcpy(filename_r + causal_dir_r_len, filename + causal_dir_len);

Are you sure this never copies more than PATH_MAX bytes into filename_r? There's more of these buffer overflow concerns that flat out aren't handled.

...

+ + if(root_len + causal_dir_r_len + f_len - causal_dir_len > PATH_MAX) { + /* absolute path is too long */ + files->resolved_path[i] = strdup(filename_r); + continue; + } + } else { + filename_r = filename; + } + + /* deal with files and paths too long to resolve*/ + if(filename[f_len-1] != '/') {

Style nit -- add space around the '-', and a space before the close of the comment. I'm not really sure how this does what the comment says.

...

+ files->resolved_path[i] = strdup(filename_r); + continue; + } + + /* construct absolute path and stat() */ + strcpy(path + root_len, filename_r); + int exists = !_alpm_lstat(path, &sbuf); + + /* deal with symlinks */ + if(exists && S_ISLNK(sbuf.st_mode)) { + i = _alpm_filelist_resolve_link(files, i, path, root_len, 1); + continue; + } + + /* deal with normal directories */ + files->resolved_path[i] = strdup(filename_r); + + /* deal with children of non-existent directories to reduce lstat() calls */ + if (!exists) { + char *f; + i++; + while(i < files->count && strncmp(files->files[i].name, filename, f_len) == 0) { + f = files->files[i].name; + if(resolving && strlen(f + causal_dir_len) + causal_dir_r_len <= PATH_MAX) { + strcpy(filename_r + causal_dir_r_len, f + causal_dir_len); + files->resolved_path[i] = strdup(filename_r); + } else { + files->resolved_path[i] = strdup(f); + } + i++; + } + i--; + } + } + + if(resolving) { + free(filename_r); + } + + return i-1; +} + +/** + * @brief Takes a file list and resolves all directory paths according to filesystem + * + * @attention Pre-condition: files must be sorted! + * + * @note A symlink and directory at the same path in two difference packages + * causes a conflict so the filepath can not change as packages get installed + * + * @param handle the context handle + * @param files list of files to resolve + */ +void _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files) +{ + char path[PATH_MAX]; + size_t root_len; + + if(!files || files->resolved_path) { + return; + } + + MALLOC(files->resolved_path, files->count * sizeof(char*), return); + memset(files->resolved_path, 0, files->count);

Just use CALLOC instead of MALLOC+memset

...

+ + /* not much point in going on if we can't even resolve root */ + if(realpath(handle->root, path) == NULL){ + return; + } + root_len = strlen(path) + 1; + if(root_len >= PATH_MAX) { + return; + } + strcpy(path + root_len - 1, "/");

Why not use direct assignment or memcpy here?

...

+ + _alpm_filelist_resolve_link(files, 0, path, root_len, 0); + + qsort(files->resolved_path, files->count, sizeof(char *), _strcmp); +} +

/* Returns the difference of the provided two lists of files. * Pre-condition: both lists are sorted! diff --git a/lib/libalpm/filelist.h b/lib/libalpm/filelist.h index 2d5cbc0..3152b9d 100644 --- a/lib/libalpm/filelist.h +++ b/lib/libalpm/filelist.h @@ -21,6 +21,8 @@

#include "alpm.h"

+size_t _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t i, char *path, size_t root_len, int resolving);

Wrap at 80 columns, please.

...

+void _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files);

alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA, alpm_filelist_t *filesB); -- 1.7.11.3

Andrew Gregory

31 Jul 31 Jul

12:36 a.m.

New subject: [pacman-dev] [PATCH 2/5] Add _alpm_filelist_resolve

On Sun, 29 Jul 2012 09:50:13 -0400 Dave Reisner wrote:

...

On Sun, Jul 29, 2012 at 04:18:32PM +1000, Allan McRae wrote:

...
From: Andrew Gregory

The _alpm_filelist_resolve function takes a filelist and creates a list with any symlinks in directory paths resolved.

Signed-off-by: Allan McRae --- lib/libalpm/filelist.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/filelist.h | 2 + 2 files changed, 158 insertions(+)

diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index 1928056..d32a3e5 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -17,10 +17,166 @@ * along with this program. If not, see http://www.gnu.org/licenses/. */

+#include #include +#include

/* libalpm */ #include "filelist.h" +#include "util.h" + +/** Helper function for comparing strings when sorting */ +static int _strcmp(const void *s1, const void *s2)

I'm worried about symbol clashes here, since technically '_' isn't our namespace. Please pick a name similar to what we use elsewhere for comparison callbacks, such as _alpm_str_cmp or _alpm_files_cmp.

...
+{ + return strcmp(*(char **)s1, *(char **)s2); +} + +/** + * @brief Resolves a symlink and its children. + * + * @attention Pre-condition: files must be sorted! + * + * @param files filelist to resolve + * @param i index in files to start processing + * @param path absolute path for the symlink being resolved + * @param root_len length of the root portion of path + * @param resolving is file \i in \files a symlink that needs to be resolved + * + * @return the index of the last file resolved + */ +size_t _alpm_filelist_resolve_link( + alpm_filelist_t *files, size_t i, char *path, size_t root_len, int resolving) +{ + struct stat sbuf; + const char *causal_dir; /* symlink being resolved */ + char *filename_r; /* resolved filename */ + size_t causal_dir_len = 0, causal_dir_r_len = 0; + + if(resolving) { + /* deal with the symlink being resolved */ + filename_r = malloc(PATH_MAX);

This seems very prone to leakage. Why not just allocate it on the stack?

filename_r is only needed if a symlink is being resolved, so most of the time allocating memory for it would be a waste. It also helps prevent a stack overflow since this function is recursive. You are right about the leakage though, as evidenced a whole 5 lines below... I suppose it should be changed to the MALLOC macro though; I keep forgetting about those things.

...

...
+ causal_dir = files->files[i].name; + causal_dir_len = strlen(causal_dir); + if(realpath(path, filename_r) == NULL) { + files->resolved_path[i] = strdup(causal_dir);

STRDUP macro?

...
+ return i; + } + causal_dir_r_len = strlen(filename_r + root_len) + 1; + if(causal_dir_r_len >= PATH_MAX) { + files->resolved_path[i] = strdup(causal_dir);

STRDUP...

...
+ return i; + } + /* remove root_r from filename_r */ + memmove(filename_r, filename_r + root_len, causal_dir_r_len + 1); + strcpy(filename_r + causal_dir_r_len - 1, "/"); + files->resolved_path[i] = strdup(filename_r);

STRDUP... I'm done pointing these out, but there's more.

...
+ i++; + } + + for(; i < files->count && (!resolving || + strncmp(files->files[i].name, causal_dir, causal_dir_len) == 0); i++) {

This seems really ugly. Can't the second half of the exit condition be moved into the body of the loop?

...
+ char *filename = files->files[i].name; + size_t f_len = strlen(filename); + + if(resolving) { + if(f_len + causal_dir_r_len - causal_dir_len > PATH_MAX) { + files->resolved_path[i] = strdup(filename); + continue; + } + + strcpy(filename_r + causal_dir_r_len, filename + causal_dir_len);

Are you sure this never copies more than PATH_MAX bytes into filename_r? There's more of these buffer overflow concerns that flat out aren't handled.

Unless I got the math wrong, the if directly above ensures that very thing. All of the strcpy()s, with the notable exception of the one directly below, have similar checks.

...

...
+ + if(root_len + causal_dir_r_len + f_len - causal_dir_len > PATH_MAX) { + /* absolute path is too long */ + files->resolved_path[i] = strdup(filename_r); + continue; + } + } else { + filename_r = filename; + } + + /* deal with files and paths too long to resolve*/ + if(filename[f_len-1] != '/') {

Style nit -- add space around the '-', and a space before the close of the comment. I'm not really sure how this does what the comment says.

Probably because I erroneously moved the length check above when refactoring...

...

...
+ files->resolved_path[i] = strdup(filename_r); + continue; + } + + /* construct absolute path and stat() */ + strcpy(path + root_len, filename_r); + int exists = !_alpm_lstat(path, &sbuf); + + /* deal with symlinks */ + if(exists && S_ISLNK(sbuf.st_mode)) { + i = _alpm_filelist_resolve_link(files, i, path, root_len, 1); + continue; + } + + /* deal with normal directories */ + files->resolved_path[i] = strdup(filename_r); + + /* deal with children of non-existent directories to reduce lstat() calls */ + if (!exists) { + char *f; + i++; + while(i < files->count && strncmp(files->files[i].name, filename, f_len) == 0) { + f = files->files[i].name; + if(resolving && strlen(f + causal_dir_len) + causal_dir_r_len <= PATH_MAX) { + strcpy(filename_r + causal_dir_r_len, f + causal_dir_len); + files->resolved_path[i] = strdup(filename_r); + } else { + files->resolved_path[i] = strdup(f); + } + i++; + } + i--; + } + } + + if(resolving) { + free(filename_r); + } + + return i-1; +} + +/** + * @brief Takes a file list and resolves all directory paths according to filesystem + * + * @attention Pre-condition: files must be sorted! + * + * @note A symlink and directory at the same path in two difference packages + * causes a conflict so the filepath can not change as packages get installed + * + * @param handle the context handle + * @param files list of files to resolve + */ +void _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files) +{ + char path[PATH_MAX]; + size_t root_len; + + if(!files || files->resolved_path) { + return; + } + + MALLOC(files->resolved_path, files->count * sizeof(char*), return); + memset(files->resolved_path, 0, files->count);

Just use CALLOC instead of MALLOC+memset

...
+ + /* not much point in going on if we can't even resolve root */ + if(realpath(handle->root, path) == NULL){ + return; + } + root_len = strlen(path) + 1; + if(root_len >= PATH_MAX) { + return; + } + strcpy(path + root_len - 1, "/");

Why not use direct assignment or memcpy here?

...
+ + _alpm_filelist_resolve_link(files, 0, path, root_len, 0); + + qsort(files->resolved_path, files->count, sizeof(char *), _strcmp); +} +

/* Returns the difference of the provided two lists of files. * Pre-condition: both lists are sorted! diff --git a/lib/libalpm/filelist.h b/lib/libalpm/filelist.h index 2d5cbc0..3152b9d 100644 --- a/lib/libalpm/filelist.h +++ b/lib/libalpm/filelist.h @@ -21,6 +21,8 @@

#include "alpm.h"

+size_t _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t i, char *path, size_t root_len, int resolving);

Wrap at 80 columns, please.

...
+void _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files);

alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA, alpm_filelist_t *filesB); -- 1.7.11.3

Andrew Gregory

7 Aug 7 Aug

2:08 a.m.

New subject: [pacman-dev] [PATCH] Add _alpm_filelist_resolve

The _alpm_filelist_resolve function takes a filelist and creates a list with any symlinks in directory paths resolved. Signed-off-by: Andrew Gregory --- Made changes according to Dave's suggestions. Also made some minor adjustments to bring it better into line with the coding standards and hopefully make it more readable. lib/libalpm/filelist.c | 178 +++++++++++++++++++++++++++++++++++++++++++++++++ lib/libalpm/filelist.h | 3 + 2 files changed, 181 insertions(+) diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index 1928056..783c8bf 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -17,10 +17,188 @@ * along with this program. If not, see http://www.gnu.org/licenses/. */ +#include #include +#include /* libalpm */ #include "filelist.h" +#include "util.h" + +/** Helper function for comparing strings when sorting */ +static int _alpm_filelist_strcmp(const void *s1, const void *s2) +{ + return strcmp(*(char **)s1, *(char **)s2); +} + +/** + * @brief Resolves a symlink and its children. + * + * @attention Pre-condition: files must be sorted! + * + * @param files filelist to resolve + * @param i index in files to start processing + * @param path absolute path for the symlink being resolved + * @param root_len length of the root portion of path + * @param resolving is file \i in \files a symlink that needs to be resolved + * + * @return the index of the last file resolved + */ +size_t _alpm_filelist_resolve_link( + alpm_filelist_t *files, size_t i, char *path, size_t root_len, int resolving) +{ + char *causal_dir; /* symlink being resolved */ + char *filename_r = NULL; /* resolved filename */ + size_t causal_dir_len = 0, causal_dir_r_len = 0; + + if(resolving) { + /* deal with the symlink being resolved */ + MALLOC(filename_r, PATH_MAX, goto error); + causal_dir = files->files[i].name; + causal_dir_len = strlen(causal_dir); + if(realpath(path, filename_r) == NULL) { + STRDUP(files->resolved_path[i], causal_dir, goto error); + FREE(filename_r); + return i; + } + causal_dir_r_len = strlen(filename_r + root_len) + 1; + if(causal_dir_r_len >= PATH_MAX) { + STRDUP(files->resolved_path[i], causal_dir, goto error); + FREE(filename_r); + return i; + } + /* remove root_r from filename_r */ + memmove(filename_r, filename_r + root_len, causal_dir_r_len); + filename_r[causal_dir_r_len - 1] = '/'; + filename_r[causal_dir_r_len] = '\0'; + STRDUP(files->resolved_path[i], filename_r, goto error); + i++; + } + + for(; i < files->count; i++) { + char *filename = files->files[i].name; + size_t filename_len = strlen(filename); + size_t filename_r_len = filename_len; + struct stat sbuf; + int exists; + + if(resolving) { + if(filename_len < causal_dir_len || strncmp(filename, causal_dir, causal_dir_len) != 0) { + /* not inside causal_dir anymore */ + break; + } + + filename_r_len = filename_len + causal_dir_r_len - causal_dir_len; + if(filename_r_len >= PATH_MAX) { + /* resolved path is too long */ + STRDUP(files->resolved_path[i], filename, goto error); + continue; + } + + strcpy(filename_r + causal_dir_r_len, filename + causal_dir_len); + } else { + filename_r = filename; + } + + /* deal with files and paths too long to resolve*/ + if(filename[filename_len - 1] != '/' || root_len + filename_r_len >= PATH_MAX) { + STRDUP(files->resolved_path[i], filename_r, goto error); + continue; + } + + /* construct absolute path and stat() */ + strcpy(path + root_len, filename_r); + exists = !_alpm_lstat(path, &sbuf); + + /* deal with symlinks */ + if(exists && S_ISLNK(sbuf.st_mode)) { + i = _alpm_filelist_resolve_link(files, i, path, root_len, 1); + continue; + } + + /* deal with normal directories */ + STRDUP(files->resolved_path[i], filename_r, goto error); + + /* deal with children of non-existent directories to reduce lstat() calls */ + if (!exists) { + for(i++; i < files->count; i++) { + char *f = files->files[i].name;; + size_t f_len = strlen(f); + size_t f_r_len; + + if(f_len < filename_len || strncmp(f, filename, filename_len) != 0) { + /* not inside the non-existent dir anymore */ + break; + } + + f_r_len = f_len + causal_dir_r_len - causal_dir_len; + if(resolving && f_r_len <= PATH_MAX) { + strcpy(filename_r + causal_dir_r_len, f + causal_dir_len); + STRDUP(files->resolved_path[i], filename_r, goto error); + } else { + STRDUP(files->resolved_path[i], f, goto error); + } + } + i--; + } + } + + if(resolving) { + FREE(filename_r); + } + + return i-1; + +error: + if(resolving) { + FREE(filename_r); + } + /* out of memory, not much point in going on */ + return files->count; +} + +/** + * @brief Takes a file list and resolves all directory paths according to the + * filesystem + * + * @attention Pre-condition: files must be sorted! + * + * @note A symlink and directory at the same path in two difference packages + * causes a conflict so the filepath can not change as packages get installed + * + * @param handle the context handle + * @param files list of files to resolve + */ +void _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files) +{ + char path[PATH_MAX]; + size_t root_len; + + if(!files || files->resolved_path) { + return; + } + + CALLOC(files->resolved_path, files->count, sizeof(char *), return); + + /* not much point in going on if we can't even resolve root */ + if(realpath(handle->root, path) == NULL){ + return; + } + root_len = strlen(path) + 1; + if(root_len >= PATH_MAX) { + return; + } + path[root_len - 1] = '/'; + path[root_len] = '\0'; + + _alpm_filelist_resolve_link(files, 0, path, root_len, 0); + + qsort(files->resolved_path, files->count, sizeof(char *), + _alpm_filelist_strcmp); + + return; +} + /* Returns the difference of the provided two lists of files. * Pre-condition: both lists are sorted! diff --git a/lib/libalpm/filelist.h b/lib/libalpm/filelist.h index 2d5cbc0..ef865da 100644 --- a/lib/libalpm/filelist.h +++ b/lib/libalpm/filelist.h @@ -21,6 +21,9 @@ #include "alpm.h" +size_t _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t i, + char *path, size_t root_len, int resolving); +void _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files); alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA, alpm_filelist_t *filesB); -- 1.7.11.4

Allan McRae

29 Jul 29 Jul

6:18 a.m.

New subject: [pacman-dev] [PATCH 3/5] _alpm_filelist_resolve: use original filenames where possible

From: Andrew Gregory If a filename isn't resolved, the original can be used instead of strdup()ing it. Signed-off-by: Andrew Gregory --- lib/libalpm/filelist.c | 8 ++++---- lib/libalpm/package.c | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index d32a3e5..288907a 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -80,7 +80,7 @@ size_t _alpm_filelist_resolve_link( if(resolving) { if(f_len + causal_dir_r_len - causal_dir_len > PATH_MAX) { - files->resolved_path[i] = strdup(filename); + files->resolved_path[i] = filename; continue; } @@ -97,7 +97,7 @@ size_t _alpm_filelist_resolve_link( /* deal with files and paths too long to resolve*/ if(filename[f_len-1] != '/') { - files->resolved_path[i] = strdup(filename_r); + files->resolved_path[i] = resolving ? strdup(filename_r) : filename; continue; } @@ -112,7 +112,7 @@ size_t _alpm_filelist_resolve_link( } /* deal with normal directories */ - files->resolved_path[i] = strdup(filename_r); + files->resolved_path[i] = resolving ? strdup(filename_r) : filename; /* deal with children of non-existent directories to reduce lstat() calls */ if (!exists) { @@ -124,7 +124,7 @@ size_t _alpm_filelist_resolve_link( strcpy(filename_r + causal_dir_r_len, f + causal_dir_len); files->resolved_path[i] = strdup(filename_r); } else { - files->resolved_path[i] = strdup(f); + files->resolved_path[i] = f; } i++; } diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index ab84329..4887e21 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -591,17 +591,24 @@ void _alpm_pkg_free(alpm_pkg_t *pkg) free_deplist(pkg->replaces); FREELIST(pkg->groups); if(pkg->files.count) { - size_t i; - for(i = 0; i < pkg->files.count; i++) { - FREE(pkg->files.files[i].name); - } - free(pkg->files.files); + size_t i, j, k; if(pkg->files.resolved_path) { - for(i = 0; i < pkg->files.count; i++) { + for(i = 0, j = 0; i < pkg->files.count; i++) { + for(k = j; k <= pkg->files.count; k++) { + if(pkg->files.resolved_path[i] == pkg->files.files[k].name) { + pkg->files.files[k].name = NULL; + j = k + 1; + break; + } + } free(pkg->files.resolved_path[i]); } free(pkg->files.resolved_path); } + for(j = 0; j < pkg->files.count; j++) { + FREE(pkg->files.files[j].name); + } + free(pkg->files.files); } alpm_list_free_inner(pkg->backup, (alpm_list_fn_free)_alpm_backup_free); alpm_list_free(pkg->backup); -- 1.7.11.3

Andrew Gregory

7 Aug 7 Aug

2:18 a.m.

New subject: [pacman-dev] [PATCH 1/2] _alpm_filelist_resolve: use original filenames where possible

If a filename isn't resolved, the original can be used instead of strdup()ing it. Signed-off-by: Andrew Gregory --- Modified to account for changes to the previous patch. lib/libalpm/filelist.c | 37 +++++++++++++++++++++---------------- lib/libalpm/package.c | 19 +++++++++++++------ 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index 783c8bf..58b1971 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -57,13 +57,13 @@ size_t _alpm_filelist_resolve_link( causal_dir = files->files[i].name; causal_dir_len = strlen(causal_dir); if(realpath(path, filename_r) == NULL) { - STRDUP(files->resolved_path[i], causal_dir, goto error); + files->resolved_path[i] = causal_dir; FREE(filename_r); return i; } causal_dir_r_len = strlen(filename_r + root_len) + 1; if(causal_dir_r_len >= PATH_MAX) { - STRDUP(files->resolved_path[i], causal_dir, goto error); + files->resolved_path[i] = causal_dir; FREE(filename_r); return i; } @@ -91,23 +91,25 @@ size_t _alpm_filelist_resolve_link( filename_r_len = filename_len + causal_dir_r_len - causal_dir_len; if(filename_r_len >= PATH_MAX) { /* resolved path is too long */ - STRDUP(files->resolved_path[i], filename, goto error); + files->resolved_path[i] = filename; continue; } strcpy(filename_r + causal_dir_r_len, filename + causal_dir_len); - } else { - filename_r = filename; } /* deal with files and paths too long to resolve*/ if(filename[filename_len - 1] != '/' || root_len + filename_r_len >= PATH_MAX) { - STRDUP(files->resolved_path[i], filename_r, goto error); + if(resolving) { + STRDUP(files->resolved_path[i], filename_r, goto error); + } else { + files->resolved_path[i] = filename; + } continue; } /* construct absolute path and stat() */ - strcpy(path + root_len, filename_r); + strcpy(path + root_len, resolving ? filename_r : filename); exists = !_alpm_lstat(path, &sbuf); /* deal with symlinks */ @@ -117,7 +119,11 @@ size_t _alpm_filelist_resolve_link( } /* deal with normal directories */ - STRDUP(files->resolved_path[i], filename_r, goto error); + if(resolving) { + STRDUP(files->resolved_path[i], filename_r, goto error); + } else { + files->resolved_path[i] = filename; + } /* deal with children of non-existent directories to reduce lstat() calls */ if (!exists) { @@ -136,25 +142,24 @@ size_t _alpm_filelist_resolve_link( strcpy(filename_r + causal_dir_r_len, f + causal_dir_len); STRDUP(files->resolved_path[i], filename_r, goto error); } else { - STRDUP(files->resolved_path[i], f, goto error); + files->resolved_path[i] = f; } } i--; } } - if(resolving) { - FREE(filename_r); - } + FREE(filename_r); return i-1; error: - if(resolving) { - FREE(filename_r); + FREE(filename_r); + /* out of memory, set remaining files to their original names */ + for(; i < files->count; (i)++) { + files->resolved_path[i] = files->files[i].name; } - /* out of memory, not much point in going on */ - return files->count; + return i-1; } /** diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index ab84329..4887e21 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -591,17 +591,24 @@ void _alpm_pkg_free(alpm_pkg_t *pkg) free_deplist(pkg->replaces); FREELIST(pkg->groups); if(pkg->files.count) { - size_t i; - for(i = 0; i < pkg->files.count; i++) { - FREE(pkg->files.files[i].name); - } - free(pkg->files.files); + size_t i, j, k; if(pkg->files.resolved_path) { - for(i = 0; i < pkg->files.count; i++) { + for(i = 0, j = 0; i < pkg->files.count; i++) { + for(k = j; k <= pkg->files.count; k++) { + if(pkg->files.resolved_path[i] == pkg->files.files[k].name) { + pkg->files.files[k].name = NULL; + j = k + 1; + break; + } + } free(pkg->files.resolved_path[i]); } free(pkg->files.resolved_path); } + for(j = 0; j < pkg->files.count; j++) { + FREE(pkg->files.files[j].name); + } + free(pkg->files.files); } alpm_list_free_inner(pkg->backup, (alpm_list_fn_free)_alpm_backup_free); alpm_list_free(pkg->backup); -- 1.7.11.4

Andrew Gregory

2:18 a.m.

New subject: [pacman-dev] [PATCH 2/2] _alpm_filelist_resolve: add useful return code

Return -1 if a path is too long to resolve or we run out of memory. Signed-off-by: Andrew Gregory --- lib/libalpm/filelist.c | 81 ++++++++++++++++++++++++++++---------------------- lib/libalpm/filelist.h | 4 +-- 2 files changed, 47 insertions(+), 38 deletions(-) diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index 58b1971..bf390ab 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -31,21 +31,25 @@ static int _alpm_filelist_strcmp(const void *s1, const void *s2) return strcmp(*(char **)s1, *(char **)s2); } +/* TODO make sure callers check the return value so we can bail on errors. + * For now we soldier on as best we can, skipping paths that are too long to + * resolve and using the original filenames on memory errors. */ /** * @brief Resolves a symlink and its children. * * @attention Pre-condition: files must be sorted! * * @param files filelist to resolve - * @param i index in files to start processing + * @param i pointer to the index in files to start processing, will point to + * the last file processed on return * @param path absolute path for the symlink being resolved * @param root_len length of the root portion of path * @param resolving is file \i in \files a symlink that needs to be resolved * - * @return the index of the last file resolved + * @return 0 on success, -1 on error */ -size_t _alpm_filelist_resolve_link( - alpm_filelist_t *files, size_t i, char *path, size_t root_len, int resolving) +int _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t *i, + char *path, size_t root_len, int resolving) { char *causal_dir; /* symlink being resolved */ char *filename_r = NULL; /* resolved filename */ @@ -54,29 +58,29 @@ size_t _alpm_filelist_resolve_link( if(resolving) { /* deal with the symlink being resolved */ MALLOC(filename_r, PATH_MAX, goto error); - causal_dir = files->files[i].name; + causal_dir = files->files[*i].name; causal_dir_len = strlen(causal_dir); if(realpath(path, filename_r) == NULL) { - files->resolved_path[i] = causal_dir; + files->resolved_path[*i] = causal_dir; FREE(filename_r); - return i; + return -1; } causal_dir_r_len = strlen(filename_r + root_len) + 1; if(causal_dir_r_len >= PATH_MAX) { - files->resolved_path[i] = causal_dir; + files->resolved_path[*i] = causal_dir; FREE(filename_r); - return i; + return -1; } /* remove root_r from filename_r */ memmove(filename_r, filename_r + root_len, causal_dir_r_len); filename_r[causal_dir_r_len - 1] = '/'; filename_r[causal_dir_r_len] = '\0'; - STRDUP(files->resolved_path[i], filename_r, goto error); - i++; + STRDUP(files->resolved_path[*i], filename_r, goto error); + (*i)++; } - for(; i < files->count; i++) { - char *filename = files->files[i].name; + for(; *i < files->count; (*i)++) { + char *filename = files->files[*i].name; size_t filename_len = strlen(filename); size_t filename_r_len = filename_len; struct stat sbuf; @@ -91,7 +95,7 @@ size_t _alpm_filelist_resolve_link( filename_r_len = filename_len + causal_dir_r_len - causal_dir_len; if(filename_r_len >= PATH_MAX) { /* resolved path is too long */ - files->resolved_path[i] = filename; + files->resolved_path[*i] = filename; continue; } @@ -101,9 +105,9 @@ size_t _alpm_filelist_resolve_link( /* deal with files and paths too long to resolve*/ if(filename[filename_len - 1] != '/' || root_len + filename_r_len >= PATH_MAX) { if(resolving) { - STRDUP(files->resolved_path[i], filename_r, goto error); + STRDUP(files->resolved_path[*i], filename_r, goto error); } else { - files->resolved_path[i] = filename; + files->resolved_path[*i] = filename; } continue; } @@ -114,21 +118,21 @@ size_t _alpm_filelist_resolve_link( /* deal with symlinks */ if(exists && S_ISLNK(sbuf.st_mode)) { - i = _alpm_filelist_resolve_link(files, i, path, root_len, 1); + _alpm_filelist_resolve_link(files, i, path, root_len, 1); continue; } /* deal with normal directories */ if(resolving) { - STRDUP(files->resolved_path[i], filename_r, goto error); + STRDUP(files->resolved_path[*i], filename_r, goto error); } else { - files->resolved_path[i] = filename; + files->resolved_path[*i] = filename; } /* deal with children of non-existent directories to reduce lstat() calls */ if (!exists) { - for(i++; i < files->count; i++) { - char *f = files->files[i].name;; + for((*i)++; *i < files->count; (*i)++) { + char *f = files->files[*i].name; size_t f_len = strlen(f); size_t f_r_len; @@ -140,26 +144,28 @@ size_t _alpm_filelist_resolve_link( f_r_len = f_len + causal_dir_r_len - causal_dir_len; if(resolving && f_r_len <= PATH_MAX) { strcpy(filename_r + causal_dir_r_len, f + causal_dir_len); - STRDUP(files->resolved_path[i], filename_r, goto error); + STRDUP(files->resolved_path[*i], filename_r, goto error); } else { - files->resolved_path[i] = f; + files->resolved_path[*i] = f; } } - i--; + (*i)--; } } + (*i)--; FREE(filename_r); - return i-1; + return 0; error: FREE(filename_r); /* out of memory, set remaining files to their original names */ - for(; i < files->count; (i)++) { - files->resolved_path[i] = files->files[i].name; + for(; *i < files->count; (*i)++) { + files->resolved_path[*i] = files->files[*i].name; } - return i-1; + (*i)--; + return -1; } /** @@ -173,35 +179,38 @@ error: * * @param handle the context handle * @param files list of files to resolve + * + * @return 0 on success, -1 on error */ -void _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files) +int _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files) { char path[PATH_MAX]; - size_t root_len; + size_t root_len, i=0; + int ret = 0; if(!files || files->resolved_path) { - return; + return 0; } - CALLOC(files->resolved_path, files->count, sizeof(char *), return); + CALLOC(files->resolved_path, files->count, sizeof(char *), return -1); /* not much point in going on if we can't even resolve root */ if(realpath(handle->root, path) == NULL){ - return; + return -1; } root_len = strlen(path) + 1; if(root_len >= PATH_MAX) { - return; + return -1; } path[root_len - 1] = '/'; path[root_len] = '\0'; - _alpm_filelist_resolve_link(files, 0, path, root_len, 0); + ret = _alpm_filelist_resolve_link(files, &i, path, root_len, 0); qsort(files->resolved_path, files->count, sizeof(char *), _alpm_filelist_strcmp); - return; + return ret; } diff --git a/lib/libalpm/filelist.h b/lib/libalpm/filelist.h index ef865da..8c4f8eb 100644 --- a/lib/libalpm/filelist.h +++ b/lib/libalpm/filelist.h @@ -21,9 +21,9 @@ #include "alpm.h" -size_t _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t i, +int _alpm_filelist_resolve_link(alpm_filelist_t *files, size_t *i, char *path, size_t root_len, int resolving); -void _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files); +int _alpm_filelist_resolve(alpm_handle_t *handle, alpm_filelist_t *files); alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA, alpm_filelist_t *filesB); -- 1.7.11.4

Allan McRae

29 Jul 29 Jul

6:18 a.m.

New subject: [pacman-dev] [PATCH 4/5] Resolve file paths during inter-package conflict check

File paths are resolved if necessary during inter-package conflict checks so that packages carrying the same effective file due to directory symlinks on the filesystem are flagged as conflicting. Signed-off-by: Allan McRae --- lib/libalpm/conflict.c | 5 +++++ lib/libalpm/filelist.c | 13 +++++-------- test/pacman/tests/fileconflict001.py | 2 -- test/pacman/tests/fileconflict016.py | 2 -- test/pacman/tests/fileconflict017.py | 26 ++++++++++++++++++++++++++ 5 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 test/pacman/tests/fileconflict017.py diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index ccfe990..afef56c 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -361,12 +361,17 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, int percent = (current * 100) / numtargs; PROGRESS(handle, ALPM_PROGRESS_CONFLICTS_START, "", percent, numtargs, current); + + _alpm_filelist_resolve(handle, alpm_pkg_get_files(p1)); + /* CHECK 1: check every target against every target */ _alpm_log(handle, ALPM_LOG_DEBUG, "searching for file conflicts: %s\n", p1->name); for(j = i->next; j; j = j->next) { alpm_list_t *common_files; alpm_pkg_t *p2 = j->data; + _alpm_filelist_resolve(handle, alpm_pkg_get_files(p2)); + common_files = _alpm_filelist_intersection(alpm_pkg_get_files(p1), alpm_pkg_get_files(p2)); diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index 288907a..59bf1ec 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -241,21 +241,18 @@ alpm_list_t *_alpm_filelist_intersection(alpm_filelist_t *filesA, int cmp, isdirA, isdirB; char *strA, *strB; - alpm_file_t *fileA = filesA->files + ctrA; - alpm_file_t *fileB = filesB->files + ctrB; - isdirA = 0; - strA = fileA->name; + strA = filesA->resolved_path[ctrA]; if(strA[strlen(strA)-1] == '/') { isdirA = 1; - strA = strndup(fileA->name, strlen(strA)-1); + strA = strndup(filesA->resolved_path[ctrA], strlen(strA)-1); } isdirB = 0; - strB = fileB->name; + strB = filesB->resolved_path[ctrB]; if(strB[strlen(strB)-1] == '/') { isdirB = 1; - strB = strndup(fileB->name, strlen(strB)-1); + strB = strndup(filesB->resolved_path[ctrB], strlen(strB)-1); } cmp = strcmp(strA, strB); @@ -273,7 +270,7 @@ alpm_list_t *_alpm_filelist_intersection(alpm_filelist_t *filesA, /* when not directories, item in both qualifies as an intersect */ if(! (isdirA && isdirB)) { - ret = alpm_list_add(ret, fileA); + ret = alpm_list_add(ret, filesA->files + ctrA); } ctrA++; ctrB++; diff --git a/test/pacman/tests/fileconflict001.py b/test/pacman/tests/fileconflict001.py index e1371f8..b1ad5e1 100644 --- a/test/pacman/tests/fileconflict001.py +++ b/test/pacman/tests/fileconflict001.py @@ -25,5 +25,3 @@ self.addrule("!PKG_EXIST=pkg2") self.addrule("FILE_EXIST=dir/realdir/realfile") self.addrule("!FILE_EXIST=dir/realdir/file") - -self.expectfailure = True diff --git a/test/pacman/tests/fileconflict016.py b/test/pacman/tests/fileconflict016.py index 5625984..c5daf48 100644 --- a/test/pacman/tests/fileconflict016.py +++ b/test/pacman/tests/fileconflict016.py @@ -22,5 +22,3 @@ self.addrule("PACMAN_RETCODE=1") self.addrule("!PKG_EXIST=pkg1") self.addrule("!PKG_EXIST=pkg2") - -self.expectfailure = True diff --git a/test/pacman/tests/fileconflict017.py b/test/pacman/tests/fileconflict017.py new file mode 100644 index 0000000..3855a93 --- /dev/null +++ b/test/pacman/tests/fileconflict017.py @@ -0,0 +1,26 @@ +self.description = "file conflict with same effective path across packages (directory symlink - deep)" + +lp1 = pmpkg("filesystem", "1.0-1") +lp1.files = ["usr/", + "usr/lib/", + "lib -> usr/lib/"] +self.addpkg2db("local", lp1) + +p1 = pmpkg("pkg1") +p1.files = ["lib/", + "lib/foo/", + "lib/foo/bar"] +self.addpkg2db("sync", p1) + +p2 = pmpkg("pkg2") +p2.files = ["usr/", + "usr/lib/", + "usr/lib/foo/", + "usr/lib/foo/bar"] +self.addpkg2db("sync", p2) + +self.args = "-S pkg1 pkg2" + +self.addrule("PACMAN_RETCODE=1") +self.addrule("!PKG_EXIST=pkg1") +self.addrule("!PKG_EXIST=pkg2") -- 1.7.11.3

Allan McRae

6:18 a.m.

New subject: [pacman-dev] [PATCH 5/5] Avoid upgrade conflict with unchanged effective path

This applies to a case such as when /lib is a symlink to /usr/lib. If a package is installed which contains /lib/libfoo.so, pacman will complain if this package is then "fixed" to contain /usr/lib/libfoo.so. Since these have the same effective path and it exists within the same package, ignore the conflict. Fixes FS#30681. Signed-off-by: Allan McRae --- lib/libalpm/conflict.c | 1 + lib/libalpm/filelist.c | 5 ++--- test/pacman/tests/fileconflict013.py | 2 -- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c index afef56c..f087ace 100644 --- a/lib/libalpm/conflict.c +++ b/lib/libalpm/conflict.c @@ -396,6 +396,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle, _alpm_log(handle, ALPM_LOG_DEBUG, "searching for filesystem conflicts: %s\n", p1->name); dbpkg = _alpm_db_get_pkgfromcache(handle->db_local, p1->name); + _alpm_filelist_resolve(handle, alpm_pkg_get_files(dbpkg)); /* Do two different checks here. If the package is currently installed, * then only check files that are new in the new package. If the package diff --git a/lib/libalpm/filelist.c b/lib/libalpm/filelist.c index 59bf1ec..9b7b8bd 100644 --- a/lib/libalpm/filelist.c +++ b/lib/libalpm/filelist.c @@ -190,9 +190,8 @@ alpm_list_t *_alpm_filelist_difference(alpm_filelist_t *filesA, while(ctrA < filesA->count && ctrB < filesB->count) { alpm_file_t *fileA = filesA->files + ctrA; - alpm_file_t *fileB = filesB->files + ctrB; - const char *strA = fileA->name; - const char *strB = fileB->name; + const char *strA = filesA->resolved_path[ctrA]; + const char *strB = filesB->resolved_path[ctrB]; /* skip directories, we don't care about them */ if(strA[strlen(strA)-1] == '/') { ctrA++; diff --git a/test/pacman/tests/fileconflict013.py b/test/pacman/tests/fileconflict013.py index 521b62e..a83923c 100644 --- a/test/pacman/tests/fileconflict013.py +++ b/test/pacman/tests/fileconflict013.py @@ -18,5 +18,3 @@ self.addrule("PACMAN_RETCODE=0") self.addrule("PKG_VERSION=pkg1|1.0-2") - -self.expectfailure = True -- 1.7.11.3

4280

Age (days ago)

4289

Last active (days ago)

List overview

Download

10 comments

3 participants

participants (3)

Allan McRae
Andrew Gregory
Dave Reisner

[pacman-dev] [PATCH 0/5] Properly detect conflicts with directory symlinks

tags

participants (3)