Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More changes for Python3 compatibility #24512

Merged
merged 14 commits into from Oct 22, 2019
Merged

Conversation

@marmeladema
Copy link
Contributor

marmeladema commented Oct 20, 2019

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
@highfive
Copy link

highfive commented Oct 20, 2019

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 marmeladema force-pushed the marmeladema:issue-23607/compat branch 2 times, most recently from 0e1352c to c6e4d7a Oct 20, 2019
@marmeladema
Copy link
Contributor Author

marmeladema commented Oct 21, 2019

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

Copy link

JacobAWilkins left a comment

Very nice

@saschanaz
Copy link
Contributor

saschanaz commented Oct 21, 2019

r? @jdm

i don't know how to add you as reviewer

This is how 😁

@highfive highfive assigned jdm and unassigned SimonSapin Oct 21, 2019
@jdm
Copy link
Member

jdm commented Oct 21, 2019

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

@highfive highfive assigned SimonSapin and unassigned jdm Oct 21, 2019
@marmeladema
Copy link
Contributor Author

marmeladema commented Oct 21, 2019

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

Copy link
Member

SimonSapin left a comment

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:

./mach build --dev

python/tidy/servo_tidy/tidy.py Outdated Show resolved Hide resolved
@marmeladema
Copy link
Contributor Author

marmeladema commented Oct 22, 2019

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:

./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
Copy link
Member

SimonSapin commented Oct 22, 2019

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
Copy link
Contributor Author

marmeladema commented Oct 22, 2019

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
Copy link
Member

SimonSapin commented Oct 22, 2019

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

@marmeladema marmeladema force-pushed the marmeladema:issue-23607/compat branch from c6e4d7a to 7fdd0b9 Oct 22, 2019
@marmeladema marmeladema requested a review from SimonSapin Oct 22, 2019
@SimonSapin
Copy link
Member

SimonSapin commented Oct 22, 2019

Looks good, thanks!

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Oct 22, 2019

📌 Commit 7fdd0b9 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

bors-servo commented Oct 22, 2019

Testing commit 7fdd0b9 with merge a1d4342...

bors-servo added a commit that referenced this pull request Oct 22, 2019
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.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 22, 2019

💔 Test failed - status-taskcluster

@CYBAI
Copy link
Collaborator

CYBAI commented Oct 22, 2019

@bors-servo retry

  • network issue in magicleap build
@marmeladema
Copy link
Contributor Author

marmeladema commented Oct 22, 2019

@bors-servo retry

* network issue in magicleap build

I don't think the retry worked?

@bors-servo
Copy link
Contributor

bors-servo commented Oct 22, 2019

@marmeladema: 🔑 Insufficient privileges: not in try users

@jdm
Copy link
Member

jdm commented Oct 22, 2019

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

@bors-servo
Copy link
Contributor

bors-servo commented Oct 22, 2019

Testing commit 7fdd0b9 with merge 3e7a0bf...

bors-servo added a commit that referenced this pull request Oct 22, 2019
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.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 22, 2019

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

@bors-servo bors-servo merged commit 7fdd0b9 into servo:master Oct 22, 2019
2 checks passed
2 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
homu Test successful
Details
bors-servo added a commit that referenced this pull request Nov 12, 2019
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.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [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)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.