Skip to content

Conversation

@compnerd
Copy link
Member

This addresses a missing component of the initial Android support. When the user specifies -sysroot, that should always be given precedence as it is explicitly specified by the user. The ANDROID_NDK_ROOT environment variable is set by the Android NDK and is used as a default value in the scenario that the user does not specify the -sysroot option. This makes Android both behave more like the Windows platform and also ensures that the user has full control over the behaviour of the toolchain.

Take the opportunity to refactor some of the code to extract the Android NDK specific helpers into an uninhabited enum and provide some behavioural test cases.

This addresses a missing component of the initial Android support. When
the user specifies `-sysroot`, that should always be given precedence as
it is explicitly specified by the user. The `ANDROID_NDK_ROOT`
environment variable is set by the Android NDK and is used as a
_default_ value in the scenario that the user does not specify the
`-sysroot` option. This makes Android both behave more like the Windows
platform and also ensures that the user has full control over the
behaviour of the toolchain.

Take the opportunity to refactor some of the code to extract the Android
NDK specific helpers into an uninhabited enum and provide some
behavioural test cases.
@compnerd compnerd requested a review from hyp August 18, 2024 19:02
@compnerd
Copy link
Member Author

CC: @finagolfin

I think that this might have been the misunderstanding that we had. The ANDROID_NDK_ROOT was never meant to be the way to configure the behaviour of the frontend, but rather a fallback. The initial implementation did not provide the control to the user that it should have, but did unblock some work. This change now fixes that limitation and "demotes" the ANDROID_NDK_ROOT environment variable to the fallback path where it always should have been.

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd compnerd requested a review from artemcm August 19, 2024 17:31
@hyp
Copy link
Contributor

hyp commented Aug 19, 2024

thank you

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