This is the mail archive of the
mailing list for the Cygwin project.
Re: [PATCH] Incidental setup.exe patches #3: Simplify packagedb task handling
On 14/04/2010 14:30, Dave Korn wrote:
> On 14/04/2010 09:21, Corinna Vinschen wrote:
>> Are you sure? Please check again. The original situation which this
>> was supposed to fix is this:
>> - You have an existing installation in C:\cygwin
>> - You want to create a new installation in D:\cygwin-new
>> - You choose D:\cygwin-new as root dir in the root dir dialog
>> - The package list was nevertheless based on the c:\cygwin directory
>> installation, because the package_db stuff was already initialized
>> with the Cygwin root dir set to C:\cygwin
> 1. There's no problem setting the class-static task variable early: it's just
> constructing the first packagedb that mustn't be done until after the root is set.
> 1a. So we can still get rid of the caching-and-deferred-initialisation. The
> original bug was solved by /not/ constructing a packagedb at the original site
> where the task was set, rather than specifically by constructing one later.
> 1b. It never needed to construct a packagedb at that point anyway, because you
> don't need a this-pointer to set a static member; it was that misunderstanding
> that caused the bug in the first place.
> 2. There's no need to construct a packagedb immediately after the root dir is
> set; whenever we first construct one subsequently, it'll work.
> 3. For that reason the patch ought to fix the original bug in any case.
> I have to run out to the shops for half an hour now, and when I get back
> I'll fire up a VM, and make absolutely certain.
So I verified not just that the bug was indeed fixed (point 3), but also
that my theory in point 1 was correct by adding code back to save_dialog() in
source.cc that just constructed a packagedb and threw it away and verifying
that it re-introduced the bug.
I'll rewrite the comments now I know exactly what's going on and resend it