-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC #218: Python vendoring #218
base: master
Are you sure you want to change the base?
Conversation
@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. |
This generally seems fine to me, and looks like a significant improvement over the status quo. I have the following comments:
|
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.
Per web-platform-tests/wpt#49752 (comment) this all looks like pre-existing flakiness to me. |
@Ms2ger suggested I file an RFC for this.
Rendered.
Implementation: web-platform-tests/wpt#49752.