FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 07-08-2012, 12:13 PM
Allan McRae
 
Default Add conflict for replacing empty directory owned

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@archlinux.org>
---

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;
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;
+ }
+ }
+
+ /* 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
",
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")
+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
 
Old 07-10-2012, 01:28 PM
Dan McGee
 
Default Add conflict for replacing empty directory owned

On Sun, Jul 8, 2012 at 7:13 AM, Allan McRae <allan@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@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
",
> 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
>
>
 

Thread Tools




All times are GMT. The time now is 09:18 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org