Closed Bug 872231 Opened 7 years ago Closed 7 years ago

Bootstrapper should upgrade Python and Mercurial

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla24

People

(Reporter: gps, Assigned: gps)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Currently the bootstrapper just ensures packages are installed. I'd like to extend it to support ensuring specific minimum versions of packages are installed. Let's start with Python and Mercurial.
Attachment #749486 - Flags: review?(ted)
Absolute fly-by nit: checkin message says "Bootstrapper ensures Python 2.7.3 and Mercurial 2.5 are installed" but if I'm not wrong the following code:

+# Upgrade Mercurial older than this.
+MODERN_MERCURIAL_VERSION = StrictVersion('2.6')

may mean that even Mercurial 2.5.4 (whatever we are updating to on our releng instances) is updated to 2.6.

Is this intended?
No, I meant 2.5. I updated my patch locally.

Also, I don't intend for this code to run in automation. Although, I wouldn't be surprised if that's not always a requirement.
Comment on attachment 749486 [details] [diff] [review]
Upgrade Mercurial and Python, v1

Review of attachment 749486 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozboot/mozboot/base.py
@@ +65,5 @@
> +'''
> +
> +
> +# Upgrade Mercurial older than this.
> +MODERN_MERCURIAL_VERSION = StrictVersion('2.6')

This seems super strict. Do we really need to push everyone to 2.6? I'd hate to confuse a bunch of users who don't have it available for whatever reason.

@@ +216,5 @@
> +        """Upgrade Mercurial.
> +
> +        Child classes should reimplement this.
> +        """
> +        print(MERCURIAL_UNABLE_UPGRADE % (current, MODERN_MERCURIAL_VERSION))

We should only ever get this if the bootstrapper for this platform didn't implement this method, right? It would be nicer if we could force the subclasses to implement this, but I guess that's tricky in a dynamic language...

::: python/mozboot/mozboot/osx.py
@@ +318,5 @@
> +    def upgrade_python(self, current):
> +        if self.package_manager == 'homebrew':
> +            self._upgrade_package('python')
> +        else:
> +            self._upgrade_package('python27')

It kind of feels like you want separate bootstrap subclasses for brew and macports...
Attachment #749486 - Flags: review?(ted) → review+
Assignee: nobody → gps
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #4)
> It kind of feels like you want separate bootstrap subclasses for brew and
> macports...

Indeed. Code always starts simple.
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/b7175c5829b5

Also, reduced Mercurial requirement to 2.5 per comment 3.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.