Skip to content

[test] Style fixes for "Modify test scripts for aarch64 and modern NDKs" #23506

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

Merged

Conversation

drodriguez
Copy link
Contributor

Some style fixes that @compnerd proposed in PR #19503.

  • Instead of checking for the two known ARMv7 subarchitectures, check for everything that starts with armv7 and consider it armv7.
  • Try finding the current platform to select the right prebuilt directory for the NDK tools.
  • Use -isystem for the system header includes when passing them to
    clang.
  • Modify the format strings which started to be very wild into simpler ' '.join([...]) which, IMO, read better.
  • Hoist some common path parts to not repeat the same long paths in a
    couple of places.
  • Add more library search paths (missing for atomic)

Some style fixes that @compnerd proposed in PR swiftlang#19503.

- Instead of checking for the two known ARMv7 subarchitectures, check for everything that starts with armv7 and consider it armv7.
- Try finding the current platform to select the right prebuilt directory for the NDK tools.
- Use -isystem for the system header includes when passing them to
  clang.
- Modify the format strings which started to be very wild into simpler `' '.join([...])` which, IMO, read better.
- Hoist some common path parts to not repeat the same long paths in a
  couple of places.
- Add more library search paths (missing for atomic)
@drodriguez drodriguez requested a review from compnerd March 23, 2019 00:42
Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

The ' '.join([...]) is wildly better to follow than the format, that helps this quite a bit.

# TODO: NDK distributes for Windows 32 and 64 bits. platform.machine()
# should allow us to find out the word size, but I don't have a
# machine to test right now. I think the values are AMD64 and x86, but
# I'm not sure. Everybody gets the 64 bits version for now.
Copy link
Member

Choose a reason for hiding this comment

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

More bits is more better right? :-p

@compnerd
Copy link
Member

@swift-ci please smoke test

@compnerd compnerd merged commit 31f3d51 into swiftlang:master Mar 25, 2019
@drodriguez drodriguez deleted the android-aarch64-test-script-redux branch April 8, 2019 20:31
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.

2 participants