-
Notifications
You must be signed in to change notification settings - Fork 68
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
Requirements for Python dependency management #82
Comments
Here's an overview of how wptrunner deps work on the Chromium side:
Chromium's use of wptrunner has been broken by new dependencies in the past, but it happens infrequently. We are almost always "behind" on versions. That is, we don't react to every single minor version update of deps. Instead, we roll up to the latest version of a package when something breaks (we get a chance to intervene without breaking the CI because we roll |
Thanks @LukeZielinski, that's super helpful! Given "our own package server ... currently a manual process" would it be fair to say that it'd be an improvement if dependencies in WPT were updated rarely, on a very predictable schedule like quarterly? Right now we're landing pyup changes as they come in. And am I right to assume that moving to only installed dependencies would be better, to not have to worry about vendored+installed compatibility and which takes precedence? (It seems to me that in WPT, the vendored deps will take precedence. Oops?) |
web-platform-tests/wpt#24829 has some good discussion too. |
On the gecko side:
Basically for Gecko, at this time, moving toward more installed deps would be A Bad Thing, and in general we don't have loads of problems with the current setup where actually running tests can be done using entirely in-tree deps (at least when the tree is mozilla-central). |
I'd consider this an improvement yes. Mostly in terms of planning, so you could dedicate some time each quarter (or whatever) to get your deps figured out, and not have to scramble.
All-vendored or all-installed both seem better than the hybrid thing that exists today. One argument I could see against all-vendored deps is that it checks a whole pile of random code into the tree (pollutes codesearch results for instance). Although we've never tried to go down this path, so there could be technical challenges I'm not aware of. |
Thanks @jgraham, that's very helpful! In case you saw it, I want to be clear that the all-installed web-platform-tests/wpt#28796 isn't really the direction I'm hoping for here, that's was an exploration to figure out what the dependencies actually are. It sounds like a requirements from Gecko is that it should be easy import WPT and all of its dependencies so that everything needed is in-tree, and no network access is needed while running WPT in Gecko. Having precisely the dependencies that Gecko doesn't already have manually vendored into WPT meets that requirement. A rough sketch of the direction I am hoping for now:
|
So one reasonable goal here would be to remove
That would have the following properties:
|
Getting rid of
Yes!
I looked at the recent commits of everything in web-platform-tests/wpt#28801 and didn't find anything with local modifications. Only |
One concern here is startup latency; if every |
For WebKit, there's a few notable cases:
Historically, the goal was for at least At least from my point-of-view, I'd rather we avoid the status-quo where we have our tools adding our own versions of the packages to To note, I think
One problem is that some of the packages that wptrunner requires depend on compiled extensions (such as Pillow). We need to install them somewhere as a result. |
That's precisely one of the things I'd like to resolve here. When I run |
Commands we have left which don't use a venv:
And I believe what we have vendored that we directly use is:
Note that pywebsocket3 we have several patches on, and mozlog which we have more clearly forked (maybe? web-platform-tests/wpt#48948). Especially with |
I do very much wonder the value of having mozilla-tooltool-client/pytest/pytest-asyncio/websockets vendored actually makes sense — they're only used by wptrunner, and that's always had its own dependencies? That leaves atomicwrites and html5lib used by manifest, and h2 and pywebsocket3 used by wptserve. |
It's a little bit complicated by the fact that mozilla-central is in the unique position of having mozbase packages already in-tree, so we end up with all the deps required to run wptrunner available without depending on PyPI. So changes here do have a material impact for us. |
You also have all the other non-mozbase packages requires to run wptrunner, though? It's not like these are the first non-mozbase dependencies. |
Well the point is that in CI the only wpt-specific thing we install from a package is aioquic, and that's caused ongoing problems because we need to update the internal mirror which currently isn't a very streamlined process. For things like pytest, we have vendored copies already, but they aren't necessarily the same version we're using in wpt. So moving to vendoring ~nothing in wpt isn't impossible, but neither is it easy for us. |
To make concrete one of my motivations for caring here: I want to be able to import web-platform-tests/wpt#49034 is one possible path forward here (moving to importing everything from I guess we could do |
Hi @web-platform-tests/wpt-core-team!
Recently I have taken a closer look at how we track and install Python dependencies here in WPT. I wrote up two fairly detailed issues about what we're currently doing in web-platform-tests/wpt#28801 and web-platform-tests/wpt#28809.
I suspect that there's room to improve matters here, but want to gather requirements before I suggest something. But first, some problem I think are worth trying to solve:
./wpt run
(virtualenv is the additional dependency)git subtree
, and can't be merged like regular wpt PRsHere are the requirements I would consider for any change here, roughly in order of importance:
./wpt run
should need at most one tool in addition to Python (currently Virtualenv) but zero is even better.package-lock.json
orPipfile.lock
)I'm interested to hear if others have had trouble with dependencies, and if there are additional constraints/requirements here.
The text was updated successfully, but these errors were encountered: