[Webkit-unassigned] [Bug 38886] Add script to check for minimum python version and install if missing on Tiger

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 11 10:06:56 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=38886





--- Comment #3 from Eric Seidel <eric at webkit.org>  2010-05-11 10:06:55 PST ---
(In reply to comment #2)
> (From update of attachment 55666 [details])

> I’m not sure that this is accurate.  The package appears to write Python.framework in to /Library/Frameworks/Python.framework.  That’s not where the system version of Python lives.
> 
> If I’m mistake about that and your initial comment is correct then… we really shouldn’t be overwriting the system version of Python.  Overwriting system files is not supported.  I’d really prefer that we didn’t encourage this, and we certainly shouldn’t be doing this without informing the user.

You're correct, the installer appears to install on /Library.  Do you know of any way to perform a DISTROOT install with installer?  So I can actually see every file it dumps?  Since it uses several sub installers, it's confusing (using lsbom Archive.bom) to figure out where they all drop their payloads.

> “Apple Computer, Inc” no longer exists.  It should not be referenced in new license text.

Fixed.

> > +sub checkPythonVersion()
> > +{
> > +    # Will exit 0 if python is 2.5 or greater, non-zero otherwise.
> > +    `python -c "import sys;sys.exit(sys.version_info[:2] < (2,6))"`;
> > +    return exitStatus($?) == 0;
> > +}
> 
> The comment here mentions Python 2.5 but the check appears to be for v2.6.  Is that intentional?

A mistake left over from debugging on Leopard (which already has 2.5).  Fixed.

> This will result in the disk image opening in the Finder and appearing in the sidebar.  hdiutil has a “-nobrowse” flag to prevent this.

Correct.  This was intentional.  If the script failed I didn't want to a random invisible mount around.  I'm happy to change it if you think it's better to hide the mount.

> > +    # One of the lines of output is: "/dev/disk_name    Apple_HFS    /Volumes/Mount Point"
> > +    if ($hdiutilOuput !~ /^(.*)\s+Apple_HFS\s+(.*)$/m) {
> > +        die "Failed to find mountpoint in \n$hdiutilOuput\nMay have left a disk image mounted."
> > +    }
> > +    my $mountPoint = $2;
> 
> Rather than doing this regexp hackery you could generate a mount point path yourself (somewhere under $TMPDIR?) and use hdiutil’s “-mountpoint” flag to arrange for it to be mounted there.

I considered that, however I didn't know how rm would fair trying to delete a mount point.  I guess I'll just have to test it.  The temp directory from File::Temp->createdir automatically deletes itself when perl exits.

> Do we care about the exit status of the installer?

Yes, sorry.  Fixed.

> Should this delete the disk image when it is done?  Can we refer to it as a “disk image” instead of “DMG” in the error message since that is the term by which they’re typically known?

We download the image to our temp directory which gets deleted by File::Temp automatically when python exits.

I've fixed the log to say "disk image", thanks.

> I’d suggest that “Python” have an initial capital in these messages as that is how the name of the language and implementation is typically rendered in text.  I’d suggest mentioning “WebKit’s Python code” rather than “WebKit’s Python” for sake of clarity.

Fixed.

I'll test having it mount into the temp directory with nobrowse and see how that does.  Then I'll post a new patch.

Thank you for the quick review.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list