The default bug view has changed. See this FAQ.

make python3 compatible



Release Engineering
10 months ago
9 months ago


(Reporter: aki, Assigned: aki)


Firefox Tracking Flags

(Not tracked)



(2 attachments)



10 months ago
I need to be able to sign things from scriptworker->signingscript.  Scriptworker and signingscript are using python3, so I had three options:

* call `python2`
* write my own python3 signing functions in signingscript.
* port to python3

The first seemed wrong to me.  The second is tempting and may still be the future when all signing is done through signingscript, but we still use signtool a lot of places.

I ported to py3, and got all the build/tools unittests green in both py27 and py35.  I left the bulk of the tools scripts untouched, as out of scope.


10 months ago
Blocks: 1253309

Comment 1

10 months ago
Created attachment 8762507 [details]

Output from `flake8 --exclude=vendor --max-line-length 160 > flake8.txt`
We still have work to do if we want all of build/tools to be python3 compatible and passing flake8.

Comment 2

10 months ago
Created attachment 8762512 [details] [diff] [review]

This patch:

* adds python3 compatibility to and lib/python, so it passes all unittests
* adds py35 to tox.ini and .travis.yml
* adds git config items to, since I was hitting email/name errors despite having a ~/.gitconfig
* switched to using resp.app_iter instead of resp.body in the signingserver tests, because the latter tried to do a b''.join() on a str or bytes object in py3, which doesn't seem like it would ever work
* chmods now require an explicit octal notation
* new version of pefile!
* requirements-py3.txt.  This is so we can install unreleased versions of poster and redo.

I had the biggest problems with the user auth tests in signingserver, and testApplyAndPushRebaseFails in hg.

All green:

This is a pretty big patch, but it's largely simple changes.  Let me know if you have questions, or if you want someone else to take a look?
Attachment #8762512 - Flags: review?(rail)

Comment 3

10 months ago
Oh: testApplyAndPushRebaseFails: I think the update has to happen *after* the strip, otherwise we can update to the wrong head, strip that head, and then create a new head when applying the new changesets.  Moving the update after the strip fixes this.

This was working with the incorrect behavior in py27 tox, but was broken in py27 nosetests when only running the file, and broken across the board in py35.  The new behavior works in all 4 scenarios.
Comment on attachment 8762512 [details] [diff] [review]

Review of attachment 8762512 [details] [diff] [review]:

In overall the patch looks good to me. To avoid extra churn with new deps, I suggest to drop six into lib/python/vendor and adjust the pth file. Otherwise we would need to fix at least release runner's puppet manifest to install six, and probably many other scripts using libs from tools
Attachment #8762512 - Flags: review?(rail) → review+


9 months ago
No longer blocks: 1253309

Comment 5

9 months ago is no longer working in py3 for me :(
I'm hitting poster multipart encoding errors.  I don't know how it ever worked, although I do know that I was editing my venv for debugging purposes.  That's a likely culprit as to why worked then but no longer works.

For the short term, I created a py2 venv to run signtool from build-tools.
Later I can revisit fixing up poster or rewriting signtool with aiohttp.

Comment 6

9 months ago

py3 compatible after I ported it to use requests; 65% test coverage.
Last Resolved: 9 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.