1654457 - Upgrade vendored virtualenv

Status: RESOLVED FIXED

(Firefox Build System :: General, task)

Description

[Mike Hommey [:glandium]]

I was trying to reproduce an automation test run on a mac under catalina, and it failed running virtualenv with an error that looks like the ones mentioned in https://github.com/pypa/virtualenv/issues/1561, which suggests it's fixed in later versions of virtualenv.

Comment 1

[Mike Hommey [:glandium]]

It failed on Mojave too, but not on High Sierra.

Comment 2

[Andrew Halberstadt [:ahal]]

Yes, upgrading would be nice. For context, there are two copies of virtualenv in-tree. One used by mozharness, and then the typical one under third_party/python. I'm not sure which copy of virtualenv you were using, but they are both below version 20. Bug 1408051 will get rid of the mozharness copy and should be landing soon (whether it sticks is another question).

So this should block on bug 1408051, though if it ends up being backed out due to virtualenv related issues, we could reverse the dependencies and fix this one first.

I won't have time to do this myself for awhile. I'll see if Hamzah is interested in taking this one after 1408051 is wrapped up.

p.s moving to Firefox Build System :: General as the virtualenv dependency is much broader than just testing.

Comment 3

[Hamzah Akhtar]

Attachment: Bug 1654457 - Update virtualenv to latest release version

Comment 5

[Ricky Stewart]

Just checking in because bug 1660183 is making this more important: what's the status of this work. The review suggests that we're blocked on removing pipenv (bug 1659542, itself blocked on bug 1659539), but then we can proceed as planned with this. Is that correct?

Comment 6

[Hamzah Akhtar]

Yeah, you are correct, pipenv is blocking this.
Since virtualenv20 was one of the biggest refactors after virtualenv16 and was refactored into a full-fledged module. In venv16 there used to be virtualenv.py which used to be executed everytime a virtualenv was to be created, but in venv20 it was broken down into several different files as earlier it was difficult to maintain, because of that we could no longer call virtualenv.py and instead had to work on modifying the PYTHONPATH
So jezdez suggested that we use the venv zipapp file which is just a zip file that contains the total content of the virtualenv packages and script https://phabricator.services.mozilla.com/D84577#2640136.
So I think once we are done removing pipenv we can proceed with upgrading the virtualenv.

Comment 7

[Ricky Stewart]

Okay, pipenv is gone, this work can continue.

Comment 8

[Mike Hommey [:glandium]]

Attachment: Bug 1654457 - Update virtualenv to 20.0.31.

Comment 9

[Mike Hommey [:glandium]]

Attachment: Bug 1654457 - Use `pip list --format freeze` instead of `pip freeze`.

Newer versions of pip output something like:
certifi @ file:///builds/worker/workspace/build/tests/tools/wpt_third_party/certifi
rather than
certifi==2018.4.16

and the package_versions method doesn't know how to handle that.

Both old and new versions of pip, however, output the same thing for
pip list --format freeze, which is equivalent to the old
pip freeze --all output (which means we'll now list more packages,
essentially pip, setuptools and wheels).

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9dfb511b5dc3
Use `pip list --format freeze` instead of `pip freeze`. r=ahal
https://hg.mozilla.org/integration/autoland/rev/91f4934e4063
Update virtualenv to 20.0.31. r=firefox-build-system-reviewers,mhentges,rstewart

Comment 11

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9dfb511b5dc3
https://hg.mozilla.org/mozilla-central/rev/91f4934e4063

Comment 12

[fzhu]

Hi, may I know that whether this patch will be applied to ESR78 branch? If so, when will it happen? Thanks!

Comment 13

[Mitchell Hentges [:mhentges] 🦀]

Sure, I'll nominate this patch and its dependencies for ESR uplift.
Heads up that there's a lot of dependencies (see the regressions list!), so this might not land smoothly. Let's find out :)

Comment 14

[Mitchell Hentges [:mhentges] 🦀]

Comment on attachment 9179169 [details]
Bug 1654457 - Update virtualenv to 20.0.31.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Common platforms (e.g.: macOS Mojave) are unable to run some ./mach commands.
• User impact if declined: Harder for downstreams to perform builds on various platforms.
• Fix Landed on Version: 83
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Is a build patch, failures should be found in CI.
  Application users are not affected.
