1724274 - Lock/pin dependencies in centralized python dependency system

Status: NEW

(Firefox Build System :: Mach Core, enhancement, P2)

Description

[Mitchell Hentges [:mhentges] 🦀]

Pin transitive Python package versions to ensure that commands behave consistently.
Python "environment markers" will be needed to ensure cross-platform/version compatibility.

At the time of original investigation half a year ago, poetry seemed to be the best solution for implementing this.

Comment 1

[Mitchell Hentges [:mhentges] 🦀]

We're reaching the point where locking is required to port all the install_pip_requirements(...) usages.
Unfortunately, the newest version of pip-compile (6.2.0) still doesn't support environment markers. It looks like poetry is going to be the tool we use to generate lockfiles. Fortunately, poetry seems pretty slick and is already doing intelligent work around parallel installs, so hopefully performance won't be too rough. It looks like it is really effective at re-using known information with a cache, but that won't help us when we're running in clean containers in CI. I hope that the initialization time won't be too rough.

---

There's a couple things we need for this ticket:

• Re-generating a lockfile (requirements.txt) should change an existing lockfile as minimally as necessary to satisfy new requirements. So, if the requirements/constraints haven't changed, then the lockfile shouldn't either.
  • However, it should be possible to run a command to update lockfile packages to their newest versions.
• Lockfiles should be asserted to be up-to-date against their requirements files in CI - it should be a tier-2 (?) failure if a lockfile doesn't match its requirements definition.
• The locking system should be compatible with both packages installed from PyPI as well as vendored dependencies.
• [1725708] Assert that the lockfile is compatible with major platforms and Python versions

Comment 2

[Mitchell Hentges [:mhentges] 🦀]

Just finished a local proof-of-concept that translates a Mach requirements definition file into a poetry.lock and requirements.txt file. Awesome :)

Comment 3

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1724274: Add missing "pypi-optional" docs

These were missed when pypi-optional support was added.

Comment 4

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1724274: Don't allow Thunderbird requirements to include PyPI pkgs

Thunderbird can't currently define its own Mach commands, which means
that it doesn't define its own virtualenvs.

However, it does add some tweaks to the common virtualenv requirements
(for build scripts, I believe).

So, we should allow it to continue making those tweaks, but we should
remove the capability of specifying PyPI packages. This is needed for
lockfile-generation, since lockfiles are created and placed in the
Mozilla repo, and we don't want "conditional lockfiles" based on the
existence of a "comm" repo.

This will not regress existing Thunderbird expectations.

Depends on D124514

Comment 5

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1724274: Simplify VirtualenvManager

• Removes the unused call_setup() function
• Inlines functions that were internally called only once

Depends on D124517

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6c4c17f436fb
Add missing "pypi-optional" docs r=ahal
https://hg.mozilla.org/integration/autoland/rev/bfd986800e40
Don't allow Thunderbird requirements to include PyPI pkgs r=ahal
https://hg.mozilla.org/integration/autoland/rev/a6200969a92a
Simplify VirtualenvManager r=ahal

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/6c4c17f436fb
https://hg.mozilla.org/mozilla-central/rev/bfd986800e40
https://hg.mozilla.org/mozilla-central/rev/a6200969a92a

Comment 8

[Mathew Hodson]

Can this bug be closed? All the patches landed.

Comment 9

[Mitchell Hentges [:mhentges] 🦀]

Not yet, the bulk of the work associated with this bug hasn't been pushed to Phabricator at the moment.

Comment 10

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1724274: Move virtualenv dependency manifests to python/virtualenvs

This is needed for a few reasons:

• All mach commands can use virtualenvs, not just build-related
  commands, so the files don't make sense to be in build/.
• When locking is added, more files associated with virtualenvs will be
  added, and this will change will ease the related directory structure
  setup.
• This removes the need for a redundant "_virtualenv_packages" keyword
  as part of the manifest filenames.

Comment 11

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1724274: Downgrade vendored packages for poetry compatibility

importlib_metadata and packaging need to be older versions to be
compatible with the modern version of poetry.

Comment 12

[Mitchell Hentges [:mhentges] 🦀]

Attachment: Bug 1724274: [WILL-SQUASH] `./mach vendor python`

Depends on D141675

Comment 13

[Mitchell Hentges [:mhentges] 🦀]

