Closed Bug 1449629 Opened 3 years ago Closed 3 years ago

Install Python 3.5 in Debian build images

Categories

(Firefox Build System :: General, enhancement)

Product:
Firefox Build System
Component:
General
Version:
3 Branch
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox61 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox61 --- fixed

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(3 files)

Bug 1449629 - Install Python 3.5 in Debian 7 base image;
59 bytes, text/x-review-board-request
glandium
: review+
Details
Bug 1449629 - Install Python 3.5 in debian-base;
59 bytes, text/x-review-board-request
glandium
: review+
Details
Bug 1449629 - Use -L when downloading OpenSSL;
59 bytes, text/x-review-board-request
glandium
: review+
Details 
We need to install Python 3 in our Debian build images to support bug 1388447.

wheezy ships Python 3.2. So we'll need some kind of custom package to get a modern Python version installed.

Since Python is used by various other system packages, swapping out the official python3* packages for Python 3.5 scares me a little. I'm not sure if we could get away with that or if we should provide python3.x packages that don't conflict with the official Python packages (e.g. by attempting to manage the `python3` executable). The build system does check for `python3.x` binaries before `python3`. So it /should/ be possible to have multiple Python 3.x installs living side-by-side. FWIW, I do see the real possibility of having multiple Python 3.x installs in the same image. We may want 3.5 around for minimal version compatibility. But we may want to aggressively use 3.6, 3.7, etc in builds due to perf advantages, etc. I suppose we could have different images with different Python versions. I dunno how this will shake out.

glandium: do you have any opinions on how we should go about the Debian packaging?
Flags: needinfo?(mh+mozilla)
I think for the most part the debian python packages are co-installable. So it should be possible to build a python 3.5 backport that doesn't interfere with the existing python 3 package.
Flags: needinfo?(mh+mozilla)
The Try push for this is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=77fc2766288b7c5036e133da16979facf379fefe.

When reviewing, assume I don't know what I'm doing. This is my first time seriously foraging into our custom Debian packaging setup.
Assignee: nobody → gps
Status: NEW → ASSIGNED
Comment on attachment 8963863 [details]
Bug 1449629 - Install Python 3.5 in Debian 7 base image;

https://reviewboard.mozilla.org/r/232724/#review238600

::: taskcluster/ci/packages/kind.yml:45
(Diff revision 1)
> +    treeherder:
> +      symbol: Deb7(python3.5)
> +    run:
> +      using: debian-package
> +      dsc:
> +        url: http://snapshot.debian.org/archive/debian/20180301T094050Z/pool/main/p/python3.5/python3.5_3.5.4-4.dsc

I'd take 3.5.3-1, because that's the version in debian stable and if there are necessary security updates they will happen on that version but likely won't on 3.5.4. That would also be the same version as for debian-9 jobs, reducing possible differences.
Attachment #8963863 - Flags: review?(mh+mozilla)
Comment on attachment 8963864 [details]
Bug 1449629 - Install Python 3.5 in debian-base;

https://reviewboard.mozilla.org/r/232726/#review238608

::: commit-message-b3a85:11
(Diff revision 1)
> +There /might/ be an issue with the default package declaring a
> +libexpat dependency that conflicts with some 32-bit -dev packages.
> +But the Try push seemed happy and I /think/ we only have a 32-bit
> +variant image based on Debian 7 currently. So this might not be
> +an issue.

The libexpat-dev thing is probably not a problem in Debian 9. The problem in Debian 7 was that multi-arch was barely started back then, and co-installable i386 and amd64 packages weren't a given. libexpat wasn't one of them. In Debian 9, multi-arch is more prominent, and libexpat-dev is probably co-installable.

All that to say, you can skip this paragraph.

::: commit-message-b3a85:19
(Diff revision 1)
> +variant image based on Debian 7 currently. So this might not be
> +an issue.
> +
> +This does increase the size of the images a bit:
> +
> +deb7-base: 218,155,008 / 79,005,868 -> 246,023,680 / 87,370,182

Not /too/ far off.
Looking at the logs for deb7-base on central:

[task 2018-03-24T21:03:37.850Z] Need to get 31.9 MB of archives.
[task 2018-03-24T21:03:37.850Z] After this operation, 90.5 MB of additional disk space will be used.

on your try:

[task 2018-03-30T03:54:44.813Z] Need to get 37.6 MB of archives.
[task 2018-03-30T03:54:44.813Z] After this operation, 109 MB of additional disk space will be used.

So as compressed .deb the difference is ~5.7M, and 19M uncompressed, but the uncompressed size given by apt is an estimate based on metadata in packages, and that can be rather wrong, especially for packages, like python, that generate file in postinstall (iirc, it generates .pyc or .pyo files).
Attachment #8963864 - Flags: review?(mh+mozilla) → review+
(In reply to Gregory Szorc [:gps] from comment #4)
> The Try push for this is at
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=77fc2766288b7c5036e133da16979facf379fefe.
> 
> When reviewing, assume I don't know what I'm doing. This is my first time
> seriously foraging into our custom Debian packaging setup.

You did a pretty good job for a first attempt :)
Comment on attachment 8963864 [details]
Bug 1449629 - Install Python 3.5 in debian-base;

https://reviewboard.mozilla.org/r/232726/#review238608

> Not /too/ far off.
> Looking at the logs for deb7-base on central:
> 
> [task 2018-03-24T21:03:37.850Z] Need to get 31.9 MB of archives.
> [task 2018-03-24T21:03:37.850Z] After this operation, 90.5 MB of additional disk space will be used.
> 
> on your try:
> 
> [task 2018-03-30T03:54:44.813Z] Need to get 37.6 MB of archives.
> [task 2018-03-30T03:54:44.813Z] After this operation, 109 MB of additional disk space will be used.
> 
> So as compressed .deb the difference is ~5.7M, and 19M uncompressed, but the uncompressed size given by apt is an estimate based on metadata in packages, and that can be rather wrong, especially for packages, like python, that generate file in postinstall (iirc, it generates .pyc or .pyo files).