• String or UUID changes made by this patch:

Comment 15

[fzhu]

@Mitchell, great. I am planning to upgrade our browser version from esr68 to esr 78 due to Mac Big Sur release, so I have to rebuild my code from scratch.

Here is another bug I have ever encountered when I try to build the source from scratch:
https://bugzilla.mozilla.org/show_bug.cgi?id=1674162
Since the code change is not that large, I merged it by hand in my local codebase, not sure whether it can be merged as well, that'll be good for other ESR users.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

There's various conflicts along the way trying to graft this and dependencies to ESR78. Most of them are pretty trivial to rebase around, but I hit conflicts in bug 1670167 I wasn't sure how to deal with and feel it'd be better for someone who knows the build system better to handle them. Can you please try to create a rebased set of patches for uplift? If it makes it easier, we can graft them directly from a Try push rather than attaching various patches throughout all the different bugs.

Comment 17

[Mitchell Hentges [:mhentges] 🦀]

I've got a stack of rebased patches on try here (tip is id 829aaf14f1c5).
I'm running local Mac, Linux and Windows builds right now, I'll report the status in an hour or two.

A couple notes:

• I had to take some liberties with that last patch, just slorping the if PY2 bit and losing the rest.
• For my local macOS dev, I have a newer SDK, so I locally grafted this patch. I didn't include it in my try stack because theoretically it shouldn't be needed, since downstream can use an older SDK.
• There's issues around ./mach bootstrap and Big Sur that will be solved by this bug. However, this is workaroundable by not bootstrapping from the ESR78 checkout.
• There's a lot of uplifted patches here. If this is an excessive amount, please let me know! I'm not sure the rules around this :)

Comment 18

[Mitchell Hentges [:mhentges] 🦀]

Seems to be happy on my machines.
Should be good to go :RyanVM, let me know how it goes :)

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/try/rev/829aaf14f1c5 has a .orig file removal in it, can we clean up the patch stack to avoid introducing it in the first place?

Comment 20

[Mitchell Hentges [:mhentges] 🦀]

Heh, thought I had properly hg absorb'd the removal of that .orig file, but I suppose not.
Thanks for the good eye Ryan!

Comment 21

[Mitchell Hentges [:mhentges] 🦀]

Second try's the charm: a59f8db313ccb1a73b43a159b0d230da5da71388 should be a clean stack.

Comment 22

[Mitchell Hentges [:mhentges] 🦀]

CI is angry.
Update: I've made some progress, but CI still wants to write to /usr/lib/python2.7/sitecustomize.py. More work is needed.

Comment 23

[Mitchell Hentges [:mhentges] 🦀]

Update: I set up the stack anew (tip is 80667ddf82858373813b073cf650db60b0e642fc), and CI seems happy.
I've got successful local builds in Linux and Mac, and I'm expecting Windows to pass as well (running it locally overnight tonight).

The uplift stack I have here is the minimal stack so that this bug's patch is moved to ESR. If you're building locally with Big Sur, you may run into other issues fixed by other tickets:

• You may need 1686168 if you're on Mac and using system (XCode's) python instead of brew's python.
• You may need 1669471 so you don't see a bunch of DeprecationWarnings for the imp module.
• You may need all the patches in 1680802 if you're on Big Sur.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9179169 [details]
Bug 1654457 - Update virtualenv to 20.0.31.

Needed to better support running mach on newer macOS releases. Approved for 78.8esr.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/abb435d3a512

Comment 26

[fzhu]

Great to see it. I also found there was another fix which is also required by esr78:
https://bugzilla.mozilla.org/show_bug.cgi?id=1683929

Comment 27

[Mitchell Hentges [:mhentges] 🦀]

Comment on attachment 9179170 [details]
Bug 1654457 - Use pip list --format freeze instead of pip freeze.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed to be compatible with newer version of pip used after https://phabricator.services.mozilla.com/D83181
• User impact if declined: None
• Fix Landed on Version: 83
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It's a build-only patch
• String or UUID changes made by this patch:

Comment 28

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/ce5691cc777d