Closed Bug 1347949 Opened 4 years ago Closed 4 years ago

Add six to marionette requirements file

Categories

(Testing :: Marionette, enhancement)

Version 3
enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jgraham, Assigned: jgraham)

References

Details

Attachments

(1 file)

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.
Flags: needinfo?(jmaher)
Flags: needinfo?(dburns)
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?
Flags: needinfo?(dburns)
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 james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd31c07e24e0
Add six to wptserve dependencies, r=whimboo
https://hg.mozilla.org/mozilla-central/rev/dd31c07e24e0
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Assignee: nobody → james
No longer blocks: 1348585
Depends on: 1348585
Flags: needinfo?(jmaher)
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/18c19b825b91
Add six to wptserve dependencies, r=whimboo
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/mozilla-inbound/rev/34c41d2a5227
Add six to wptserve dependencies, r=whimboo
https://hg.mozilla.org/mozilla-central/rev/34c41d2a5227
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Flags: needinfo?(james)
You need to log in before you can comment on or make changes to this bug.