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

On Python 3, Mach doesn't set executable permission when extracting zips, breaking `bootstrap-android` #25618

Closed
zhuowei opened this issue Jan 26, 2020 · 1 comment
Labels

Comments

@zhuowei
Copy link
Contributor

@zhuowei zhuowei commented Jan 26, 2020

When running python3 ./mach bootstrap-android --build using Python 3, I get this error:

zhuowei@dora:~/servo-work/servo$ python3 ./mach bootstrap-android --build
Downloading https://dl.google.com/android/repository/android-ndk-r15c-linux-x86_64.zip ...
Downloading android-ndk-r15c-linux-x86_64.zip: 100.0%
Extracting android-ndk-r15c-linux-x86_64.zip
Downloading https://dl.google.com/android/repository/sdk-tools-linux-4333796.zip ...
Downloading sdk-tools-linux-4333796.zip:  99.8%
Extracting sdk-tools-linux-4333796.zip
Error running mach:

    ['bootstrap-android', '--build']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.
You can invoke |./mach busted| to check if this issue is already on file. If it
isn't, please use |./mach busted file| to report it. If |./mach busted| is
misbehaving, you can also inspect the dependencies of bug 1543241.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

PermissionError: [Errno 13] Permission denied: '/home/zhuowei/servo-work/servo/android-toolchains/sdk/tools/bin/sdkmanager'

  File "/home/zhuowei/servo-work/servo/python/servo/bootstrap_commands.py", line 185, in bootstrap_android
    subprocess.check_call(sdkmanager)
  File "/usr/lib/python3.7/subprocess.py", line 358, in check_call
    retcode = call(*popenargs, **kwargs)
  File "/usr/lib/python3.7/subprocess.py", line 339, in call
    with Popen(*popenargs, **kwargs) as p:
  File "/usr/lib/python3.7/subprocess.py", line 800, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python3.7/subprocess.py", line 1551, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)

It looks like

# https://stackoverflow.com/questions/39296101/python-zipfile-removes-execute-permissions-from-binaries
is supposed to set the executable bit when extracting zips, but the linked Stack Overflow issue states that the current workaround broke on Python 3.6 and above.

@garasubo
Copy link
Contributor

@garasubo garasubo commented Feb 7, 2020

As the second and third answer in that Stack Overflow issue, this is because this library uses different method in it in Python 3.6 and later.

@garasubo garasubo mentioned this issue Feb 7, 2020
2 of 5 tasks complete
bors-servo added a commit that referenced this issue Feb 10, 2020
fix zip extraction for python 3

Fix #25618

Confirmed that this worked in Python 3.6 and Python 2.7.

Ref: https://stackoverflow.com/questions/805066/call-a-parent-classs-method-from-child-class

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #25618 (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. -->
bors-servo added a commit that referenced this issue Feb 10, 2020
fix zip extraction for python 3

Fix #25618

Confirmed that this worked in Python 3.6 and Python 2.7.

Ref: https://stackoverflow.com/questions/805066/call-a-parent-classs-method-from-child-class

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #25618 (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 pull requests

Successfully merging a pull request may close this issue.

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