[pacman-dev] [PATCH 4/8] conflict.c: use real path for filesystem checks
Allan McRae
allan at archlinux.org
Sun Apr 28 10:33:12 EDT 2013
On 27/04/13 10:00, Andrew Gregory wrote:
> In the event of a symlink<->dir transition, the paths for the incoming
> package need to be resolved to match the filelist for installed
> packages.
Can you explain this to me? Every time I think I understand, I don't...
e.g.
outdated local package:
/usr/foo -> /usr/bar/
/usr/bar/baz
update sync package:
/usr/foo/baz
So /usr/foo/baz is a new file so it is tested for presence in the
filesystem. But /usr/foo/baz "exists" on the filesystem as /usr/bar/baz.
>From what I understand, you resolve the package file "/usr/foo/baz" to
"/usr/bar/baz" and note that it is in the old package so not a conflict.
Which feels wrong to me...
Can we do something like testing whether the directory of the
"conflicting" file and the resolved path of that directory are
different? If not, then we must be dealing with a case of a filesystem
symlink being replaced with a package directory (otherwise the package
directory would have conflicted with the filesystem symlink earlier).
> Note: this only enables the symlink<->dir transition itself. Any
> installed packages with invalid file lists containing symlinks are not
> detected.
>
> Signed-off-by: Andrew Gregory <andrew.gregory.8 at gmail.com>
> ---
> lib/libalpm/conflict.c | 42 ++++++++++++++++-------------------
> src/common/util-common.c | 43 ++++++++++++++++++++++++++++++++++++
> src/common/util-common.h | 1 +
> test/pacman/tests/fileconflict007.py | 2 --
> test/pacman/tests/sync701.py | 2 --
> test/pacman/tests/sync702.py | 2 --
> 6 files changed, 63 insertions(+), 29 deletions(-)
>
> diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
> index 518a2c6..08c1cbe 100644
> --- a/lib/libalpm/conflict.c
> +++ b/lib/libalpm/conflict.c
> @@ -501,7 +501,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
> /* have we acted on this conflict? */
> int resolved_conflict = 0;
> struct stat lsbuf;
> - char path[PATH_MAX];
> + char path[PATH_MAX], resolved_path[PATH_MAX];
> size_t pathlen;
>
> pathlen = snprintf(path, PATH_MAX, "%s%s", handle->root, filestr);
> @@ -513,7 +513,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
>
> _alpm_log(handle, ALPM_LOG_DEBUG, "checking possible conflict: %s\n", path);
>
> - if(filestr[strlen(filestr) - 1] == '/') {
> + if(path[pathlen - 1] == '/') {
> if(S_ISDIR(lsbuf.st_mode)) {
> _alpm_log(handle, ALPM_LOG_DEBUG, "file is a directory, not a conflict\n");
> continue;
> @@ -524,7 +524,19 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
> path[pathlen - 1] = '\0';
> }
>
> - relative_path = path + rootlen;
> + if(lrealpath(path, resolved_path)
> + && strncmp(resolved_path, handle->root, rootlen) == 0) {
> + relative_path = resolved_path + rootlen;
> + } else {
> + relative_path = path + rootlen;
> + }
> +
> + if(!resolved_conflict && dbpkg
> + && alpm_filelist_contains(alpm_pkg_get_files(dbpkg), relative_path)) {
> + _alpm_log(handle, ALPM_LOG_DEBUG,
> + "package contained the resolved realpath\n");
> + resolved_conflict = 1;
> + }
>
> /* Check remove list (will we remove the conflicting local file?) */
> for(k = rem; k && !resolved_conflict; k = k->next) {
> @@ -546,12 +558,12 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
> alpm_pkg_t *localp2 = _alpm_db_get_pkgfromcache(handle->db_local, p2->name);
>
> /* localp2->files will be removed (target conflicts are handled by CHECK 1) */
> - if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), filestr)) {
> + if(localp2 && alpm_filelist_contains(alpm_pkg_get_files(localp2), relative_path)) {
> /* skip removal of file, but not add. this will prevent a second
> * package from removing the file when it was already installed
> * by its new owner (whether the file is in backup array or not */
> handle->trans->skip_remove =
> - alpm_list_add(handle->trans->skip_remove, strdup(filestr));
> + alpm_list_add(handle->trans->skip_remove, strdup(relative_path));
> _alpm_log(handle, ALPM_LOG_DEBUG,
> "file changed packages, adding to remove skiplist\n");
> resolved_conflict = 1;
> @@ -560,8 +572,8 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
>
> /* check if all files of the dir belong to the installed pkg */
> if(!resolved_conflict && S_ISDIR(lsbuf.st_mode) && dbpkg) {
> - char *dir = malloc(strlen(filestr) + 2);
> - sprintf(dir, "%s/", filestr);
> + char *dir = malloc(strlen(relative_path) + 2);
> + sprintf(dir, "%s/", relative_path);
> if(alpm_filelist_contains(alpm_pkg_get_files(dbpkg), dir)) {
> _alpm_log(handle, ALPM_LOG_DEBUG,
> "checking if all files in %s belong to %s\n",
> @@ -571,22 +583,6 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
> free(dir);
> }
>
> - /* check if a component of the filepath was a link. canonicalize the path
> - * and look for it in the old package. note that the actual file under
> - * consideration cannot itself be a link, as it might be unowned- path
> - * components can be safely checked as all directories are "unowned". */
> - if(!resolved_conflict && dbpkg && !S_ISLNK(lsbuf.st_mode)) {
> - char rpath[PATH_MAX];
> - if(realpath(path, rpath)) {
> - const char *relative_rpath = rpath + rootlen;
> - if(alpm_filelist_contains(alpm_pkg_get_files(dbpkg), relative_rpath)) {
> - _alpm_log(handle, ALPM_LOG_DEBUG,
> - "package contained the resolved realpath\n");
> - resolved_conflict = 1;
> - }
> - }
> - }
> -
> /* is the file unowned and in the backup list of the new package? */
> if(!resolved_conflict && _alpm_needbackup(filestr, p1)) {
> alpm_list_t *local_pkgs = _alpm_db_get_pkgcache(handle->db_local);
> diff --git a/src/common/util-common.c b/src/common/util-common.c
> index 7145e49..c2422e0 100644
> --- a/src/common/util-common.c
> +++ b/src/common/util-common.c
> @@ -73,6 +73,49 @@ char *mdirname(const char *path)
> return strdup(".");
> }
>
> +/** Resolve the canonicalized absolute path of a symlink.
> + * @param path path to resolve
> + * @param resolved_path destination for the resolved path, will be malloc'd if
> + * NULL
> + * @return the resolved path
> + */
> +char *lrealpath(const char *path, char *resolved_path)
> +{
> + const char *bname = mbasename(path);
> + char *rpath = NULL, *dname = NULL;
> + int success = 0;
> +
> + if(strcmp(path, ".") == 0 || strcmp(path, "..") == 0) {
> + /* the entire path needs to be resolved */
> + return realpath(path, resolved_path);
> + }
> +
> + if(!(dname = mdirname(path))) {
> + goto cleanup;
> + }
> + if(!(rpath = realpath(dname, NULL))) {
> + goto cleanup;
> + }
> + if(!resolved_path) {
> + if(!(resolved_path = malloc(strlen(rpath) + strlen(bname) + 2))) {
> + goto cleanup;
> + }
> + }
> +
> + strcpy(resolved_path, rpath);
> + if(resolved_path[strlen(resolved_path) - 1] != '/') {
> + strcat(resolved_path, "/");
> + }
> + strcat(resolved_path, bname);
> + success = 1;
> +
> +cleanup:
> + free(dname);
> + free(rpath);
> +
> + return (success ? resolved_path : NULL);
> +}
> +
> #ifndef HAVE_STRNDUP
> /* A quick and dirty implementation derived from glibc */
> /** Determines the length of a fixed-size string.
> diff --git a/src/common/util-common.h b/src/common/util-common.h
> index 04d4e9d..b3a4df1 100644
> --- a/src/common/util-common.h
> +++ b/src/common/util-common.h
> @@ -22,6 +22,7 @@
>
> const char *mbasename(const char *path);
> char *mdirname(const char *path);
> +char *lrealpath(const char *path, char *resolved_path);
>
> #ifndef HAVE_STRNDUP
> char *strndup(const char *s, size_t n);
> diff --git a/test/pacman/tests/fileconflict007.py b/test/pacman/tests/fileconflict007.py
> index b61ddb4..7fe65ed 100644
> --- a/test/pacman/tests/fileconflict007.py
> +++ b/test/pacman/tests/fileconflict007.py
> @@ -16,5 +16,3 @@
> self.addrule("PKG_EXIST=pkg")
> self.addrule("PKG_VERSION=pkg|1.0-2")
> self.addrule("FILE_TYPE=dir/symdir/|dir")
> -
> -self.expectfailure = True
> diff --git a/test/pacman/tests/sync701.py b/test/pacman/tests/sync701.py
> index 912c794..590845f 100644
> --- a/test/pacman/tests/sync701.py
> +++ b/test/pacman/tests/sync701.py
> @@ -19,5 +19,3 @@
> self.addrule("PKG_VERSION=pkg1|1.0-2")
> self.addrule("PKG_EXIST=pkg2")
> self.addrule("FILE_TYPE=lib|dir")
> -
> -self.expectfailure = True
> diff --git a/test/pacman/tests/sync702.py b/test/pacman/tests/sync702.py
> index 8f4c0ad..c3e2320 100644
> --- a/test/pacman/tests/sync702.py
> +++ b/test/pacman/tests/sync702.py
> @@ -19,5 +19,3 @@
> self.addrule("PKG_VERSION=pkg2|1.0-2")
> self.addrule("PKG_EXIST=pkg1")
> self.addrule("FILE_TYPE=lib|dir")
> -
> -self.expectfailure = True
>
More information about the pacman-dev
mailing list