Re: setup.exe packagedb cache refresh bug

[Dave Korn <dave dot korn dot cygwin at gmail dot com>]

On 22/05/2010 16:52, Brendan Conoboy wrote:
> On 05/21/2010 11:18 PM, Christopher Faylor wrote:
>> This comment in  RootPage::OnNext would suggest that this was already
>> supposed to have been handled.
>>
>> /* Deferred initialization of packagedb *after* the root dir has been
>>       chosen. */
> 
> It appears you're looking at rev 2.25 of root.cc, while I'm looking at
> the latest, 2.26 which doesn't have that.  Dave Korn's changelog entry:
> 
> 2010-04-17  Dave Korn  
> 
>         * root.cc (RootPage::OnNext): Don't construct a packagedb here nor
>         do deferred initialisation of static packagedb::task.
>         * source.cc (save_dialog): Don't construct a packagedb here, and
>         set static packagedb::task directly instead of chosen_db_task.
>         * package_meta.cc (packagemeta::action_caption): Don't bother to
>         construct a packagedb here, just access packagedb::task directly.
>         * package_db.cc: Move 'static members' comment near static members.
>         (chosen_db_task): Delete.
>         * package_db.h (chosen_db_task): Don't declare extern.
>         (packagedb): Extend comments on class.
> 
> Presumably this has caused the problem.  Dave, what was the goal of this
> patch?

  The goal of that patch was to defer initialisation of packagedb not merely
until after the root dir had been chosen, but even later than that: it should
not now be populated until the setup.ini parsing stage runs.  So, I think that
regardless of that comment, the bug should have present in the earlier version
of the code anyway.

  You wrote:

> The problem occurs because packagedb is populated before the user has a
> chance to specify what directory to use for the installation.

  I can't find any way that can happen; can you describe the steps you used to
trigger the problem?  OTOH I _can_ see a problem if you get as far as the
chooser screen and then go all the way back to the root dir page, so I figure
we do need some kind of reset mechanism anyway.  I don't think we want to call
it from way down in mount.cc though, so I'll look for a higher level to put it
at, and I'll fix it to free up the existing data while I do that.

    cheers,
      DaveK