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

Enable some mach commands to be run with python3 #24575

Merged
merged 12 commits into from Dec 10, 2019

Conversation

@marmeladema
Copy link
Contributor

marmeladema commented Oct 29, 2019

This change finally enable the following commands to be run with python3:

  • build
  • test-unit
  • package

As previously explained, test-tidy will require more work in the wpt repository directly. Maybe test-tidy --no-wpt is achievable relatively quickly though.

For possible remaining bits that might need to be worked on, see #23607


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes
@highfive
Copy link

highfive commented Oct 29, 2019

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/requirements.txt, python/servo/command_base.py, python/tidy/servo_tidy/tidy.py, python/mach_bootstrap.py
@marmeladema marmeladema force-pushed the marmeladema:issue-23607/compat branch 2 times, most recently from 3e6a093 to db98e22 Oct 29, 2019
@marmeladema
Copy link
Contributor Author

marmeladema commented Nov 8, 2019

Friendly ping @SimonSapin :-) Is there any concern about this PR?

@jdm
Copy link
Member

jdm commented Nov 12, 2019

Review ping @SimonSapin.

@SimonSapin
Copy link
Member

SimonSapin commented Nov 14, 2019

Sorry for the delay! The diff looks good

@bors-servo try=linux

bors-servo added a commit that referenced this pull request Nov 14, 2019
Enable some mach commands to be run with python3

This change finally enable the following commands to be run with python3:
* `build`
* `test-unit`
* `package`

As previously explained, `test-tidy` will require more work in the wpt repository directly. Maybe `test-tidy --no-wpt` is achievable relatively quickly though.

For possible remaining bits that might need to be worked on, see #23607

---
<!-- 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

<!-- Either: -->
- [x] There are tests for these changes

<!-- 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 Nov 14, 2019

Trying commit db98e22 with merge eadacc5...

@SimonSapin
Copy link
Member

SimonSapin commented Nov 14, 2019

The Tidy+Unit task failed with:

$ python3 ./mach build --dev
Python virtualenv is not installed. Please install it prior to running mach.

It looks like python/mach_bootstrap.py wants to import virtualenv (so using the same Python version as itself), not just run a virtualenv executable.

Try adding virtualenv among the APT packages installed in etc/taskcluster/docker/base.dockerfile?

@bors-servo
Copy link
Contributor

bors-servo commented Nov 14, 2019

💔 Test failed - status-taskcluster

@SimonSapin
Copy link
Member

SimonSapin commented Nov 14, 2019

Try adding virtualenv among the APT packages installed in etc/taskcluster/docker/base.dockerfile?

Actually it looks like I can do this for you. I took the liberty of pushing to your branch.

@bors-servo r=linux

@bors-servo
Copy link
Contributor

bors-servo commented Nov 14, 2019

📌 Commit 555435c has been approved by linux

@SimonSapin
Copy link
Member

SimonSapin commented Nov 14, 2019

Err

@bors-servo r- try=linux

@bors-servo
Copy link
Contributor

bors-servo commented Nov 14, 2019

Trying commit 555435c with merge 9bb904f...

bors-servo added a commit that referenced this pull request Nov 14, 2019
Enable some mach commands to be run with python3

This change finally enable the following commands to be run with python3:
* `build`
* `test-unit`
* `package`

As previously explained, `test-tidy` will require more work in the wpt repository directly. Maybe `test-tidy --no-wpt` is achievable relatively quickly though.

For possible remaining bits that might need to be worked on, see #23607

---
<!-- 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

<!-- Either: -->
- [x] There are tests for these changes

<!-- 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. -->
@SimonSapin
Copy link
Member

SimonSapin commented Nov 14, 2019

https://community-tc.services.mozilla.com/tasks/CKazBdxJSTejY8R7TEPZgA/runs/0/logs/live/https%3A%2F%2Fcommunity-tc.services.mozilla.com%2Fapi%2Fqueue%2Fv1%2Ftask%2FCKazBdxJSTejY8R7TEPZgA%2Fruns%2F0%2Fartifacts%2Fpublic%2Flogs%2Flive.log