I was going off image.zst size. I think there is a mismatch between what I stated in the commit message and the readings I took. Plus, I think the commit message lagged behind Try pushes by a few iterations.

Also, the zstandard compression ratio in Try is different from the compression ratio on other repos. The reasoning is we want Try pushes to be fast, so we shouldn't spend time doing compression. So the compressed numbers don't mean that much. I'll remove those.
Comment on attachment 8963863 [details]
Bug 1449629 - Install Python 3.5 in Debian 7 base image;

https://reviewboard.mozilla.org/r/232724/#review238624

I thought I'd get lucky with the patch application. But it failed on a Try run. I'll submit a new version in a few minutes once I've worked out the patch conflicts.
Comment on attachment 8963863 [details]
Bug 1449629 - Install Python 3.5 in Debian 7 base image;

https://reviewboard.mozilla.org/r/232724/#review238634

The diff is whitespace damaged. FML.
Attachment #8963863 - Flags: review-
Comment on attachment 8963863 [details]
Bug 1449629 - Install Python 3.5 in Debian 7 base image;

https://reviewboard.mozilla.org/r/232724/#review238640

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cb03903f50a5af988fd303d7fac91ab2123f985c is looking like it will take. Seriously, I should know better than to attempt to copy and paste a patch from my terminal.
Comment on attachment 8963863 [details]
Bug 1449629 - Install Python 3.5 in Debian 7 base image;

https://reviewboard.mozilla.org/r/232724/#review238650

::: build/debian-packages/python3.5-wheezy.diff:1
(Diff revision 5)
> +diff --git a/debian/changelog b/debian/changelog

FWIW, the way I generate those diffs, usually:
$ dpkg-source -x original-package.dsc
$ ... do changes in the extracted directory
$ dpkg-source -b extracted-directory
# this generates a package-new_version.dsc
# debdiff original-package.dsc package-new_version.dsc

::: taskcluster/ci/packages/kind.yml:45
(Diff revision 5)
> +    treeherder:
> +      symbol: Deb7(python3.5)
> +    run:
> +      using: debian-package
> +      dsc:
> +        url: http://snapshot.debian.org/archive/debian/20180301T094050Z/pool/main/p/python3.5/python3.5_3.5.3-1.dsc

Not sure how you found this, but http://snapshot.debian.org/package/python3.5/3.5.3-1/ gives 20170119T211826Z.
Attachment #8963863 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8963863 [details]
Bug 1449629 - Install Python 3.5 in Debian 7 base image;

https://reviewboard.mozilla.org/r/232724/#review238650

> FWIW, the way I generate those diffs, usually:
> $ dpkg-source -x original-package.dsc
> $ ... do changes in the extracted directory
> $ dpkg-source -b extracted-directory
> # this generates a package-new_version.dsc
> # debdiff original-package.dsc package-new_version.dsc

I was wondering about that.

For round 1, I used dquilt, per some Debian docs I found. That gave me some horrible MQ PTSD (and was super annoying to use because I had to remember to mark a file for tracking), so for round 2 I just did `git init . && git add *` and used `git diff` to produce the diff.

I'll try to remember to use `dpkg-source` + `debdiff` next time.

> Not sure how you found this, but http://snapshot.debian.org/package/python3.5/3.5.3-1/ gives 20170119T211826Z.

I used a recent snapshot, thinking it didn't much matter. I'll change to the first revision containing this package version.
Comment on attachment 8964459 [details]
Bug 1449629 - Use -L when downloading OpenSSL;

https://reviewboard.mozilla.org/r/233198/#review238664
Attachment #8964459 - Flags: review?(mh+mozilla) → review+
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/abb14d1d2f0c
Use -L when downloading OpenSSL; r=glandium
https://hg.mozilla.org/integration/autoland/rev/4b89260566da
Install Python 3.5 in Debian 7 base image; r=glandium
https://hg.mozilla.org/integration/autoland/rev/87840a3dd715
Install Python 3.5 in debian-base; r=glandium
abb14d1d2f0c should probably be uplifted to all release repos. If it isn't, if we ever trigger a Docker/toolchain rebuild, things will fail due to failure fetching the OpenSSL source distribution.

The patch should generally be harmless: it just tells curl to follow redirects when downloading OpenSSL when building sccache. However, landing the patch will cause sccache and some Docker images to be rebuilt. That's mostly deterministic on trunk, but not so much so on <60 (or maybe 59 - I can't recall when Docker images switched to Debian and were made deterministic).

(I tried to flag this patch for tracking, but I didn't see the options. Perhaps this Bugzilla component isn't configured to allow that?)
Flags: needinfo?(ryanvm)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/d03b5dcda9ec
Port bug 1449629 [Install Python 3.5 in Debian build images]. rs=bustage-fix
(In reply to Gregory Szorc [:gps] from comment #27)
> abb14d1d2f0c should probably be uplifted to all release repos. If it isn't,
> if we ever trigger a Docker/toolchain rebuild, things will fail due to
> failure fetching the OpenSSL source distribution.

I have a hard time believing we need this on m-r for 59 at this point, but I agree about getting it on Beta60 ahead of the next ESR anyway.

https://hg.mozilla.org/releases/mozilla-beta/rev/0027aa6624e4
Flags: needinfo?(ryanvm)
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.