1345274 - marionette-harness sdist package misses key files for secure connections

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Eric Rahm [:erahm]]

STR:

1. Open windows shell: start-shell-msvc2015-x64.bat
2. Install marionette-harness and run a dummy test:

> mkdir bug_test && cd bug_test
> virtualenv venv
> source venv/Scripts/activate
> touch test_hosed.py
> pip install marionette-harness mozdownload mozinstall
> mozdownload --type daily && mozinstall *.exe
> marionette --binary *mozilla-central-firefox-55/core/firefox.exe test_hosed.py

Example failure:

> $ marionette --binary *-mozilla-central-firefox-55/core/firefox.exe test_hosed.py
> Using workspace for temporary data: "c:\dev\test-install\atsy-test"
> mozversion application_buildid: 20170307030205
> mozversion application_changeset: b7e42143bbbc9dc3e5c05bd1e93b6485ce1d0ad4
> mozversion application_display_name: Nightly
> mozversion application_id: {ec8030f7-c20a-464f-9b0e-13a3a9e97384}
> mozversion application_name: Firefox
> mozversion application_remotingname: firefox
> mozversion application_repository: https://hg.mozilla.org/mozilla-central
> mozversion application_vendor: Mozilla
> mozversion application_version: 55.0a1
> mozversion platform_buildid: 20170307030205
> mozversion platform_changeset: b7e42143bbbc9dc3e5c05bd1e93b6485ce1d0ad4
> mozversion platform_repository: https://hg.mozilla.org/mozilla-central
> mozversion platform_version: 55.0a1
> Application command: 2017-03-07-03-02-05-mozilla-central-firefox-55/core/firefox.exe -no-remote -marionette -profile c:\users\ericra~1\appdata\local\temp\tmpnjtghq.mozrunner
> Profile path is c:\users\ericra~1\appdata\local\temp\tmpnjtghq.mozrunner
> Starting fixture servers
> Process ServerProxy-2:
> Traceback (most recent call last):
>   File "c:\mozilla-build\python\Lib\multiprocessing\process.py", line 258, in _bootstrap
>     self.run()
>   File "c:\dev\test-install\atsy-test\venv\lib\site-packages\marionette_harness-4.0.0-py2.7.egg\marionette_harness\runner\serve.py", line 72, in run
>     server = self.init_func(*self.init_args, **self.init_kwargs)
>   File "c:\dev\test-install\atsy-test\venv\lib\site-packages\marionette_harness-4.0.0-py2.7.egg\marionette_harness\runner\serve.py", line 149, in https_server
>     **kwargs)
>   File "c:\dev\test-install\atsy-test\venv\lib\site-packages\marionette_harness-4.0.0-py2.7.egg\marionette_harness\runner\httpd.py", line 77, in __init__
>     key_file=ssl_key)
>   File "c:\dev\test-install\atsy-test\venv\lib\site-packages\wptserve-1.4.0-py2.7.egg\wptserve\server.py", line 403, in __init__
>     assert os.path.exists(key_file)
> AssertionError

Comment 1

[Eric Rahm [:erahm]]

This blocks atsy [1] which I plan to use to evaluate e10s-multi memory usage.

[1] https://github.com/EricRahm/atsy

Comment 2

[Eric Rahm [:erahm]]

:ato it looks like at least lower down this is code you've most recently worked on.

Comment 3

[Eric Rahm [:erahm]]

This repros on OSX, but not Linux for me.

Comment 4

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

See also bug 1321517. How did you install marionette? Via "setup.py install"? I would assume that the package is missing the files.

Comment 5

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

Indeed, the sdist package misses those files, while they are present for the wheel package. AFAIK on Linux the wheel package is the default, so it's not visible on that platform but others?

Comment 6

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

An interesting fact:

> If using the setuptools-specific include_package_data argument, files specified by package_data will not be automatically added to the manifest unless they are listed in the MANIFEST.in file.)

In our case the key files are only listed in setup.py under package_data. As such they won't be automatically added. But even when I add the files to MANIFEST.in I do not see the files being present when running "python setup.py sdist". Only when I comment out include_package_data it works.

So I would propose to remove the include_package_data argument, which will make it work for both distribution methods which are sdist and bdist_wheel.

Comment 7

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

(In reply to Henrik Skupin (:whimboo) from comment #6)
> So I would propose to remove the include_package_data argument, which will
> make it work for both distribution methods which are sdist and bdist_wheel.

Ups. No, we actually cannot do this. It would mean that the www sub folder is part of the package but any files are not installed when running `pip install package`.

Does anyone have a good idea?

Comment 8

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

I think I found the solution. Given that both certificate files are not actually source code we should move them out of the runner folder into a new top-level certificates folder. With that we can easily get them installed similar to the www folder.

Comment 9

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

Attachment: Bug 1345274 - marionette-harness sdist package misses certificate files.

test.key and test.cert are data files and should not be located next to the
source code inside a package. Instead move them into their own top-level folder
where they can be included via MANIFEST.in.

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

Comment 10

[Andreas Tolfsen ❲:ato❳]

(In reply to Henrik Skupin (:whimboo) from comment #5)
> Indeed, the sdist package misses those files, while they are present for the
> wheel package. AFAIK on Linux the wheel package is the default, so it's not
> visible on that platform but others?

Ugh, Python packaging sucks.

(In reply to Henrik Skupin (:whimboo) from comment #8)
> I think I found the solution. Given that both certificate files are not
> actually source code we should move them out of the runner folder into a new
> top-level certificates folder. With that we can easily get them installed
> similar to the www folder.

Sounds reasonable.

Comment 11

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8844853 [details]
Bug 1345274 - marionette-harness sdist package misses certificate files.

https://reviewboard.mozilla.org/r/118178/#review120028

Comment 12

[Andreas Tolfsen ❲:ato❳]

Note that this is likely also a problem if you try using the sdist package of wptrunner (https://github.com/w3c/wptrunner/).  It uses the same setup with regards to certificate files, I believe.

Comment 13

[Eric Rahm [:erahm]]

Until this gets released the following can be used instead:

> cd $MOZILLA_CENTRAL
> pip install -e testing/marionette/client/ testing/marionette/harness/

Since it's setup in dev mode we don't have to worry about packaging issues. Clearing [e10s-multi] as I have a workaround.

Comment 14

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

(In reply to Andreas Tolfsen ‹:ato› from comment #12)
> Note that this is likely also a problem if you try using the sdist package
> of wptrunner (https://github.com/w3c/wptrunner/).  It uses the same setup
> with regards to certificate files, I believe.

Maybe. This repo doesn't contain certificate files which are necessary bug other data files. So it depends on if those would be required when the package gets installed or not. Anyway, it's something the maintainer would have to check.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f5644dde03c9
marionette-harness sdist package misses certificate files. r=ato

Comment 16

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

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