Attachment: WIP: Bug 1724274 - Implement lockfiles for all `mach` virtualenvs

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8b2534181c08
Move virtualenv dependency manifests to python/virtualenvs r=ahal
https://hg.mozilla.org/integration/autoland/rev/5131c4696320
Downgrade vendored packages for poetry compatibility r=ahal

Comment 15

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/8b2534181c08
https://hg.mozilla.org/mozilla-central/rev/5131c4696320

Comment 16

[Mitchell Hentges]

Attachment: lockfile-patches

The patch stack for the lockfile work here.
Some try tasks are still failing, but all necessary functionality is covered by these patches.
Good luck!

Comment 17

[Andrew Halberstadt [:ahal]]

Attachment: WIP: Bug 1724274 - Revendor WPT deps (`html5lib` and `webencodings`) as distributions

The upcoming lockfile-generation logic has a greater understanding of
package dependencies, including knowing that some vendored dependencies
fulfill package requirements of other dependencies.

Lockfiles are installed with pip's --require-hashes flag, which
isn't compatible with "setup.py"-defined dependencies. So, we include
our "setup.py" packages as //optional// dependencies in our Poetry
config, so that they influence dependency selection (and cause proper
errors if there's a conflict), yet aren't included in the generated
requirements.txt file.

Unfortunately, html5lib is the one vendored package that's a "fly in
the ointment": it's depended on by an external dependency (poetry
itself, ironically), and therefore it must be included in the created
requirements.txt.

(As an aside: we can't just parse out the html5lib name and package
number and use the PyPI.org version, as //the vendored html5lib
version doesn't exist on PyPI.org//.)

However, we can solve this problem by re-vendoring html5lib as
a //distribution// (.dist-info) rather than as source (setup.py),
thereby allowing --require-hashes to be happy, and sidestepping the
issue.

Since html5lib depends on webencodings, it too needed to change
to have its distribution vendored instead of its source.

Comment 18

[Andrew Halberstadt [:ahal]]

Attachment: WIP: Bug 1724274 - Make `mod_pywebsocket` directory name match package

For the upcoming lockfile system, each tracked package must be
registered with poetry: usually with a package name and version.
For vendored packages that just have source details (a setup.py file,
but no distribution details like a PKG-INFO/.dist-info directory),
we don't have any files to parse - so, we get the name from the
containing directory.

Unluckily for us, poetry, enforces that this name matches the real
package name that setup.py spits out.

All of our vendored packages are compliant with this requirement ...
except pywebsocket3. Accordingly, rename pywebsocket3's directory to
mod_pywebsocket to match the package name.

Depends on D146075

Comment 19

[Andrew Halberstadt [:ahal]]

Attachment: WIP: Bug 1724274 - Revendor `h2`, `hpack`, and `hyperframe` as distributions

Slightly speed up lockfile generation by not having to generate h2
package details with setup.py.

Revender hpack and hyperframe as distributions as well so
that pip check is content with vendored packages.

Depends on D146077

Comment 20

[Andrew Halberstadt [:ahal]]

Attachment: WIP: Bug 1724274 - Vendor `pkginfo` at version `1.9.4`

pkginfo is needed to parse vendored Python package information at Mach
init time.

Depends on D146078

Comment 21

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:ahal, maybe it's time to close this bug?
For more information, please visit auto_nag documentation.

Comment 22

[Andrew Halberstadt [:ahal]]

Hey Alex, are you planning to work through these patches and get them landed? Or are we already on a different path we should close this out? I don't think I'm going to have bandwidth to work on this.

Comment 23

[Alex Hochheiden [:ahochheiden] (needinfo? me)]

It's on the roadmap for this half.

Comment 24

[Alex Hochheiden [:ahochheiden] (needinfo? me)]

Attachment: Bug 1724274 - Regenerate lockfile with Python 3.7 as the minimum version r?#build

Depends on D171151

Comment 25

[Phabricator Automation]

Comment on attachment 9321147 [details]
Bug 1724274 - Regenerate lockfile with Python 3.7 as the minimum version r?#build

Revision D171628 was moved to bug 1734402. Setting attachment 9321147 [details] to obsolete.

Comment 26

[Alex Hochheiden [:ahochheiden] (needinfo? me)]

Attachment: WIP: Bug 1724274 - Vendor all wpt deps in python/third_party

Depends on D182280