More changes for Python3 compatibility #24512

marmeladema · 2019-10-20T22:53:21Z

Following #24435 for #23607 here are even more changes to be compatible with Python3.
I managed to get ./mach build properly work with Python3 but test-tidy does not work yet because of a lot of problems in web-platform-tests which i will have to deal with at some point.

Because flake8 appears to be incompatible with the distro package in some way, i had to change to way we call flake8 (which was deprecated anyway). With this change, we should be able to update flake8 to a recent version but subprocess can be surprising on Windows platform so i'd like someone to try out those changes.

./mach build -d does not report any errors
./mach test-tidy does not report any errors

This is done in order to be compatible with Python3

highfive · 2019-10-20T22:53:26Z

Heads up! This PR modifies the following files:

@wafflespeanut: python/servo/bootstrap.py, python/requirements.txt, python/servo/command_base.py, python/tidy/servo_tidy/tidy.py, python/mach_bootstrap.py and 1 more
@jgraham: tests/wpt/update/fetchlogs.py, tests/wpt/grouping_formatter.py, tests/wpt/manifestupdate.py, tests/wpt/update/github.py, tests/wpt/update/upstream.py

…orm module platform.linux_distribution() is deprecated since Python 3.5 and will be removed with Python 3.8.

marmeladema · 2019-10-21T10:54:59Z

@jdm i don't know how to add you as reviewer so i thought i would just ping you 👍

JacobAWilkins

Very nice

saschanaz · 2019-10-21T12:32:22Z

r? @jdm

i don't know how to add you as reviewer

This is how 😁

jdm · 2019-10-21T12:45:01Z

r? @SimonSapin
Honestly I think Simon has more experience reviewing python3 than I do.

marmeladema · 2019-10-21T12:54:49Z

r? @SimonSapin
Honestly I think Simon has more experience reviewing python3 than I do.

Yeah i did not imply to remove @SimonSapin from reviewers, just wanted to add you

SimonSapin

I’d like to also get some testing so we don’t accidentally regress this. If ./mach build is supposed to work on Python 3 now, please change it to python3 mach build here, for example:

servo/etc/taskcluster/decision_task.py

Line 180 in 5fb941d

./mach build --dev

python/tidy/servo_tidy/tidy.py

marmeladema · 2019-10-22T07:28:12Z

I’d like to also get some testing so we don’t accidentally regress this. If ./mach build is supposed to work on Python 3 now, please change it to python3 mach build here, for example:

servo/etc/taskcluster/decision_task.py

Line 180 in 5fb941d

./mach build --dev

It passes on a branch based on this one with a few more tweaks. We can either merge this one, and i'll come back in a few days with the everything sorted out to build with python3 or i add the remaining bits in this one.

Also about just changing ./mach build to python3 mach build, i wonder the effect on the subsequent commands that will run with python2 like ./mach test-tidy --no-progress --self-test

SimonSapin · 2019-10-22T07:31:20Z

I’ll leave it up to you whether you prefer to add a commit in this PR for those few more tweaks (maybe they’re already ready) or if you prefer not to block landing this PR and make a new one later.

I don’t expect that a Python 3 command followed by a Python 2 command would cause any trouble, they’re independent processes. Finding out for sure is what CI is for!

marmeladema · 2019-10-22T07:38:51Z

I’ll leave it up to you whether you prefer to add a commit in this PR for those few more tweaks (maybe they’re already ready) or if you prefer not to block landing this PR and make a new one later.

Then i would prefer merging this now and come back later with the remaining bits. My other branch actually only check that you have python3, not python2 anymore (even though the code remains compatible) and will take some time to make python version detection compatible as its not ready yet.

I don’t expect that a Python 3 command followed by a Python 2 command would cause any trouble, they’re independent processes. Finding out for sure is what CI is for!

We might have to remove python/_virtualenv between python3 and python2 though

SimonSapin · 2019-10-22T08:34:23Z

Oh yeah, there’s virtualenv. It may be worth having it at a different location based on the Python version.

This also pin the six module version to 1.12 in order to be sure six.ensure_text is available.

This is done in order to be compatible with Python3

SimonSapin · 2019-10-22T11:23:13Z

Looks good, thanks!

@bors-servo r+

bors-servo · 2019-10-22T11:23:14Z

📌 Commit 7fdd0b9 has been approved by SimonSapin

