[pacman-dev] [PATCH] Read 'force' entry from packages

[Dan McGee]

On Thu, Oct 14, 2010 at 3:39 PM, Xavier Chantry
<chantry.xavier at gmail.com> wrote:
> On Thu, Oct 14, 2010 at 8:48 PM, Xavier Chantry
> <chantry.xavier at gmail.com> wrote:
>> On Tue, Oct 12, 2010 at 12:51 AM, Dan McGee <dan at archlinux.org> wrote:
>>> We weren't reading this in from our packages, thus causing us not to write
>>> it out to our local database. Adding this now will help ease the upgrade
>>> path for epoch later and not require reinstallation of all force packages.
>>>
>>
>> I did not see a problem with that patch, and still don't see any, but
>> I have some new insight after trying out git master.
>>
>> I think we need a similar patch in master, we still need to support
>> old package with just force. We should do that just like sync db does
>> (force -> epoch=1). This will ensure that epoch is always written in
>> local db for package with force.

You are correct, we do need this.

>> diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
>> index ddcad59..254f830 100644
>> --- a/lib/libalpm/be_package.c
>> +++ b/lib/libalpm/be_package.c
>> @@ -184,6 +184,11 @@ static int parse_descfile(struct archive *a,
>> pmpkg_t *newpkg)
>>                                STRDUP(newpkg->version, ptr,
>> RET_ERR(PM_ERR_MEMORY, -1));
>>                        } else if(strcmp(key, "pkgdesc") == 0) {
>>                                STRDUP(newpkg->desc, ptr,
>> RET_ERR(PM_ERR_MEMORY, -1));
>> +                       } else if(strcmp(key, "force") == 0) {
>> +                               /* For backward compatibility, like in
>> sync_db_read */
>> +                               if(!newpkg->epoch) {
>> +                                       newpkg->epoch = 1;
>> +                               }
>>                        } else if(strcmp(key, "epoch") == 0) {
>>                                newpkg->epoch = atoi(ptr);
>>                        } else if(strcmp(key, "group") == 0) {
>>
>>
>> As for the maint patch, it seems weird that we will start to write
>> FORCE entries to local database now we decided to kill FORCE. Wouldn't
>> it be better to start writing EPOCH so that we don't start to clutter
>> the local db now with something which is dead ? :)

No, because we are then half-introducing a feature to a maint branch
where I want as little breakage as possible, and also a database that
doesn't require a lot of extra parsing code.

>> On my system I just have 17 packages to reinstall, 19MB download (but
>> 0MB download actually, thats what a download cache is for). This is
>> done automatically on the first pacman -Su, and with above patch, the
>> transition should be made.
>> So I don't think we need to start writing anything to the local
>> database now, do we ?
>>
>> If we still want to do that just to reduce a bit the (already small)
>> number of packages to reinstall, what about starting to write EPOCH in
>> maint ?
>> I believe we just need two small changes :
>> 1) in maint : never write FORCE in local db but write EPOCH=1 instead
>> 2) in master : we can drop reading FORCE from local db since it never
>> ever got written. (and EPOCH is already read)
>>
>> But anyway this is really a minor point, and my last proposal would
>> actually need the 2-lines fix that got pushed to maint.
>>
>
> I just pushed this work, 3 small patches.
>
> http://code.toofishes.net/cgit/xavier/pacman.git/log/?h=epoch-maint
> * be_files: write EPOCH instead of FORCE
> http://code.toofishes.net/cgit/xavier/pacman.git/log/?h=epoch-master
> * be_package: read force entry and convert to epoch
> * be_local: drop FORCE in db_read

So yes, I'm a bit torn here, but I always fall on the "let's not touch
maint more than necessary" side of the fence, and this starts to push
that line. I'm fine with the "read force from package" patch, but the
rest I'm just not so keen on.

-Dan