[pacman-dev] [PATCH 0/5] Version the local database
With removing directory symlink support, we need to make an adjustment to the users local database to adjust any path that relied in that "feature". As this is not a readily detectable change, as was the case in our last local database adjustment, we need to add something to track the local database version. The file ".alpm_db_version" is added to the local database root. This file is automatically created whenever the local database directory is absent or empty. This store the current database version as an integer. I have chosen to use the (next) libalpm soname version so it is easy for us to track what versions are needed. Allan McRae (5): Create local database directory if it is missing Add version file when creating local database directory Add version file to empty local database Check the version of the local database during validation Extend database upgrade script to handle alpm db version 9 lib/libalpm/be_local.c | 118 +++++++++++++++++++++++++++------------- scripts/pacman-db-upgrade.sh.in | 46 ++++++++++++---- test/pacman/pmdb.py | 2 + test/pacman/pmtest.py | 3 + 4 files changed, 121 insertions(+), 48 deletions(-) -- 1.8.3.3
This means that a missing local database becomes an error (as it should be immediately created). Note this only creates the "local" directory and not its parent, which is check for during locking. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/be_local.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 49b1c74..c7f590a 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -366,6 +366,16 @@ static int is_dir(const char *path, struct dirent *entry) return 0; } +static int local_db_create(alpm_db_t *db, const char *dbpath) +{ + if(mkdir(dbpath, 0755) != 0) { + _alpm_log(db->handle, ALPM_LOG_ERROR, _("could not create directory %s: %s\n"), + dbpath, strerror(errno)); + RET_ERR(db->handle, ALPM_ERR_DB_CREATE, -1); + } + return 0; +} + static int local_db_validate(alpm_db_t *db) { struct dirent *ent = NULL; @@ -387,12 +397,19 @@ static int local_db_validate(alpm_db_t *db) dbdir = opendir(dbpath); if(dbdir == NULL) { if(errno == ENOENT) { - /* database dir doesn't exist yet */ - db->status |= DB_STATUS_VALID; - db->status &= ~DB_STATUS_INVALID; - db->status &= ~DB_STATUS_EXISTS; - db->status |= DB_STATUS_MISSING; - return 0; + /* database dir doesn't exist yet - create it*/ + if(!local_db_create(db, dbpath)) { + db->status |= DB_STATUS_VALID; + db->status &= ~DB_STATUS_INVALID; + db->status |= DB_STATUS_EXISTS; + db->status &= ~DB_STATUS_MISSING; + return 0; + } else { + db->status &= DB_STATUS_EXISTS; + db->status |= ~DB_STATUS_MISSING; + /* pm_errno is set by local_db_create */ + return -1; + } } else { RET_ERR(db->handle, ALPM_ERR_DB_OPEN, -1); } @@ -445,7 +462,9 @@ static int local_db_populate(alpm_db_t *db) if(db->status & DB_STATUS_INVALID) { RET_ERR(db->handle, ALPM_ERR_DB_INVALID, -1); } - /* note: DB_STATUS_MISSING is not fatal for local database */ + if(db->status & DB_STATUS_MISSING) { + RET_ERR(db->handle, ALPM_ERR_DB_NOT_FOUND, -1); + } dbpath = _alpm_db_path(db); if(dbpath == NULL) { @@ -455,12 +474,6 @@ static int local_db_populate(alpm_db_t *db) dbdir = opendir(dbpath); if(dbdir == NULL) { - if(errno == ENOENT) { - /* no database existing yet is not an error */ - db->status &= ~DB_STATUS_EXISTS; - db->status |= DB_STATUS_MISSING; - return 0; - } RET_ERR(db->handle, ALPM_ERR_DB_OPEN, -1); } if(fstat(dirfd(dbdir), &buf) != 0) { -- 1.8.3.3
The version of the local pacman database is stored in its root in the file .alpm_db_version. The version is starting at 9, corresponding to the next libalpm library version. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/be_local.c | 18 ++++++++++++++++++ test/pacman/pmdb.py | 2 ++ 2 files changed, 20 insertions(+) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index c7f590a..eb462df 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -44,6 +44,9 @@ #include "deps.h" #include "filelist.h" +/* local database format version */ +size_t ALPM_LOCAL_DB_VERSION = 9; + static int local_db_read(alpm_pkg_t *info, alpm_dbinfrq_t inforeq); #define LAZY_LOAD(info, errret) \ @@ -366,6 +369,20 @@ static int is_dir(const char *path, struct dirent *entry) return 0; } +static int local_db_add_version(alpm_db_t UNUSED *db, const char *dbpath) +{ + char dbverpath[PATH_MAX]; + FILE *dbverfile; + + snprintf(dbverpath, PATH_MAX, "%s.alpm_db_version", dbpath); + + dbverfile = fopen(dbverpath, "w"); + fprintf(dbverfile, "%zu\n", ALPM_LOCAL_DB_VERSION); + fclose(dbverfile); + + return 0; +} + static int local_db_create(alpm_db_t *db, const char *dbpath) { if(mkdir(dbpath, 0755) != 0) { @@ -373,6 +390,7 @@ static int local_db_create(alpm_db_t *db, const char *dbpath) dbpath, strerror(errno)); RET_ERR(db->handle, ALPM_ERR_DB_CREATE, -1); } + local_db_add_version(db, dbpath); return 0; } diff --git a/test/pacman/pmdb.py b/test/pacman/pmdb.py index 3e9d305..425bf0e 100644 --- a/test/pacman/pmdb.py +++ b/test/pacman/pmdb.py @@ -88,6 +88,8 @@ def db_read(self, name): if self.read_dircache is None: self.read_dircache = os.listdir(self.dbdir) for entry in self.read_dircache: + if entry == ".alpm_db_version": + continue [pkgname, pkgver, pkgrel] = entry.rsplit("-", 2) if pkgname == name: dbentry = entry -- 1.8.3.3
If a user manually creates the local database directory, or has an empty local database for some other reason, we silently add a version file Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/be_local.c | 50 +++++++++++++++++++++++++------------------------- test/pacman/pmtest.py | 3 +++ 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index eb462df..fe9e05a 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -398,8 +398,9 @@ static int local_db_validate(alpm_db_t *db) { struct dirent *ent = NULL; const char *dbpath; + char dbverpath[PATH_MAX]; + FILE *dbverfile; DIR *dbdir; - int ret = -1; if(db->status & DB_STATUS_VALID) { return 0; @@ -435,37 +436,36 @@ static int local_db_validate(alpm_db_t *db) db->status |= DB_STATUS_EXISTS; db->status &= ~DB_STATUS_MISSING; - while((ent = readdir(dbdir)) != NULL) { - const char *name = ent->d_name; - char path[PATH_MAX]; + snprintf(dbverpath, PATH_MAX, "%s.alpm_db_version", dbpath); - if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) { - continue; - } - if(!is_dir(dbpath, ent)) { - continue; + if((dbverfile = fopen(dbverpath, "r")) == NULL) { + /* create dbverfile if local database is empty - otherwise version error */ + while((ent = readdir(dbdir)) != NULL) { + const char *name = ent->d_name; + if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) { + continue; + } else { + closedir(dbdir); + goto version_error; + } } - snprintf(path, PATH_MAX, "%s%s/depends", dbpath, name); - if(access(path, F_OK) == 0) { - /* we found a depends file- bail */ - db->status &= ~DB_STATUS_VALID; - db->status |= DB_STATUS_INVALID; - db->handle->pm_errno = ALPM_ERR_DB_VERSION; - goto done; - } - } - /* we found no depends file after full scan */ - db->status |= DB_STATUS_VALID; - db->status &= ~DB_STATUS_INVALID; - ret = 0; + local_db_add_version(db, dbpath); -done: - if(dbdir) { closedir(dbdir); + goto version_latest; } - return ret; +version_latest: + db->status |= DB_STATUS_VALID; + db->status &= ~DB_STATUS_INVALID; + return 0; + +version_error: + db->status &= ~DB_STATUS_VALID; + db->status |= DB_STATUS_INVALID; + db->handle->pm_errno = ALPM_ERR_DB_VERSION; + return -1; } static int local_db_populate(alpm_db_t *db) diff --git a/test/pacman/pmtest.py b/test/pacman/pmtest.py index f5a9680..307a752 100644 --- a/test/pacman/pmtest.py +++ b/test/pacman/pmtest.py @@ -180,6 +180,9 @@ def generate(self, pacman): for pkg in self.db["local"].pkgs: vprint("\tinstalling %s" % pkg.fullname()) pkg.install_package(self.root) + if self.db["local"].pkgs: + path = os.path.join(self.root, util.PM_DBPATH, "local") + util.mkfile(path, ".alpm_db_version", "9") # Done. vprint(" Taking a snapshot of the file system") -- 1.8.3.3
Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/be_local.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index fe9e05a..3d6b8d6 100644 --- a/lib/libalpm/be_local.c +++ b/lib/libalpm/be_local.c @@ -398,9 +398,11 @@ static int local_db_validate(alpm_db_t *db) { struct dirent *ent = NULL; const char *dbpath; + DIR *dbdir; char dbverpath[PATH_MAX]; FILE *dbverfile; - DIR *dbdir; + int t; + size_t version; if(db->status & DB_STATUS_VALID) { return 0; @@ -456,6 +458,15 @@ static int local_db_validate(alpm_db_t *db) goto version_latest; } + t = fscanf(dbverfile, "%zu", &version); + (void)t; + + fclose(dbverfile); + + if(version != ALPM_LOCAL_DB_VERSION) { + goto version_error; + } + version_latest: db->status |= DB_STATUS_VALID; db->status &= ~DB_STATUS_INVALID; -- 1.8.3.3
Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/pacman-db-upgrade.sh.in | 46 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/scripts/pacman-db-upgrade.sh.in b/scripts/pacman-db-upgrade.sh.in index a1630c5..d2cd78d 100644 --- a/scripts/pacman-db-upgrade.sh.in +++ b/scripts/pacman-db-upgrade.sh.in @@ -109,18 +109,44 @@ fi # do not let pacman run while we do this touch "$lockfile" -# pacman-3.4 to 3.5 upgrade - merge depends into desc -if [[ $(find "$dbroot"/local -name depends) ]]; then - msg "$(gettext "Pre-3.5 database format detected - upgrading...")" - for i in "$dbroot"/local/*; do - if [[ -f "$i"/depends ]]; then - cat "$i"/depends >> "$i"/desc - rm "$i"/depends - fi - done - msg "$(gettext "Done.")" +if [[ -f "${dbroot}"/local/.alpm_db_version ]]; then + db_version=($(cat "${dbroot}"/local/.alpm_db_version)) fi +if [[ -z "$db_version" ]]; then + # pacman-3.4 to 3.5 upgrade - merge depends into desc + if [[ $(find "$dbroot"/local -name depends) ]]; then + msg "$(gettext "Pre-3.5 database format detected - upgrading...")" + for i in "$dbroot"/local/*; do + if [[ -f "$i"/depends ]]; then + cat "$i"/depends >> "$i"/desc + rm "$i"/depends + fi + done + msg "$(gettext "Done.")" + fi + + # pacman 4.1 to 4.2 upgrade - remove directory symlink support + dirlist=() + + unset GREP_OPTIONS + while IFS= read -r dir; do + dirlist+=("/${dir%/}") + done < <(grep -h '/$' "$dbroot"/local/*/files | sort -u) + + mapfile -t dirlist < <(find "${dirlist[@]}" -maxdepth 0 -type l) + + if [[ ${#dirlist[@]} != 0 ]]; then + msg "$(gettext "Pre-4.2 database format detected - upgrading...")" + for dir in ${dirlist[@]}; do + realdir=( $(cd $dir; pwd -P) ) + sed -i "s#^${dir#/}/#${realdir#/}/#" "$dbroot"/local/*/files + done + fi +fi + +echo "9" > "$dbroot"/local/.alpm_db_version + # remove the lock file rm -f "$lockfile" -- 1.8.3.3
On Tue, Jul 16, 2013 at 8:22 PM, Allan McRae <allan@archlinux.org> wrote:
Signed-off-by: Allan McRae <allan@archlinux.org> --- scripts/pacman-db-upgrade.sh.in | 46 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/scripts/pacman-db-upgrade.sh.in b/scripts/pacman-db-upgrade.sh.in index a1630c5..d2cd78d 100644 --- a/scripts/pacman-db-upgrade.sh.in +++ b/scripts/pacman-db-upgrade.sh.in @@ -109,18 +109,44 @@ fi # do not let pacman run while we do this touch "$lockfile"
-# pacman-3.4 to 3.5 upgrade - merge depends into desc -if [[ $(find "$dbroot"/local -name depends) ]]; then - msg "$(gettext "Pre-3.5 database format detected - upgrading...")" - for i in "$dbroot"/local/*; do - if [[ -f "$i"/depends ]]; then - cat "$i"/depends >> "$i"/desc - rm "$i"/depends - fi - done - msg "$(gettext "Done.")" +if [[ -f "${dbroot}"/local/.alpm_db_version ]]; then + db_version=($(cat "${dbroot}"/local/.alpm_db_version))
Why is db_version an array?
fi
+if [[ -z "$db_version" ]]; then + # pacman-3.4 to 3.5 upgrade - merge depends into desc + if [[ $(find "$dbroot"/local -name depends) ]]; then + msg "$(gettext "Pre-3.5 database format detected - upgrading...")" + for i in "$dbroot"/local/*; do + if [[ -f "$i"/depends ]]; then + cat "$i"/depends >> "$i"/desc + rm "$i"/depends + fi + done + msg "$(gettext "Done.")" + fi + + # pacman 4.1 to 4.2 upgrade - remove directory symlink support + dirlist=() + + unset GREP_OPTIONS + while IFS= read -r dir; do + dirlist+=("/${dir%/}") + done < <(grep -h '/$' "$dbroot"/local/*/files | sort -u) + + mapfile -t dirlist < <(find "${dirlist[@]}" -maxdepth 0 -type l) + + if [[ ${#dirlist[@]} != 0 ]]; then + msg "$(gettext "Pre-4.2 database format detected - upgrading...")" + for dir in ${dirlist[@]}; do
missing quote, "${dirlist[@]}"
+ realdir=( $(cd $dir; pwd -P) )
missing quote, "$dir"; why is realdir an array?
+ sed -i "s#^${dir#/}/#${realdir#/}/#" "$dbroot"/local/*/files
If it's almost safe to assume paths don't include '\n', it's far less so to assume they don't include '#'. Also $dir is treated as a pattern, not a fixed string here. This should be better: awk -v "dir=${dir#/}/" -v "realdir=${realdir#/}/" ' BEGIN { i = length(dir) + 1 } { if (index($0, dir) == 1) { printf("%s%s\n", realdir, substr($0, i)) } else { print } }' awk doesn't have the convenience of in place file editting though, so we may have to wrap this inside a loop, and store the updated data in a temp file (or just a var?)...
+ done + fi +fi + +echo "9" > "$dbroot"/local/.alpm_db_version + # remove the lock file rm -f "$lockfile"
-- 1.8.3.3
On 08/08/13 04:42, lolilolicon wrote:
+ # pacman 4.1 to 4.2 upgrade - remove directory symlink support
+ dirlist=() + + unset GREP_OPTIONS + while IFS= read -r dir; do + dirlist+=("/${dir%/}") + done < <(grep -h '/$' "$dbroot"/local/*/files | sort -u) + + mapfile -t dirlist < <(find "${dirlist[@]}" -maxdepth 0 -type l) + + if [[ ${#dirlist[@]} != 0 ]]; then + msg "$(gettext "Pre-4.2 database format detected - upgrading...")" + for dir in ${dirlist[@]}; do missing quote, "${dirlist[@]}"
+ realdir=( $(cd $dir; pwd -P) ) missing quote, "$dir"; why is realdir an array?
+ sed -i "s#^${dir#/}/#${realdir#/}/#" "$dbroot"/local/*/files If it's almost safe to assume paths don't include '\n', it's far less so to assume they don't include '#'. Also $dir is treated as a pattern, not a fixed string here. This should be better:
awk -v "dir=${dir#/}/" -v "realdir=${realdir#/}/" ' BEGIN { i = length(dir) + 1 } { if (index($0, dir) == 1) { printf("%s%s\n", realdir, substr($0, i)) } else { print } }'
awk doesn't have the convenience of in place file editting though, so we may have to wrap this inside a loop, and store the updated data in a temp file (or just a var?)...
@Dave: comments here? I believe I got the sed from you - although I am sure it was supposed to be a quick and dirty hack... Allan
On 16/07/13 22:22, Allan McRae wrote:
With removing directory symlink support, we need to make an adjustment to the users local database to adjust any path that relied in that "feature".
As this is not a readily detectable change, as was the case in our last local database adjustment, we need to add something to track the local database version.
The file ".alpm_db_version" is added to the local database root. This file is automatically created whenever the local database directory is absent or empty. This store the current database version as an integer. I have chosen to use the (next) libalpm soname version so it is easy for us to track what versions are needed.
Any comments on these? I had one comment: <toofishes> pacman -Qip should work without a local DB, for instance Lets have a look at pacman-4.1.2: # rm -rf /var/lib/pacman/ # pacman -Qip /tmp/pacman-git-4.1.2_67_g7977aab-1-x86_64.pkg.tar.xz error: failed to initialize alpm library (could not find or read directory) So, we need /var/lib/pacman to initialise the transaction already, so having pacman automatically add the /var/lib/pacman/local directory and a version file is not much of a burden. That means the only change is when you have a local database and it is not the correct version: # pacman -Qip pacman-git-4.1.2_77_g5ee4a65-1-x86_64.pkg.tar.xz error: failed to initialize alpm library (database is incorrect version) error: try running pacman-db-upgrade (the spacing in that output is interesting...) That requires a one-off database upgrade, which will be needed for any pacman operation. Given it blocks ALL pacman operations (which I am fairly sure was the case when we remove "depends" files too), I think this is fine. Allan
On 20/07/13 10:51, Allan McRae wrote:
On 16/07/13 22:22, Allan McRae wrote:
With removing directory symlink support, we need to make an adjustment to the users local database to adjust any path that relied in that "feature".
As this is not a readily detectable change, as was the case in our last local database adjustment, we need to add something to track the local database version.
The file ".alpm_db_version" is added to the local database root. This file is automatically created whenever the local database directory is absent or empty. This store the current database version as an integer. I have chosen to use the (next) libalpm soname version so it is easy for us to track what versions are needed.
Any comments on these?
I had one comment: <toofishes> pacman -Qip should work without a local DB, for instance
Lets have a look at pacman-4.1.2:
# rm -rf /var/lib/pacman/ # pacman -Qip /tmp/pacman-git-4.1.2_67_g7977aab-1-x86_64.pkg.tar.xz error: failed to initialize alpm library (could not find or read directory)
So, we need /var/lib/pacman to initialise the transaction already, so having pacman automatically add the /var/lib/pacman/local directory and a version file is not much of a burden.
That means the only change is when you have a local database and it is not the correct version:
# pacman -Qip pacman-git-4.1.2_77_g5ee4a65-1-x86_64.pkg.tar.xz error: failed to initialize alpm library (database is incorrect version) error: try running pacman-db-upgrade
(the spacing in that output is interesting...)
That requires a one-off database upgrade, which will be needed for any pacman operation. Given it blocks ALL pacman operations (which I am fairly sure was the case when we remove "depends" files too), I think this is fine.
Final request for more comments... Allan
participants (2)
-
Allan McRae
-
lolilolicon