The default bug view has changed. See this FAQ.

make signtool.py python3 compatible

RESOLVED FIXED

Status

Release Engineering
Tools
RESOLVED FIXED
10 months ago
9 months ago

People

(Reporter: aki, Assigned: aki)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

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 signtool.py`
* write my own python3 signing functions in signingscript.
* port signtool.py 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 signtool.py 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.
(Assignee)

Updated

10 months ago
Blocks: 1253309
(Assignee)

Comment 1

10 months ago
Created attachment 8762507 [details]
flake8.txt

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.
(Assignee)

Comment 2

10 months ago
Created attachment 8762512 [details] [diff] [review]
build-tools-py3.diff

This patch:

* adds python3 compatibility to signtool.py and lib/python, so it passes all unittests
* adds py35 to tox.ini and .travis.yml
* adds git config items to init_gitrepo.sh, 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: https://github.com/mozilla/build-tools/pull/17

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)
(Assignee)

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 test_util_hg.py file, and broken across the board in py35.  The new behavior works in all 4 scenarios.
Comment on attachment 8762512 [details] [diff] [review]
build-tools-py3.diff

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+
(Assignee)

Updated

9 months ago
No longer blocks: 1253309
(Assignee)

Comment 5

9 months ago
signtool.py 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.
(Assignee)

Comment 6

9 months ago
https://pypi.python.org/pypi/signtool
https://github.com/escapewindow/signtool

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