$ python3 ./mach build --dev
"/repo/python/_virtualenv3.6/bin/python" "-m" "pip" "install" "-I" "-r" "/repo/tests/wpt/web-platform-tests/tools/wptrunner/requirements_firefox.txt" failed with error code 1:
Output:
Traceback (most recent call last):
  File "./mach", line 103, in <module>
    main(sys.argv)
  File "./mach", line 33, in main
    mach = mach_bootstrap.bootstrap(topdir)
  File "/repo/python/mach_bootstrap.py", line 274, in bootstrap
    _activate_virtualenv(topdir, is_firefox)
  File "/repo/python/mach_bootstrap.py", line 204, in _activate_virtualenv
    _process_exec([python, "-m", "pip", "install", "-I", "-r", req_path])
  File "/repo/python/mach_bootstrap.py", line 109, in _process_exec
    shutil.copyfileobj(out, sys.stdout)
  File "/usr/lib/python3.6/shutil.py", line 82, in copyfileobj
    fdst.write(buf)
TypeError: write() argument must be str, not bytes
@bors-servo
Copy link
Contributor

bors-servo commented Nov 14, 2019

💔 Test failed - status-taskcluster

@bors-servo
Copy link
Contributor

bors-servo commented Nov 20, 2019

The latest upstream changes (presumably #24785) made this pull request unmergeable. Please resolve the merge conflicts.

@bors-servo
Copy link
Contributor

bors-servo commented Dec 10, 2019

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Dec 10, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 10, 2019

Testing commit 02c55e2 with merge b358203...

bors-servo added a commit that referenced this pull request Dec 10, 2019
Enable some mach commands to be run with python3

This change finally enable the following commands to be run with python3:
* `build`
* `test-unit`
* `package`

As previously explained, `test-tidy` will require more work in the wpt repository directly. Maybe `test-tidy --no-wpt` is achievable relatively quickly though.

For possible remaining bits that might need to be worked on, see #23607

---
<!-- 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

<!-- Either: -->
- [x] There are tests for these changes

<!-- 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 Dec 10, 2019

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Dec 10, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 10, 2019

Testing commit 02c55e2 with merge d9e9dfe...

bors-servo added a commit that referenced this pull request Dec 10, 2019
Enable some mach commands to be run with python3

This change finally enable the following commands to be run with python3:
* `build`
* `test-unit`
* `package`

As previously explained, `test-tidy` will require more work in the wpt repository directly. Maybe `test-tidy --no-wpt` is achievable relatively quickly though.

For possible remaining bits that might need to be worked on, see #23607

---
<!-- 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

<!-- Either: -->
- [x] There are tests for these changes

<!-- 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 Dec 10, 2019

💔 Test failed - status-taskcluster

@jdm
Copy link
Member

jdm commented Dec 10, 2019

@bors-servo
Copy link
Contributor

bors-servo commented Dec 10, 2019

Testing commit 02c55e2 with merge 8d4cedb...

bors-servo added a commit that referenced this pull request Dec 10, 2019
Enable some mach commands to be run with python3

This change finally enable the following commands to be run with python3:
* `build`
* `test-unit`
* `package`

As previously explained, `test-tidy` will require more work in the wpt repository directly. Maybe `test-tidy --no-wpt` is achievable relatively quickly though.

For possible remaining bits that might need to be worked on, see #23607

---
<!-- 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

<!-- Either: -->
- [x] There are tests for these changes

<!-- 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 Dec 10, 2019

☀️ Test successful - status-taskcluster
Approved by: SimonSapin
Pushing 8d4cedb to master...

@bors-servo bors-servo merged commit 02c55e2 into servo:master Dec 10, 2019
2 checks passed
2 checks passed
Community-TC (pull_request) TaskGroup: success
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Dec 10, 2019
0 of 5 tasks complete
@marmeladema marmeladema deleted the marmeladema:issue-23607/compat branch Dec 10, 2019
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

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