[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