1347949 - Add six to marionette requirements file

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, enhancement)

Description

[James Graham [:jgraham]]

This is now required for wptserve, so we must ensure that it's installed.

Comment 1

[James Graham [:jgraham]]

Attachment: Bug 1347949 - Require six >= 1.8 for marionette tests,

Review commit: https://reviewboard.mozilla.org/r/121084/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/121084/

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 4

[James Graham [:jgraham]]

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.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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.

Comment 6

[David Burns :automatedtester]

(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?

Comment 7

[James Graham [:jgraham]]

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.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

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!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dd31c07e24e0
Add six to wptserve dependencies, r=whimboo

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/dd31c07e24e0

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/18c19b825b91
Add six to wptserve dependencies, r=whimboo

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/18c19b825b91

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/fbb6479621ba
Backed out changeset 18c19b825b91

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/34c41d2a5227
Add six to wptserve dependencies, r=whimboo

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/34c41d2a5227