[pacman-dev] [PATCH] Add conflict for replacing empty directory owned
Dan McGee
dpmcgee at gmail.com
Tue Jul 10 09:28:44 EDT 2012
On Sun, Jul 8, 2012 at 7:13 AM, Allan McRae <allan at archlinux.org> wrote:
> When two packages own an empty directory, pacman finds no conflict when
> one of those packages wants to replace the directory with a file or a
> symlink. When it comes to actually extracting the new file/symlink,
> pacman sees the directory is still there (we do not remove empty
> directories if they are owned by a package) and refuses to extract.
>
> Detect this potential conflict early and bail. Note that it is a
> _potential_ conflict and not a guaranteed one as the other package owning
> the directory could be updated or removed first which would remove
> the conflict. However, pacman currently can not sort package installation
> order to ensure this, so this conflict requires manual upgrade ordering.
>
> Signed-off-by: Allan McRae <allan at archlinux.org>
> ---
Only minor comments, which I've fixed myself and pulled this onto my
working branch.
> This fixes the issues people are experiencing with the /lib/ to
> /lib -> usr/lib change in Arch Linux. People using -f to resolve the
> conflict will still end up with non-booting systems, but stupidity has
> its own rewards (for us as observers).
>
> This is probably maint worthy.
>
>
> lib/libalpm/conflict.c | 33 +++++++++++++++++++++++++++++----
> test/pacman/tests/fileconflict009.py | 20 ++++++++++++++++++++
> test/pacman/tests/fileconflict010.py | 20 ++++++++++++++++++++
> 3 files changed, 69 insertions(+), 4 deletions(-)
> create mode 100644 test/pacman/tests/fileconflict009.py
> create mode 100644 test/pacman/tests/fileconflict010.py
>
> diff --git a/lib/libalpm/conflict.c b/lib/libalpm/conflict.c
> index 3b8fce0..b52c50a 100644
> --- a/lib/libalpm/conflict.c
> +++ b/lib/libalpm/conflict.c
> @@ -330,15 +330,40 @@ const alpm_file_t *_alpm_filelist_contains(alpm_filelist_t *filelist,
> return NULL;
> }
>
> -static int dir_belongsto_pkg(const char *root, const char *dirpath,
> +static int dir_belongsto_pkg(alpm_handle_t *handle, const char *dirpath,
> alpm_pkg_t *pkg)
> {
> + alpm_list_t *local, *local_pkgs;
> + const char *root = handle->root;
I prefer if at all possible to do the assignments last in these type
of blocks. Minor nitpick though...
> struct stat sbuf;
> char path[PATH_MAX];
> char abspath[PATH_MAX];
> struct dirent *ent = NULL;
> DIR *dir;
>
> + /* TODO: this is an overly strict check but currently pacman will not
> + * overwrite a directory with a file (case 10/11 in add.c). Adjusting that
> + * is not simple as even if the directory is being unowned by a conflicting
> + * package, pacman does not sort this to ensure all required directory
> + * "removals" happen before installation of file/symlink */
> +
> + /* check no other _installed_ package owns the directory */
> + local_pkgs = _alpm_db_get_pkgcache(handle->db_local);
> + for(local = local_pkgs; local; local = local->next) {
> + alpm_pkg_t *local_pkg = local->data;
> + alpm_filelist_t *filelist;
> +
> + if(pkg == local_pkg) {
> + continue;
> + }
> +
> + filelist = alpm_pkg_get_files(local_pkg);
> + if(_alpm_filelist_contains(filelist, dirpath)) {
> + return 0;
> + }
> + }
Also a TODO- this can seemingly get rather expensive, as we are adding
another iteration of the local database and all the filelists inside a
loop where we are iterating every file of every to-be-installed
package. So we're at like O(n^123) here. (OK, not that bad, but
something to keep awareness of.)
> +
> + /* check all files in directory are owned by the package */
> snprintf(abspath, PATH_MAX, "%s%s", root, dirpath);
> dir = opendir(abspath);
> if(dir == NULL) {
> @@ -351,13 +376,13 @@ static int dir_belongsto_pkg(const char *root, const char *dirpath,
> if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) {
> continue;
> }
> - snprintf(path, PATH_MAX, "%s/%s", dirpath, name);
> + snprintf(path, PATH_MAX, "%s%s", dirpath, name);
> snprintf(abspath, PATH_MAX, "%s%s", root, path);
> if(stat(abspath, &sbuf) != 0) {
> continue;
> }
> if(S_ISDIR(sbuf.st_mode)) {
> - if(dir_belongsto_pkg(root, path, pkg)) {
> + if(dir_belongsto_pkg(handle, path, pkg)) {
> continue;
> } else {
> closedir(dir);
> @@ -537,7 +562,7 @@ alpm_list_t *_alpm_db_find_fileconflicts(alpm_handle_t *handle,
> _alpm_log(handle, ALPM_LOG_DEBUG,
> "check if all files in %s belongs to %s\n",
> dir, dbpkg->name);
> - resolved_conflict = dir_belongsto_pkg(handle->root, filestr, dbpkg);
> + resolved_conflict = dir_belongsto_pkg(handle, dir, dbpkg);
> }
> free(dir);
> }
> diff --git a/test/pacman/tests/fileconflict009.py b/test/pacman/tests/fileconflict009.py
> new file mode 100644
> index 0000000..d277d52
> --- /dev/null
> +++ b/test/pacman/tests/fileconflict009.py
> @@ -0,0 +1,20 @@
> +self.description = "dir->symlink change during package upgrade (directory conflict)"
> +
> +lp1 = pmpkg("pkg1")
> +lp1.files = ["dir/"]
> +self.addpkg2db("local", lp1)
> +
> +lp2 = pmpkg("pkg2")
> +lp2.files = ["dir/"]
> +self.addpkg2db("local", lp2)
> +
> +p = pmpkg("pkg1", "1.0-2")
> +p.files = ["dir -> /usr/dir"]
> +self.addpkg2db("sync", p)
> +
> +self.args = "-S pkg1"
> +
> +self.addrule("PACMAN_RETCODE=1")
> +self.addrule("PKG_EXIST=pkg1")
> +self.addrule("PKG_VERSION=pkg1|1.0-1")
VERSION implies EXIST, so no need to really do them both. Would make
more sense to spend the time sanity checking pkg2 as well.
> +self.addrule("DIR_EXIST=dir/")
> diff --git a/test/pacman/tests/fileconflict010.py b/test/pacman/tests/fileconflict010.py
> new file mode 100644
> index 0000000..3316a02
> --- /dev/null
> +++ b/test/pacman/tests/fileconflict010.py
> @@ -0,0 +1,20 @@
> +self.description = "dir->file change during package upgrade (directory conflict)"
> +
> +lp1 = pmpkg("pkg1")
> +lp1.files = ["dir/"]
> +self.addpkg2db("local", lp1)
> +
> +lp2 = pmpkg("pkg2")
> +lp2.files = ["dir/"]
> +self.addpkg2db("local", lp2)
> +
> +p = pmpkg("pkg1", "1.0-2")
> +p.files = ["dir"]
> +self.addpkg2db("sync", p)
> +
> +self.args = "-S pkg1"
> +
> +self.addrule("PACMAN_RETCODE=1")
> +self.addrule("PKG_EXIST=pkg1")
> +self.addrule("PKG_VERSION=pkg1|1.0-1")
> +self.addrule("DIR_EXIST=dir/")
> --
> 1.7.11.1
>
>
More information about the pacman-dev
mailing list