[pacman-dev] [PATCH v2 0/4] Handle .alpm_db_version file

newer
[pacman-dev] [PATCH 1/2] makepkg:...

Allan McRae

22 Sep 2014 22 Sep '14

6:44 a.m.

This is v2 of a patchset submitted long ago. It has been rebased and had its memory leaks fixed. With this patchset, pacman will create a local database directory if it is missing (but not the root of the database - i.e. /var/lib/pacman/ still needs to be present). When creating the database or finding an empty local database it will add the .alpm_db_version file and add the version. For existing pacman databases this file is created by the pacman-db-upgrade script. Finally the version of the local database is checked during db validation. Allan McRae (4): 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 lib/libalpm/be_local.c | 119 +++++++++++++++++++++++++++++++++---------------- test/pacman/pmdb.py | 2 + test/pacman/pmtest.py | 3 ++ 3 files changed, 85 insertions(+), 39 deletions(-) -- 2.1.0

Show replies by date

Allan McRae

22 Sep 22 Sep

6:44 a.m.

New subject: [pacman-dev] [PATCH 1/4 v2] Create local database directory if it is missing

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 checked for during locking. Signed-off-by: Allan McRae --- lib/libalpm/be_local.c | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 9a9bdef..d18e9ad 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; + /* local database dir doesn't exist yet - create it */ + if(local_db_create(db, dbpath) == 0) { + 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,13 +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; - db->pkgcache = _alpm_pkghash_create(0); - return 0; - } RET_ERR(db->handle, ALPM_ERR_DB_OPEN, -1); } if(fstat(dirfd(dbdir), &buf) != 0) { -- 2.1.0

Andrew Gregory

7:29 a.m.

New subject: [pacman-dev] [PATCH 1/4 v2] Create local database directory if it is missing

On 09/22/14 at 10:44pm, Allan McRae wrote:

...

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 checked for during locking.

Signed-off-by: Allan McRae --- lib/libalpm/be_local.c | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 9a9bdef..d18e9ad 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; + /* local database dir doesn't exist yet - create it */ + if(local_db_create(db, dbpath) == 0) { + db->status |= DB_STATUS_VALID; + db->status &= ~DB_STATUS_INVALID; + db->status &= ~DB_STATUS_EXISTS; + db->status |= DB_STATUS_MISSING;

The EXISTS and MISSING operations need to be switched with the ones below.

...

+ 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,13 +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; - db->pkgcache = _alpm_pkghash_create(0); - return 0; - } RET_ERR(db->handle, ALPM_ERR_DB_OPEN, -1); } if(fstat(dirfd(dbdir), &buf) != 0) { -- 2.1.0

Allan McRae

23 Sep 23 Sep

5:23 a.m.

New subject: [pacman-dev] [PATCH 1/4 v2] Create local database directory if it is missing

On 22/09/14 23:29, Andrew Gregory wrote:

...

On 09/22/14 at 10:44pm, Allan McRae wrote:

...
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 checked for during locking.

Signed-off-by: Allan McRae --- lib/libalpm/be_local.c | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 9a9bdef..d18e9ad 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; + /* local database dir doesn't exist yet - create it */ + if(local_db_create(db, dbpath) == 0) { + db->status |= DB_STATUS_VALID; + db->status &= ~DB_STATUS_INVALID; + db->status &= ~DB_STATUS_EXISTS; + db->status |= DB_STATUS_MISSING;

The EXISTS and MISSING operations need to be switched with the ones below.

Moving the above to below is correct. However, I think the above should be: 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,13 +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; - db->pkgcache = _alpm_pkghash_create(0); - return 0; - } RET_ERR(db->handle, ALPM_ERR_DB_OPEN, -1); } if(fstat(dirfd(dbdir), &buf) != 0) { -- 2.1.0

Allan McRae

22 Sep 22 Sep

6:44 a.m.

New subject: [pacman-dev] [PATCH 2/4 v2] Add version file when creating local database directory

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 --- 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 d18e9ad..8d78ba5 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 a6cf78f..a1c0239 100644 --- a/test/pacman/pmdb.py +++ b/test/pacman/pmdb.py @@ -87,6 +87,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 -- 2.1.0

Allan McRae

6:45 a.m.

New subject: [pacman-dev] [PATCH 3/4 v2] Add version file to empty local database

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 --- lib/libalpm/be_local.c | 44 ++++++++++++++++++++++++++++++++------------ test/pacman/pmtest.py | 3 +++ 2 files changed, 35 insertions(+), 12 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 8d78ba5..1707a76 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; @@ -412,6 +413,7 @@ static int local_db_validate(alpm_db_t *db) if(dbpath == NULL) { RET_ERR(db->handle, ALPM_ERR_DB_OPEN, -1); } + dbdir = opendir(dbpath); if(dbdir == NULL) { if(errno == ENOENT) { @@ -435,6 +437,24 @@ static int local_db_validate(alpm_db_t *db) db->status |= DB_STATUS_EXISTS; db->status &= ~DB_STATUS_MISSING; + snprintf(dbverpath, PATH_MAX, "%s.alpm_db_version", dbpath); + + 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 { + goto version_error; + } + } + + local_db_add_version(db, dbpath); + goto version_latest; + } + fclose(dbverfile); + while((ent = readdir(dbdir)) != NULL) { const char *name = ent->d_name; char path[PATH_MAX]; @@ -449,23 +469,23 @@ static int local_db_validate(alpm_db_t *db) 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; + goto version_error; } } /* we found no depends file after full scan */ + +version_latest: + closedir(dbdir); db->status |= DB_STATUS_VALID; db->status &= ~DB_STATUS_INVALID; - ret = 0; - -done: - if(dbdir) { - closedir(dbdir); - } + return 0; - return ret; +version_error: + closedir(dbdir); + 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 78b9e2d..75a28d2 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") -- 2.1.0

