[pacman-dev] [PATCH] Separate local db dircetory creation and db write
Changelogs and install files were getting extracted into the local db folder before it was manually created. This created issues for uses with 0077 umasks and was highlighted with the new sudo handling of umasks (FS#12263). This moves the local db creation to its own function which is called before the start of package archive extraction. Also, added a check that the folder is actually created. Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/add.c | 10 ++++++++++ lib/libalpm/be_files.c | 24 +++++++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index eef7aab..c200c3c 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -696,6 +696,16 @@ static int commit_single_pkg(pmpkg_t *newpkg, int pkg_current, int pkg_count, } } + /* prepare directory for database entries so permission are correct after + changelog/install script installation (FS#12263) */ + if(_alpm_db_mkdir(db, newpkg)) { + alpm_logaction("error: could not create database directory %s-%s\n", + alpm_pkg_get_name(newpkg), alpm_pkg_get_version(newpkg)); + pm_errno = PM_ERR_DB_WRITE; + ret = -1; + goto cleanup; + } + if(!(trans->flags & PM_TRANS_FLAG_DBONLY)) { struct archive *archive; struct archive_entry *entry; diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index b9ff646..7d04ade 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -652,6 +652,26 @@ error: return(-1); } +int _alpm_db_mkdir(pmdb_t *db, pmpkg_t *info) +{ + mode_t oldmask; + int retval = 0; + char *pkgpath = NULL; + + oldmask = umask(0000); + + pkgpath = get_pkgpath(db, info); + + if((retval = mkdir(pkgpath, 0755)) != 0) { + _alpm_log(PM_LOG_ERROR, _("could not create directory %s: %s\n"), pkgpath, strerror(errno)); + } + + free(pkgpath); + umask(oldmask); + + return(retval); +} + int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) { FILE *fp = NULL; @@ -670,10 +690,8 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) pkgpath = get_pkgpath(db, info); - oldmask = umask(0000); - mkdir(pkgpath, 0755); /* make sure we have a sane umask */ - umask(0022); + oldmask = umask(0022); if(strcmp(db->treename, "local") == 0) { local = 1; -- 1.6.1
On Fri, Jan 2, 2009 at 3:12 AM, Allan McRae <allan@archlinux.org> wrote:
Changelogs and install files were getting extracted into the local db folder before it was manually created. This created issues for uses with 0077 umasks and was highlighted with the new sudo handling of umasks (FS#12263).
This moves the local db creation to its own function which is called before the start of package archive extraction. Also, added a check that the folder is actually created.
Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/add.c | 10 ++++++++++ lib/libalpm/be_files.c | 24 +++++++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index eef7aab..c200c3c 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -696,6 +696,16 @@ static int commit_single_pkg(pmpkg_t *newpkg, int pkg_current, int pkg_count, } }
+ /* prepare directory for database entries so permission are correct after + changelog/install script installation (FS#12263) */ + if(_alpm_db_mkdir(db, newpkg)) { + alpm_logaction("error: could not create database directory %s-%s\n", + alpm_pkg_get_name(newpkg), alpm_pkg_get_version(newpkg)); + pm_errno = PM_ERR_DB_WRITE; + ret = -1; + goto cleanup; + }
If we have a failure between here and when we actually write out the DB entry, what happens? I was hoping we would take the "extract the changelog/install file at later time" route, but that is definitely a bit more of a hassle... Calling a function _alpm_db_mkdir() ties us awfully tight to the existing be_files implementation, although I'm sure most of this code will need a lot of work if we decide to change things up.
if(!(trans->flags & PM_TRANS_FLAG_DBONLY)) { struct archive *archive; struct archive_entry *entry; diff --git a/lib/libalpm/be_files.c b/lib/libalpm/be_files.c index b9ff646..7d04ade 100644 --- a/lib/libalpm/be_files.c +++ b/lib/libalpm/be_files.c @@ -652,6 +652,26 @@ error: return(-1); }
+int _alpm_db_mkdir(pmdb_t *db, pmpkg_t *info) +{ + mode_t oldmask; + int retval = 0; + char *pkgpath = NULL; + + oldmask = umask(0000); + + pkgpath = get_pkgpath(db, info); + + if((retval = mkdir(pkgpath, 0755)) != 0) { + _alpm_log(PM_LOG_ERROR, _("could not create directory %s: %s\n"), pkgpath, strerror(errno)); + } + + free(pkgpath); + umask(oldmask); + + return(retval); +} + int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq) { FILE *fp = NULL; @@ -670,10 +690,8 @@ int _alpm_db_write(pmdb_t *db, pmpkg_t *info, pmdbinfrq_t inforeq)
pkgpath = get_pkgpath(db, info);
- oldmask = umask(0000); - mkdir(pkgpath, 0755); /* make sure we have a sane umask */ - umask(0022); + oldmask = umask(0022);
if(strcmp(db->treename, "local") == 0) { local = 1; -- 1.6.1
Dan McGee wrote:
On Fri, Jan 2, 2009 at 3:12 AM, Allan McRae <allan@archlinux.org> wrote:
Changelogs and install files were getting extracted into the local db folder before it was manually created. This created issues for uses with 0077 umasks and was highlighted with the new sudo handling of umasks (FS#12263).
This moves the local db creation to its own function which is called before the start of package archive extraction. Also, added a check that the folder is actually created.
Signed-off-by: Allan McRae <allan@archlinux.org> --- lib/libalpm/add.c | 10 ++++++++++ lib/libalpm/be_files.c | 24 +++++++++++++++++++++--- 2 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/lib/libalpm/add.c b/lib/libalpm/add.c index eef7aab..c200c3c 100644 --- a/lib/libalpm/add.c +++ b/lib/libalpm/add.c @@ -696,6 +696,16 @@ static int commit_single_pkg(pmpkg_t *newpkg, int pkg_current, int pkg_count, } }
+ /* prepare directory for database entries so permission are correct after + changelog/install script installation (FS#12263) */ + if(_alpm_db_mkdir(db, newpkg)) { + alpm_logaction("error: could not create database directory %s-%s\n", + alpm_pkg_get_name(newpkg), alpm_pkg_get_version(newpkg)); + pm_errno = PM_ERR_DB_WRITE; + ret = -1; + goto cleanup; + }
If we have a failure between here and when we actually write out the DB entry, what happens?
If a failure occurs, we have an empty folder or a folder with only .changelog and .install in it (which we could have ended up with anyway). I think dealing with failure here probably needs a clean-up anyway. As far as I can tell, if installing a package fails partway through the archive extraction, we end up with a lot of untracked files on the system. We should probably either keep a list of files installed so far so we can revert it or push the list to the db as extraction happens so we know their owner (which removes abstraction...).
I was hoping we would take the "extract the changelog/install file at later time" route, but that is definitely a bit more of a hassle...
But then we would need to re-deal with setting up reading from the archive file which seems a bit overkill.
Calling a function _alpm_db_mkdir() ties us awfully tight to the existing be_files implementation, although I'm sure most of this code will need a lot of work if we decide to change things up.
Sure, but as I said above, I think this whole area could do with a change. The patch I provided was what I think is the best way to deal with the issue in a maint release. Allan
participants (2)
-
Allan McRae
-
Dan McGee