bors-servo · 2019-10-22T11:56:32Z

⌛ Testing commit 7fdd0b9 with merge a1d4342...

More changes for Python3 compatibility Following #24435 for #23607 here are even more changes to be compatible with Python3. I managed to get `./mach build` properly work with Python3 but `test-tidy` does not work yet because of a lot of problems in `web-platform-tests` which i will have to deal with at some point. Because flake8 appears to be incompatible with the distro package in some way, i had to change to way we call flake8 (which was deprecated anyway). With this change, we should be able to update flake8 to a recent version but subprocess can be surprising on Windows platform so i'd like someone to try out those changes. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors

bors-servo · 2019-10-22T13:34:03Z

💔 Test failed - status-taskcluster

CYBAI · 2019-10-22T13:39:18Z

@bors-servo retry

network issue in magicleap build

marmeladema · 2019-10-22T15:09:05Z

@bors-servo retry

* network issue in magicleap build

I don't think the retry worked?

bors-servo · 2019-10-22T15:09:06Z

@marmeladema: 🔑 Insufficient privileges: not in try users

jdm · 2019-10-22T15:12:14Z

It does work; there is a queue at https://build.servo.org/homu/queue/servo.

bors-servo · 2019-10-22T17:48:52Z

⌛ Testing commit 7fdd0b9 with merge 3e7a0bf...

More changes for Python3 compatibility Following #24435 for #23607 here are even more changes to be compatible with Python3. I managed to get `./mach build` properly work with Python3 but `test-tidy` does not work yet because of a lot of problems in `web-platform-tests` which i will have to deal with at some point. Because flake8 appears to be incompatible with the distro package in some way, i had to change to way we call flake8 (which was deprecated anyway). With this change, we should be able to update flake8 to a recent version but subprocess can be surprising on Windows platform so i'd like someone to try out those changes. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors

bors-servo · 2019-10-22T20:15:12Z

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: SimonSapin
Pushing 3e7a0bf to master...