Allan McRae

6:45 a.m.

New subject: [pacman-dev] [PATCH 4/4 v2] Check the version of the local database during validation

When we check the database version directly, there is no longer a need to scan for depends files. Signed-off-by: Allan McRae --- It is much easier to look at the before and after here instead of the patch directly. lib/libalpm/be_local.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 1707a76..2b7260e 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; @@ -453,26 +455,15 @@ static int local_db_validate(alpm_db_t *db) local_db_add_version(db, dbpath); goto version_latest; } - fclose(dbverfile); - while((ent = readdir(dbdir)) != NULL) { - const char *name = ent->d_name; - char path[PATH_MAX]; + t = fscanf(dbverfile, "%zu", &version); + (void)t; - if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) { - continue; - } - if(!is_dir(dbpath, ent)) { - continue; - } + fclose(dbverfile); - snprintf(path, PATH_MAX, "%s%s/depends", dbpath, name); - if(access(path, F_OK) == 0) { - /* we found a depends file- bail */ - goto version_error; - } + if(version != ALPM_LOCAL_DB_VERSION) { + goto version_error; } - /* we found no depends file after full scan */ version_latest: closedir(dbdir); -- 2.1.0

Andrew Gregory

7:33 a.m.

New subject: [pacman-dev] [PATCH 4/4 v2] Check the version of the local database during validation

On 09/22/14 at 10:45pm, Allan McRae wrote:

...

When we check the database version directly, there is no longer a need to scan for depends files.

Signed-off-by: Allan McRae ---

It is much easier to look at the before and after here instead of the patch directly.

lib/libalpm/be_local.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/lib/libalpm/be_local.c b/lib/libalpm/be_local.c index 1707a76..2b7260e 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; @@ -453,26 +455,15 @@ static int local_db_validate(alpm_db_t *db) local_db_add_version(db, dbpath); goto version_latest; } - fclose(dbverfile);

- while((ent = readdir(dbdir)) != NULL) { - const char *name = ent->d_name; - char path[PATH_MAX]; + t = fscanf(dbverfile, "%zu", &version); + (void)t;

- if(strcmp(name, ".") == 0 || strcmp(name, "..") == 0) { - continue; - } - if(!is_dir(dbpath, ent)) { - continue; - } + fclose(dbverfile);

- snprintf(path, PATH_MAX, "%s%s/depends", dbpath, name); - if(access(path, F_OK) == 0) { - /* we found a depends file- bail */ - goto version_error; - } + if(version != ALPM_LOCAL_DB_VERSION) {

Add a check for t here to make sure fscanf succeeded. If fscanf failed there's no telling what's in version, and you can get rid of the (void)t.

...

+ goto version_error; } - /* we found no depends file after full scan */

version_latest: closedir(dbdir); -- 2.1.0

Dan McGee

8:02 a.m.

On Mon, Sep 22, 2014 at 7:44 AM, Allan McRae wrote:

...

This is v2 of a patchset submitted long ago. It has been rebased and had its memory leaks fixed.

With this patchset, pacman will create a local database directory if it is missing (but not the root of the database - i.e. /var/lib/pacman/ still needs to be present). When creating the database or finding an empty local database it will add the .alpm_db_version file and add the version. For existing pacman databases this file is created by the pacman-db-upgrade script. Finally the version of the local database is checked during db validation.

I'm fine with the idea, but on a more meta level, is there any reason this should be a hidden file? Why not just call it something like DB-VERSION so it is big and obvious? I find the idea of hidden files in non-user home directories less than ideal as it makes investigation that much harder until you remember to do a full `ls -la`. (For example: svn stores its version in the root of the repository in a `format` file, http://serverfault.com/questions/277441/difference-between-the-format-and-db... )

...

Allan McRae (4): 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

lib/libalpm/be_local.c | 119 +++++++++++++++++++++++++++++++++---------------- test/pacman/pmdb.py | 2 + test/pacman/pmtest.py | 3 ++ 3 files changed, 85 insertions(+), 39 deletions(-)

-- 2.1.0

Allan McRae

5:46 p.m.

On 23/09/14 00:02, Dan McGee wrote:

...

On Mon, Sep 22, 2014 at 7:44 AM, Allan McRae wrote:

...
This is v2 of a patchset submitted long ago. It has been rebased and had its memory leaks fixed.

With this patchset, pacman will create a local database directory if it is missing (but not the root of the database - i.e. /var/lib/pacman/ still needs to be present). When creating the database or finding an empty local database it will add the .alpm_db_version file and add the version. For existing pacman databases this file is created by the pacman-db-upgrade script. Finally the version of the local database is checked during db validation.

I'm fine with the idea, but on a more meta level, is there any reason this should be a hidden file? Why not just call it something like DB-VERSION so it is big and obvious? I find the idea of hidden files in non-user home directories less than ideal as it makes investigation that much harder until you remember to do a full `ls -la`.

(For example: svn stores its version in the root of the repository in a `format` file, http://serverfault.com/questions/277441/difference-between-the-format-and-db... )

I'm happy renaming this. I'll use ALPM_DB_VERSION unless someone speaks up.

3510

Age (days ago)

3511

Last active (days ago)

List overview

Download

9 comments

3 participants

participants (3)

Allan McRae
Andrew Gregory
Dan McGee

[pacman-dev] [PATCH v2 0/4] Handle .alpm_db_version file

tags

participants (3)