[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 00:44:04 PDT 2010


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


Mark Rowe (bdash) <mrowe at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #55666|review?                     |review-
               Flag|                            |




--- Comment #2 from Mark Rowe (bdash) <mrowe at apple.com>  2010-05-11 00:44:03 PST ---
(From update of attachment 55666)
> diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
> index 2a7d2eec0cec020427e124077d574d830c5a1b6c..b189ac0f631048162a62692eaa87ab05359fe462 100644
> --- a/WebKitTools/ChangeLog
> +++ b/WebKitTools/ChangeLog
> @@ -1,3 +1,30 @@
> +2010-05-10  Eric Seidel  <eric at webkit.org>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        Add script to check for minimum python version and install if missing on Tiger
> +        https://bugs.webkit.org/show_bug.cgi?id=38886
> +
> +        Per Maciej's request on webkit-dev:
> +        https://lists.webkit.org/pipermail/webkit-dev/2010-May/012785.html
> +        provide a script which can automatically install Python on Tiger where
> +        the system provided version is too old to be of use.
> +
> +        Note this uses the official Mac Python installer from python.org.
> +        This installs a system copy of Python.  It is not possible to install
> +        a non-system python without building our own copy.

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.

> diff --git a/WebKitTools/Scripts/ensure-valid-python b/WebKitTools/Scripts/ensure-valid-python
> new file mode 100755
> index 0000000000000000000000000000000000000000..cfb13f0283f908fab3c7465ce12cb135f3ab12c2
> --- /dev/null
> +++ b/WebKitTools/Scripts/ensure-valid-python
> @@ -0,0 +1,134 @@
> +#!/usr/bin/perl -w
> +# Copyright (C) 2010 Google Inc. All rights reserved.
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions
> +# are met:
> +#
> +# 1.  Redistributions of source code must retain the above copyright
> +#     notice, this list of conditions and the following disclaimer. 
> +# 2.  Redistributions in binary form must reproduce the above copyright
> +#     notice, this list of conditions and the following disclaimer in the
> +#     documentation and/or other materials provided with the distribution. 
> +# 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> +#     its contributors may be used to endorse or promote products derived
> +#     from this software without specific prior written permission. 

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

> +#
> +# THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
> +# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> +# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> +# DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
> +# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> +# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> +# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +use strict;
> +
> +use File::Basename;
> +use File::Spec;
> +use File::Temp qw(tempdir);
> +use FindBin;
> +
> +use lib $FindBin::Bin;
> +use webkitdirs;
> +use VCSUtils;
> +
> +my $macPythonURL = "http://www.python.org/ftp/python/2.6.5/python-2.6.5-macosx10.3-2010-03-24.dmg";
> +my $macPythonMD5 = "84489bba813fdbb6041b69d4310a86da";
> +my $macPythonInstallerName = "Python.mpkg";
> +
> +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?

> +sub downloadFileToPath($$)
> +{
> +    my ($remoteURL, $localPath) = @_;
> +    print "Downloading $remoteURL to $localPath\n";
> +    my $exitCode = system("curl", "-o", $localPath, $remoteURL);
> +    return exitStatus($exitCode) == 0;
> +}
> +
> +sub checkMD5($$)
> +{
> +    my ($path, $expectedMD5) = @_;
> +    my $md5Output = `md5 -q "$path"`;
> +    chomp($md5Output);
> +    my $isValid = $md5Output eq $expectedMD5;
> +    print "'$md5Output' does not match expected: '$expectedMD5'\n" unless $isValid;
> +    return $isValid;
> +}
> +
> +sub mountDMG($)
> +{
> +    my ($dmgPath) = @_;
> +    my $hdiutilOuput = `hdiutil attach $dmgPath`;

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

> +    # 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.

> +    return $mountPoint;
> +}
> +
> +sub unmountDMG($)
> +{
> +    my ($mountPoint) = @_;
> +    my $exitCode = system("hdiutil", "detach", $mountPoint);
> +    return exitStatus($exitCode) == 0;
> +}
> +
> +sub runInstaller($)
> +{
> +    my ($installerPackage) = @_;
> +    print "sudo will now ask for your password to run the Python installer.\n";
> +    system("sudo", "installer", "-verbose", "-pkg", $installerPackage, "-target", "/");
> +}

Do we care about the exit status of the installer?

> +sub installMacPython()
> +{
> +    my $mountPoint = downloadAndMountMacPythonDMG($macPythonURL, $macPythonMD5);
> +    print "Mounted python install image at: $mountPoint\n";
> +    my $installerPackage = File::Spec->join($mountPoint, $macPythonInstallerName);
> +    runInstaller($installerPackage);
> +    unmountDMG($mountPoint) or die "Failed to unmount DMG from $mountPoint";
> +}

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?

> +
> +sub main()
> +{
> +    # Congrats, your python is fine.
> +    return 0 if checkPythonVersion();
> +
> +    if (!isTiger()) {
> +        print "Your python version is insuficient to run WebKit's python.  Please update.\n";
> +        print "See http://trac.webkit.org/wiki/PythonGuidelines for more info.\n";
> +        return 1;
> +    }
> +
> +    installMacPython();
> +
> +    checkPythonVersion() or die "Final version check failed, must have failed to update python";
> +    print "Successfully updated python.\n";
> +}

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.

-- 
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