2011162 - Failure when running "./mach wpt --product safari": virtualenv 20.36.1 has requirement filelock<4,>=3.20.1; python_version >= "3.10", but you have filelock 3.18.0.

Status: RESOLVED FIXED

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

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Error running mach:

    mach wpt --product safari --webdriver-binary /System/Cryptexes/App/usr/bin/safaridriver testing/web-platform/tests/webdriver/tests/bidi/browser/set_client_window_state

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.
You can invoke ``./mach busted`` to check if this issue is already on file. If it
isn't, please use ``./mach busted file wpt`` to report it. If ``./mach busted`` is
misbehaving, you can also inspect the dependencies of bug 1543241.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

Traceback (most recent call last):
  File "/Users/henrik/code/gecko/python/mach/mach/main.py", line 390, in _run
    return Registrar._run_command_handler(
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
        handler,
        ^^^^^^^^
    ...<4 lines>...
        **vars(args.command_args),
        ^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/Users/henrik/code/gecko/python/mach/mach/registrar.py", line 124, in _run_command_handler
    result = fn(instance, **kwargs)
  File "/Users/henrik/code/gecko/testing/web-platform/mach_commands.py", line 540, in run_wpt
    return run_web_platform_tests(command_context, **params)
  File "/Users/henrik/code/gecko/testing/web-platform/mach_commands.py", line 528, in run_web_platform_tests
    return wpt_runner.run(logger, **params)
           ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
  File "/Users/henrik/code/gecko/testing/web-platform/mach_commands_base.py", line 74, in run
    kwargs = self.setup.kwargs_wptrun(kwargs)
  File "/Users/henrik/code/gecko/testing/web-platform/mach_commands.py", line 188, in kwargs_wptrun
    self.virtualenv_manager.install_pip_requirements(
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
        path, require_hashes=False
        ^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/Users/henrik/code/gecko/python/mach/mach/site.py", line 853, in install_pip_requirements
    raise InstallPipRequirementsException(
    ...<3 lines>...
    )
mach.site.InstallPipRequirementsException: As part of validation after installing "/Users/henrik/code/gecko/testing/web-platform/tests/tools/wptrunner/requirements.txt" into the "wpt" site, the site appears to contain installed packages that are incompatible with each other.

virtualenv 20.36.1 has requirement filelock<4,>=3.20.1; python_version >= "3.10", but you have filelock 3.18.0.

It's not related to testing/web-platform/tests/tools/wptrunner/requirements.txt given that it fails as well when I remove all the entries. A quick search shows that the hard requirement actually comes from third_party/python/requirements.txt.

Alex, could we maybe bump this requirement?

Comment 1

[Alex Hochheiden [:ahochheiden] [he/him]]

filelock is used by the mach site, and the mach site is tied to minimum python version 3.9, so we can't bump the vendored version until we can bump the minimum Python version to 3.10 or greater, which won't be for a while.

I don't have an environment that lets me reproduce this at the moment, but where is this dependency coming from? It's probably transitive, but can we pin whatever is bringing this in to a lower version that doesn't need the latest filelock? Or better yet, update the dependency to an older version of filelock (<3.20).

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Alex, as it looks like this is caused by a recent change in virtualenv which bumped the filelock dependency to 3.20.1 for Python >= 3.10. This was required due to a fixed CVE.

As I can see when running ./mach wpt it installs me the most recent virtualenv version and running with Python 3.11 this pulls in this requirement:

 "filelock>=3.16.1,<4; python_version<'3.10'",
 "filelock>=3.20.1,<4; python_version>='3.10'",

And that is done when mach sets-up the virtual environment, but not by the wpt requirements:

~/.mozbuild/srcdirs/gecko-2e7aa7eaa6b9/_virtualenvs/wpt/lib/python3.11/site-packages/virtualenv-
20.36.1.dist-info/

I don't think that we want to lock everyone to Python 3.9 which is no longer supported.

Comment 3

[Alex Hochheiden [:ahochheiden] [he/him]]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+1] from comment #2)

I don't think that we want to lock everyone to Python 3.9 which is no longer supported.

To be clear, the minimum supported version is python 3.9, not everyone is locked to this version, the environment just has to be compatible with it (due to downstream builder constraints that we have no control over. Either we support them, or they can't build Firefox and we lose those users).

A particular site can set their own minimum requirements. Here upload-generated-sources.txt specifies python 3.10 as their minimum, to because a specific package they need requires a higher level, but that site and all of its packages must still must be backwards compatible with the minimum supported version of `python 3.9.

With the mach virtualenv stuff using filelock and it having to be backwards compatible with python 3.9 (because the mach site activates first, before the build site) that doesn't leave me a whole lot of options here.

I can:

• Rip out filelock from the site.py code (and regress what it fixed), then unvendor filelock to remove the dependency conflict for the wpt site.
• Implement my own custom file locking for site.py, then unvendor filelock to remove the dependency conflict for the wpt site.

There might be another possible workaround, I'll have to think about it, but both of the first two options aren't great...

Comment 4

[Alex Hochheiden [:ahochheiden] [he/him]]

filelock is being used in more places than I thought, so option 1 of ripping out what's using it in-tree is not viable.

Comment 5

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2011162 - Temporarily exclude `filelock` from the `mach` site and use a wrapper to avoid dependency conflicts r?#build

This is essentially reverting D235335 (with some refactoring) so that we can work around a filelock dependency constraint.

Comment 6

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2011162 - Remove vendored `filelock` dependency r?#build,ahal

Comment 7

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2011162 - Fail on site lock timeout instead of continuing r?#build

Comment 8

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2011162 - Add `-r` include support to lockfile `requirements.txt` parsing r?#build,ahal

Comment 9

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2011162 - Strip inline comments when parsing `requirements.txt` for lockfiles r?#build,ahal

Comment 10

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 2011162 - Bump required `tox` version to `3.28.0` r?ahal

Removes a conflict in the wpt site with one of the requirements.txt files,
and we may as well keep the one in lint.txt consistent.

Comment 11

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

The severity field is not set for this bug.
:ahochheiden, could you have a look please?

For more information, please visit BugBot documentation.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/9641d7f73576
https://hg.mozilla.org/integration/autoland/rev/8d0f15225315
Replace vendored `filelock` with `mach.filelock` r=firefox-build-system-reviewers,sergesanspaille,mozperftest-reviewers,sparky
https://github.com/mozilla-firefox/firefox/commit/07e9dc124eba
https://hg.mozilla.org/integration/autoland/rev/8d0ad71cc69a
Fail on site lock timeout instead of continuing r=firefox-build-system-reviewers,sergesanspaille
https://github.com/mozilla-firefox/firefox/commit/ba4111b9132b
https://hg.mozilla.org/integration/autoland/rev/6e0397e5de7c
Remove vendored `filelock` dependency r=ahal,mach-reviewers
https://github.com/mozilla-firefox/firefox/commit/592072308d03
https://hg.mozilla.org/integration/autoland/rev/7b6b7c80e7ae
Add `-r` include support to lockfile `requirements.txt` parsing r=firefox-build-system-reviewers,sergesanspaille
https://github.com/mozilla-firefox/firefox/commit/738a09ac7c39
https://hg.mozilla.org/integration/autoland/rev/5b3e2f548943
Strip inline comments when parsing `requirements.txt` for lockfiles r=firefox-build-system-reviewers,sergesanspaille
https://github.com/mozilla-firefox/firefox/commit/e6289356f976
https://hg.mozilla.org/integration/autoland/rev/97a8d9919b2a
Bump required `tox` version to `3.28.0` r=ahal

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/8d0f15225315
https://hg.mozilla.org/mozilla-central/rev/8d0ad71cc69a
https://hg.mozilla.org/mozilla-central/rev/6e0397e5de7c
https://hg.mozilla.org/mozilla-central/rev/7b6b7c80e7ae
https://hg.mozilla.org/mozilla-central/rev/5b3e2f548943
https://hg.mozilla.org/mozilla-central/rev/97a8d9919b2a

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c949f90e8ba2
https://hg.mozilla.org/integration/autoland/rev/70da4cd8c8c2
Revert "Bug 2011162 - Bump required `tox` version to `3.28.0` r=ahal" as requested by dev. DONTBUILD

Comment 15

[Atila Butkovits]

Backed out as requested: https://hg.mozilla.org/integration/autoland/rev/70da4cd8c8c2

Comment 16

[Cosmin Sabou [:CosminS]]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/70da4cd8c8c2

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/716142be25c4
https://hg.mozilla.org/integration/autoland/rev/3aee25ff9374
Temporarily exclude `filelock` from the `mach` site and use a wrapper to avoid dependency conflicts r=firefox-build-system-reviewers,sergesanspaille,mozperftest-reviewers,sparky
https://github.com/mozilla-firefox/firefox/commit/39d76f57bbfa
https://hg.mozilla.org/integration/autoland/rev/c57730673a94
Fail on site lock timeout instead of continuing r=firefox-build-system-reviewers,sergesanspaille
https://github.com/mozilla-firefox/firefox/commit/b9e930df9bdb
https://hg.mozilla.org/integration/autoland/rev/5d3c3266a2fd
Add `-r` include support to lockfile `requirements.txt` parsing r=firefox-build-system-reviewers,sergesanspaille
https://github.com/mozilla-firefox/firefox/commit/19d8e7bd5c51
https://hg.mozilla.org/integration/autoland/rev/605067acbb42
Strip inline comments when parsing `requirements.txt` for lockfiles r=firefox-build-system-reviewers,sergesanspaille
https://github.com/mozilla-firefox/firefox/commit/901cc1c6d28d
https://hg.mozilla.org/integration/autoland/rev/7256d7e86c99
Bump required `tox` version to `3.28.0` r=ahal

Comment 18

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/3aee25ff9374
https://hg.mozilla.org/mozilla-central/rev/c57730673a94
https://hg.mozilla.org/mozilla-central/rev/5d3c3266a2fd
https://hg.mozilla.org/mozilla-central/rev/605067acbb42
https://hg.mozilla.org/mozilla-central/rev/7256d7e86c99