[pacman-dev] [PATCH] Separate local db dircetory creation and db write
Allan McRae
allan at archlinux.org
Fri Jan 2 23:04:34 EST 2009
Dan McGee wrote:
> On Fri, Jan 2, 2009 at 3:12 AM, Allan McRae <allan at 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 at 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
More information about the pacman-dev
mailing list