Closed Bug 1347949 Opened 4 years ago Closed 4 years ago
Add six to marionette requirements file
59 bytes, text/x-review-board-request
This is now required for wptserve, so we must ensure that it's installed.
Comment on attachment 8848117 [details] Bug 1347949 - Require six >= 1.8 for marionette tests, https://reviewboard.mozilla.org/r/121084/#review123026 If that package is required for wptserve we should add it to install_requires in setup.py for wptserve. Otherwise wptserve will still be broken everywhere else.
Attachment #8848117 - Flags: review?(hskupin) → review-
So the underlying issue here is that the mozharness marionette script is forcing a two-pass installation of the packages. I digged into the history of mozharness and found out that it got implemented on bug 908356. With the two-pass option we actually allow the installation of packages without having to specify them in the correct order. But that has the side-effect that also `--no-deps` is used. I'm not confident right now if we want to allow installing of remote packages at this point, even if those are hosted on our internal pypi mirror. Personally I would be more in favor of vendoring this package into /python/ as what we do with all the others. And six seems to be a good one to get started with the Python2->3 transition at some point. But I can see the complexity of getting it added. Joel, and David, if that change is fine for us we can go ahead. I just want to have some backing before r+ing the patch. Thanks.
To be clear the first thing I tried was packaging up six in tests.common.zip and installing that from an explicit path. It failed horribly on linux where six already got installed from some other requriements file, and causes pip to go into an infinite loop trying to uninstall the existing version of six. Since pip is cearly available in our pypi mirror (and has to be fock mochitest) and requirements_marionette.py is inherently CI specific (since it hardcodes paths from .zip files), I think this patch makes sense.
Another option would be to simply add the single six.py file to the wptserve repository. That's something a lot of available packages are already doing, and which is one option the author is also suggesting. For example urllib3 is doing that which is packaged by the requests package: https://dxr.mozilla.org/mozilla-central/source/python/requests/requests/packages/urllib3/packages/six.py So maybe this would be a better option for wptserve.
(In reply to Henrik Skupin (:whimboo) from comment #3) > > Personally I would be more in favor of vendoring this package into /python/ > as what we do with all the others. And six seems to be a good one to get > started with the Python2->3 transition at some point. But I can see the > complexity of getting it added. While it would be useful for the transition this would require a build peer sign off. Could this be vendored into WPTServe instead?
For reasons I don't understand, adding the requirement to wptserve seems to work: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a2a0673e19d3eb27717b3fe536a3c733e50d48f&selectedJob=84615890 Since that's effectively a change upstream I don't feel too bad about landing it with a=testonly in order to get the wpt update to stick, but if someone wants to r+ it that would be good.
Ah, that is my solution from yesterday when we talked about that on IRC. Interesting that this works even with the --no-deps options. So yeah, that is the solution I can safely give an r+. Thanks!
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/dd31c07e24e0 Add six to wptserve dependencies, r=whimboo
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/18c19b825b91 Add six to wptserve dependencies, r=whimboo
Backout by firstname.lastname@example.org: https://hg.mozilla.org/mozilla-central/rev/fbb6479621ba Backed out changeset 18c19b825b91
Backed out for neverending W(7) jobs on Windows 7 VM debug: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=f98b750bb85085a61341dc267f5ebf313820f960&filter-searchStr=2e76e93115e24a1caffbba81d961e84fb651fa8c https://hg.mozilla.org/mozilla-central/rev/fbb6479621ba
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/34c41d2a5227 Add six to wptserve dependencies, r=whimboo
Status: REOPENED → RESOLVED
Closed: 4 years ago → 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.