-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Fix android armeabi-v7a constraint #15248
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
Fix android armeabi-v7a constraint #15248
Conversation
adbc734 to
4cb7f0f
Compare
Previously this was arm instead of armv7. It seems that armv7 is the correct constraint for this case. Fixes bazelbuild#14982
4cb7f0f to
a8680ca
Compare
|
Taking a look at this. This does seem like a promising fix for #14982, and is the PR I was recalling to @gregce (comment #14982 (comment)). The big question is to find out how this will work with our internal Android processes, I'm going to import this and run a lot of tests, I'll report back when that's done (it may be a few days). |
| "armeabi-v7a": ["@platforms//cpu:armv7"], | ||
| "armeabi-v7a": [ | ||
| "@platforms//cpu:armv7", | ||
| "@platforms//os:android", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can't be right, shouldn't this be osx?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thanks for explaining.
|
Ran the internal tests and, as I suspected, there's no impact. I'm going to work on getting this submitted in the next few days. |
Previously this was arm instead of armv7. It seems that armv7 is the correct constraint for this case. Fixes #14982 Closes #15248. PiperOrigin-RevId: 444250195 Co-authored-by: Keith Smiley <[email protected]>
Previously this was arm instead of armv7. It seems that armv7 is the
correct constraint for this case.
Fixes #14982