[pacman-dev] [PATCH 1/3] pactest: build the filelist using a set()
This will prevent duplicates, which we had plenty of once I made a few tests that had a list of files greater than the normal two. The previous logic was not working quite right. Signed-off-by: Dan McGee <dan@archlinux.org> --- test/pacman/pmdb.py | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py index 60460ce..b4d0e2d 100755 --- a/test/pacman/pmdb.py +++ b/test/pacman/pmdb.py @@ -38,16 +38,14 @@ def _mkfilelist(files): usr/local/bin/ usr/local/bin/dummy """ - i = [] + file_list = set() for f in files: dir = getfilename(f) - i.append(dir) + file_list.add(dir) while "/" in dir: [dir, tmp] = dir.rsplit("/", 1) - if not dir + "/" in files: - i.append(dir + "/") - i.sort() - return i + file_list.add(dir + "/") + return sorted(file_list) def _mkbackuplist(backup): """ -- 1.7.3.5
These are probably useful anyway, but also exposed the double file list bug that will be fixed in a later commit. Signed-off-by: Dan McGee <dan@archlinux.org> --- test/pacman/tests/remove002.py | 12 ++++++++++++ test/pacman/tests/upgrade006.py | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 0 deletions(-) create mode 100644 test/pacman/tests/remove002.py create mode 100644 test/pacman/tests/upgrade006.py diff --git a/test/pacman/tests/remove002.py b/test/pacman/tests/remove002.py new file mode 100644 index 0000000..1deffb0 --- /dev/null +++ b/test/pacman/tests/remove002.py @@ -0,0 +1,12 @@ +self.description = "Remove a package with several files" + +p = pmpkg("foo") +p.files = ["usr/share/file_%d" % n for n in range(1000)] +self.addpkg2db("local", p) + +self.args = "-R %s" % p.name + +self.addrule("PACMAN_RETCODE=0") +self.addrule("!PKG_EXIST=foo") +self.addrule("!FILE_EXIST=usr/share/file_0") +self.addrule("!FILE_EXIST=usr/share/file_999") diff --git a/test/pacman/tests/upgrade006.py b/test/pacman/tests/upgrade006.py new file mode 100644 index 0000000..5e5173b --- /dev/null +++ b/test/pacman/tests/upgrade006.py @@ -0,0 +1,18 @@ +self.description = "Upgrade a package with several files" + +lp = pmpkg("dummy") +lp.files = ["usr/share/file_%d" % n for n in range(250, 750)] +self.addpkg2db("local", lp) + +p = pmpkg("dummy", "1.1-1") +p.files = ["usr/share/file_%d" % n for n in range(600, 1000)] +self.addpkg(p) + +self.args = "-U %s" % p.filename() + +self.addrule("PACMAN_RETCODE=0") +self.addrule("PKG_VERSION=dummy|1.1-1") +self.addrule("!FILE_EXIST=usr/share/file_250") +self.addrule("!FILE_EXIST=usr/share/file_599") +self.addrule("FILE_EXIST=usr/share/file_600") +self.addrule("FILE_EXIST=usr/share/file_999") -- 1.7.3.5
Due to the way we funk around with package data loading, we had a condition where the filelist got doubled up because it was loaded twice. Packages are originally loaded with INFRQ_BASE. In a upgrade/sync, the package is checked for file conflicts next, leaving us in INFRQ_BASE | INFRQ_FILES state. Later, when committing a single package, we have an explicit call to pkg_load() with INFRQ_ALL as the level. Because the packages' level did not match this, we skipped over our previous "does the incoming level match where I'm at" shortcut, and continued to load things again, because of a lack of fine-grained checking for each of DESC, FILES, and INSTALL. Fix the problem by doing a bit more bitmasking logic throughout the load method, and also fix the sanity check at the beginning of the function- this should *only* be used for local packages as opposed to the "not a package" check that was there before. A debug log message was added to upgraderemove as well to match the one already in the normal remove codepath. Signed-off-by: Dan McGee <dan@archlinux.org> --- lib/libalpm/be_local.c | 22 ++++++++++++---------- lib/libalpm/remove.c | 18 +++++++++++++----- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index d0662d9..5471fee 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -424,6 +424,10 @@ static int local_db_populate(pmdb_t *db) continue; } + pkg->origin = PKG_FROM_LOCALDB; + pkg->origin_data.db = db; + pkg->ops = &local_pkg_ops; + /* explicitly read with only 'BASE' data, accessors will handle the rest */ if(_alpm_local_db_read(db, pkg, INFRQ_BASE) == -1) { _alpm_log(PM_LOG_ERROR, _("corrupted database entry '%s'\n"), name); @@ -431,10 +435,6 @@ static int local_db_populate(pmdb_t *db) continue; } - pkg->origin = PKG_FROM_LOCALDB; - pkg->ops = &local_pkg_ops; - - pkg->origin_data.db = db; /* add to the collection */ _alpm_log(PM_LOG_FUNCTION, "adding '%s' to package cache for db '%s'\n", pkg->name, db->treename); @@ -480,8 +480,10 @@ int _alpm_local_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) return(-1); } - if(info->origin == PKG_FROM_FILE) { - _alpm_log(PM_LOG_DEBUG, "request to read database info for a file-based package '%s', skipping...\n", info->name); + if(info->origin != PKG_FROM_LOCALDB) { + _alpm_log(PM_LOG_DEBUG, + "request to read info for a non-local package '%s', skipping...\n", + info->name); return(-1); } @@ -491,7 +493,7 @@ int _alpm_local_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) * & result: 00000100 * == to inforeq? nope, we need to load more info. */ if((info->infolevel & inforeq) == inforeq) { - /* already loaded this info, do nothing */ + /* already loaded all of this info, do nothing */ return(0); } _alpm_log(PM_LOG_FUNCTION, "loading package data for %s : level=0x%x\n", @@ -510,7 +512,7 @@ int _alpm_local_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } /* DESC */ - if(inforeq & INFRQ_DESC) { + if(inforeq & INFRQ_DESC && !(info->infolevel & INFRQ_DESC)) { snprintf(path, PATH_MAX, "%sdesc", pkgpath); if((fp = fopen(path, "r")) == NULL) { _alpm_log(PM_LOG_ERROR, _("could not open file %s: %s\n"), path, strerror(errno)); @@ -639,7 +641,7 @@ int _alpm_local_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } /* FILES */ - if(inforeq & INFRQ_FILES) { + if(inforeq & INFRQ_FILES && !(info->infolevel & INFRQ_FILES)) { snprintf(path, PATH_MAX, "%sfiles", pkgpath); if((fp = fopen(path, "r")) == NULL) { _alpm_log(PM_LOG_ERROR, _("could not open file %s: %s\n"), path, strerror(errno)); @@ -666,7 +668,7 @@ int _alpm_local_db_read(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) } /* INSTALL */ - if(inforeq & INFRQ_SCRIPTLET) { + if(inforeq & INFRQ_SCRIPTLET && !(info->infolevel & INFRQ_SCRIPTLET)) { snprintf(path, PATH_MAX, "%sinstall", pkgpath); if(access(path, F_OK) == 0) { info->scriptlet = 1; diff --git a/lib/libalpm/remove.c b/lib/libalpm/remove.c index d4e3b94..5fba0b0 100644 --- a/lib/libalpm/remove.c +++ b/lib/libalpm/remove.c @@ -298,10 +298,12 @@ static void unlink_file(pmpkg_t *info, char *filename, alpm_list_t *skip_remove, } } -int _alpm_upgraderemove_package(pmpkg_t *oldpkg, pmpkg_t *newpkg, pmtrans_t *trans) +int _alpm_upgraderemove_package(pmpkg_t *oldpkg, pmpkg_t *newpkg, + pmtrans_t *trans) { alpm_list_t *skip_remove, *b; alpm_list_t *newfiles, *lp; + size_t filenum; alpm_list_t *files = alpm_pkg_get_files(oldpkg); const char *pkgname = alpm_pkg_get_name(oldpkg); @@ -315,8 +317,9 @@ int _alpm_upgraderemove_package(pmpkg_t *oldpkg, pmpkg_t *newpkg, pmtrans_t *tra } /* copy the remove skiplist over */ - skip_remove = - alpm_list_join(alpm_list_strdup(trans->skip_remove),alpm_list_strdup(handle->noupgrade)); + skip_remove = alpm_list_join( + alpm_list_strdup(trans->skip_remove), + alpm_list_strdup(handle->noupgrade)); /* Add files in the NEW backup array to the skip_remove array * so this removal operation doesn't kill them */ /* old package backup list */ @@ -340,6 +343,9 @@ int _alpm_upgraderemove_package(pmpkg_t *oldpkg, pmpkg_t *newpkg, pmtrans_t *tra } } + filenum = alpm_list_count(files); + _alpm_log(PM_LOG_DEBUG, "removing %ld files\n", (unsigned long)filenum); + /* iterate through the list backwards, unlinking files */ newfiles = alpm_list_reverse(files); for(lp = newfiles; lp; lp = alpm_list_next(lp)) { @@ -406,6 +412,9 @@ int _alpm_remove_packages(pmtrans_t *trans, pmdb_t *db) if(!(trans->flags & PM_TRANS_FLAG_DBONLY)) { alpm_list_t *files = alpm_pkg_get_files(info); + alpm_list_t *newfiles; + size_t filenum; + for(lp = files; lp; lp = lp->next) { if(!can_remove_file(lp->data, NULL)) { _alpm_log(PM_LOG_DEBUG, "not removing package '%s', can't remove all files\n", @@ -414,8 +423,7 @@ int _alpm_remove_packages(pmtrans_t *trans, pmdb_t *db) } } - size_t filenum = alpm_list_count(files); - alpm_list_t *newfiles; + filenum = alpm_list_count(files); _alpm_log(PM_LOG_DEBUG, "removing %ld files\n", (unsigned long)filenum); /* init progress bar */ -- 1.7.3.5
participants (1)
-
Dan McGee