HSTS & CA updates; Fix Debian bootstrap; Default to https on Android, too. - Updated HSTS Preload list using ./mach update-hsts-preload - Updated CA [database](https://ccadb-public.secure.force.com/mozilla/IncludedCACertificateReportPEMCSV) using etc/cert_generator.sh. - No additions. - [bug 1552374](https://bugzilla.mozilla.org/show_bug.cgi?id=1552374) removed Certinomis - Root CA - [bug 1574670](https://bugzilla.mozilla.org/show_bug.cgi?id=1574670) removed Class 2 Primary CA and Deutsche Telekom Root CA 2 - [bug 1586081](https://bugzilla.mozilla.org/show_bug.cgi?id=1586081) removed GlobalSign Extended Validation CA - SHA256 - G2 - Updated Public Suffix list using ./mach update-pub-domains - Default to https on Android, too. Desktop was done in #23363. Keep http:// after `android.webkit.URLUtil.guessUrl()` url sanitization only if the user explicitly typed it into the address bar. Small warning: I don't have an Android build environment yet, but still wanted to try to contribute these two lines. - Fixed `./mach bootstrap` for Debian Testing. Regression from #24512. After `pip install distro` (#24561) I finally got `Exception: mach bootstrap does not support Debian GNU/Linux, please file a bug`. distrib and version were "debian" and "bullseye/sid" before, now they are "debian gnu/linux" and "testing". - Use HSTS preload list for private HttpState, too. Private HttpState currently [creates an empty HSTS list](https://github.com/servo/servo/blob/f7fb130a2a21ae19cf0996251134ad23fea9068d/components/net/http_loader.rs#L93-L95). In contrast, regular HttpState first creates HstsList from Preload list and then adds further HSTS entries previously saved on disk. ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because ___

marmeladema added 3 commits October 20, 2019 23:43

Replace call to execfile(...) by call to exec(compile(open(...)))

865d737

This is done in order to be compatible with Python3

Use relative import statements to be compatible with Python3

bed6f2d

Pin six module version to 1.12

0aa6314

highfive assigned SimonSapin Oct 20, 2019

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Oct 20, 2019

Use linux_distribution() from distro package instead of builtin platf…

4d6f28d

…orm module platform.linux_distribution() is deprecated since Python 3.5 and will be removed with Python 3.8.

marmeladema force-pushed the issue-23607/compat branch 2 times, most recently from 0e1352c to c6e4d7a Compare October 20, 2019 23:16

JacobAWilkins approved these changes Oct 21, 2019

View reviewed changes

highfive assigned jdm and unassigned SimonSapin Oct 21, 2019

highfive assigned SimonSapin and unassigned jdm Oct 21, 2019

SimonSapin reviewed Oct 22, 2019

View reviewed changes

python/tidy/servo_tidy/tidy.py Outdated Show resolved Hide resolved

marmeladema added 7 commits October 22, 2019 10:15

Fix call to flake8 broken when using distro module during test-tidy

ff50cdb

Use six.text_type instead of str to check instance type

7e9ce2d

Convert git sha hash to text to be compatible with Python3

af7e4d6

This also pin the six module version to 1.12 in order to be sure six.ensure_text is available.

Use 'byte' regexp pattern to match on 'byte' string

45817ac

This is done in order to be compatible with Python3

tests/wpt/update: Fix print statements to be compatible with Python3

179caed

tests/wpt/update: use urllib module from six package

d8190a7

tests/wpt/update: remove useless import statement

0ea1af8

marmeladema added 3 commits October 22, 2019 10:16

Use input() from six instead of raw_input to be compatible with Python3

c0a9d39

tests/wpt/update: Use input() from six instead of raw_input()

f51ae46

tests/wpt: use iterkeys, itervalues and iteritems from six package

7fdd0b9

marmeladema force-pushed the issue-23607/compat branch from c6e4d7a to 7fdd0b9 Compare October 22, 2019 09:17

marmeladema requested a review from SimonSapin October 22, 2019 09:18

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Oct 22, 2019

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Oct 22, 2019

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Oct 22, 2019

bors-servo merged commit 7fdd0b9 into servo:master Oct 22, 2019

highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Oct 22, 2019

Darkspirit mentioned this pull request Nov 11, 2019

HSTS & CA updates; Fix Debian bootstrap; Default to https on Android, too. #24704

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More changes for Python3 compatibility #24512

More changes for Python3 compatibility #24512

marmeladema commented Oct 20, 2019 •

edited

highfive commented Oct 20, 2019

marmeladema commented Oct 21, 2019

JacobAWilkins left a comment

saschanaz commented Oct 21, 2019 •

edited

jdm commented Oct 21, 2019 •

edited

marmeladema commented Oct 21, 2019

SimonSapin left a comment

marmeladema commented Oct 22, 2019

SimonSapin commented Oct 22, 2019

marmeladema commented Oct 22, 2019

SimonSapin commented Oct 22, 2019

SimonSapin commented Oct 22, 2019

bors-servo commented Oct 22, 2019

bors-servo commented Oct 22, 2019

bors-servo commented Oct 22, 2019

CYBAI commented Oct 22, 2019

marmeladema commented Oct 22, 2019

bors-servo commented Oct 22, 2019

jdm commented Oct 22, 2019

bors-servo commented Oct 22, 2019

bors-servo commented Oct 22, 2019

More changes for Python3 compatibility #24512

More changes for Python3 compatibility #24512

Conversation

marmeladema commented Oct 20, 2019 • edited

highfive commented Oct 20, 2019

marmeladema commented Oct 21, 2019

JacobAWilkins left a comment

Choose a reason for hiding this comment

saschanaz commented Oct 21, 2019 • edited

jdm commented Oct 21, 2019 • edited

marmeladema commented Oct 21, 2019

SimonSapin left a comment

Choose a reason for hiding this comment

marmeladema commented Oct 22, 2019

SimonSapin commented Oct 22, 2019

marmeladema commented Oct 22, 2019

SimonSapin commented Oct 22, 2019

SimonSapin commented Oct 22, 2019

bors-servo commented Oct 22, 2019

bors-servo commented Oct 22, 2019

bors-servo commented Oct 22, 2019

CYBAI commented Oct 22, 2019

marmeladema commented Oct 22, 2019

bors-servo commented Oct 22, 2019

jdm commented Oct 22, 2019

bors-servo commented Oct 22, 2019

bors-servo commented Oct 22, 2019

marmeladema commented Oct 20, 2019 •

edited

saschanaz commented Oct 21, 2019 •

edited

jdm commented Oct 21, 2019 •

edited