Skip to content

Conversation

@gsnedders
Copy link
Member

@gsnedders gsnedders commented Jan 7, 2025

@Ms2ger suggested I file an RFC for this.

Rendered.

Implementation: web-platform-tests/wpt#49752.

@gsnedders gsnedders changed the title Add an RFC about vendoring our Python RFC #218: Python vendoring Jan 7, 2025
@jcscottiii
Copy link
Contributor

@WeizhongX I don't think this will impact Chromium CI but I thought I would double check with you.

@WeizhongX
Copy link
Contributor

@WeizhongX I don't think this will impact Chromium CI but I thought I would double check with you.

We might need to create vpython wheel for the vendor packages, but should be fine for us.

@jgraham
Copy link
Contributor

jgraham commented Jan 8, 2025

This generally seems fine to me, and looks like a significant improvement over the status quo. I have the following comments:

  • I think the risk of this affecting Mozilla's prioritization of improving our story around non-vendored deps is minimal. Most of the pressure there really comes from deps with binary components, which aren't changed either way by this proposal.
  • I think there is some risk that vendoring changes in the future in a way that breaks our use. It's specifically documented as being unstable and only designed to work with pip. I think it's a managable risk, but a real one.
  • There is also the risk that this breaks unshared tests using wpt, if they are doing custom path manipulations. That isn't any kind of blocker, but people should be aware, since it may affect syncing.
  • The implementation should be tested with full runs of wpt using the trigger branches, not just relying on CI jobs, since there's a risk of any test that does something custom with the Python path breaking.

@gsnedders
Copy link
Member Author

  • I think there is some risk that vendoring changes in the future in a way that breaks our use. It's specifically documented as being unstable and only designed to work with pip. I think it's a managable risk, but a real one.

This is fair, but it's essentially "we now use a tool to automate most of the work that may break in future" (and really it isn't a complex tool) versus "we don't really document how to vendor new things and don't really give people guidance on how to do it".

Like, the ultimate fallback is just directly doing what vendoring does:

    command = [
        "pip",
        "install",
        "--platform",
        "any",
        "-t",
        str(destination),
        "-r",
        str(requirements),
        "--no-compile",
        # We use --no-deps because we want to ensure that dependencies are provided.
        # This includes all dependencies recursively up the chain.
        "--no-deps",
    ]

Though this means we then miss things like "check we get all the licenses", and automatically applying patches, and potentially rewriting module names.

  • The implementation should be tested with full runs of wpt using the trigger branches, not just relying on CI jobs, since there's a risk of any test that does something custom with the Python path breaking.

Per web-platform-tests/wpt#49752 (comment) this all looks like pre-existing flakiness to me.

@gsnedders gsnedders merged commit 8e38522 into web-platform-tests:master Mar 10, 2025
@gsnedders gsnedders deleted the vendoring branch March 10, 2025 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants