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

bootstrap.py: fix armv7 detection #41152

Merged
merged 1 commit into from
Apr 8, 2017
Merged

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Apr 8, 2017

This matches the logic that was in ./configure before f8ca805.

This matches the logic that was in `./configure` before f8ca805.
@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

cputype = 'arm'
ostype += 'eabihf'
elif cputype == 'armv7l':
elif cputype in {'armv7l', 'armv8l'}:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

armv8l is still running in 32-bit mode. If you're running 64-bit, uname -m should say aarch64.

I just verified this on my Galaxy S7. When I run uname -m via adb shell, I get aarch64. When I run under JuiceSSH localhost, I get armv8l. Last time I checked, Termux only installed a 32-bit environment, which might explain why you don't see aarch64.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, I just tried Termux myself, and I do see aarch64. From their changelog it seems they started installing 64-bit about a year ago. Perhaps your installation predates this?

Start using 64-bit arm packages for capable devices. Up until now Termux used 32-bit arm binaries even on 64-bit devices. This change will affect only new installations - existing users may reinstall the app to get a 64-bit environment (making sure to save relevant material from the home folder first).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not my device... Do you have any idea of why does JuiceSSH differs from termux?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't mean that it was your device, just that it's the 64-bit Android I have at hand. I think the difference is simply that JuiceSSH is a 32-bit app, and my Termux installation got 64-bit.

Copy link
Contributor

@malbarbo malbarbo Apr 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misunderstood you. My english is not very good, sorry. Thank you once again.

@japaric
Copy link
Member

japaric commented Apr 8, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Apr 8, 2017

📌 Commit 5c0c3e8 has been approved by japaric

@japaric
Copy link
Member

japaric commented Apr 8, 2017

Nice catch, @cuviper!

@japaric japaric assigned japaric and unassigned alexcrichton Apr 8, 2017
@bors
Copy link
Contributor

bors commented Apr 8, 2017

⌛ Testing commit 5c0c3e8 with merge 85a9d75...

@bors
Copy link
Contributor

bors commented Apr 8, 2017

💔 Test failed - status-appveyor

@TimNN
Copy link
Contributor

TimNN commented Apr 8, 2017

TimNN added a commit to TimNN/rust that referenced this pull request Apr 8, 2017
bootstrap.py: fix armv7 detection

This matches the logic that was in `./configure` before f8ca805.
@TimNN TimNN mentioned this pull request Apr 8, 2017
bors added a commit that referenced this pull request Apr 8, 2017
Rollup of 4 pull requests

- Successful merges: #41135, #41143, #41146, #41152
- Failed merges:
@bors bors merged commit 5c0c3e8 into rust-lang:master Apr 8, 2017
@cuviper cuviper deleted the bootstrap-armv7 branch September 26, 2017 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants