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

[BUG] If there are spaces in ANDROID_SDK_ROOT, ndk-build will not work. #1400

Closed
vvb2060 opened this issue Dec 14, 2020 · 7 comments
Closed
Labels

Comments

@vvb2060
Copy link

vvb2060 commented Dec 14, 2020

Description

If there are spaces in ANDROID_SDK_ROOT, ndk-build will not work.
test case: GitHub Actions windows-latest OS
log: https://github.com/vvb2060/XposedDetector/runs/1550701901?check_suite_focus=true#step:5:62

Environment Details

  • NDK Version: 21.3.6528147 and 22.0.6917172-beta1
  • Build system: ndk-build command and gradle
  • Host OS: Windows
  • ABI: N/A
  • NDK API level: N/A
  • Device API level: N/A
@vvb2060 vvb2060 added the bug label Dec 14, 2020
@DanAlbert
Copy link
Member

I doubt this is something that will ever work reliably, FYI. This is extremely difficult to do correctly in a make based system, especially one like ours which makes heavy use of eval (you need to escape variables differently depending on how deep it will be used in a callstack).

We should probably emit a warning if the NDK is installed to such a path. We already do that when the source is in a path that contains spaces, but we don't check the NDK itself I guess.

We should also squash this particular case, but I suspect we'll regress.

@DanAlbert DanAlbert added this to Triaged in r23 via automation Dec 14, 2020
@DanAlbert DanAlbert removed this from Triaged in r23 Jun 10, 2021
@DanAlbert DanAlbert added this to Triaged in r24 via automation Jun 10, 2021
@DanAlbert DanAlbert removed this from Triaged in r24 Jan 12, 2022
@DanAlbert DanAlbert added this to Triaged in r25 via automation Jan 12, 2022
@DanAlbert DanAlbert removed this from Triaged in r25 Jun 27, 2022
@DanAlbert
Copy link
Member

Apparently this is already forbidden: https://android.googlesource.com/platform/ndk/+/refs/heads/master/build/ndk-build#53 and https://android.googlesource.com/platform/ndk/+/refs/heads/master/build/core/build-local.mk#43. The former of course isn't used on Windows, but the latter doesn't appear to work there either. Your GHA log has unfortunately been culled, so I have no idea what the original report was, but this does seem quite broken on Windows.

@DanAlbert
Copy link
Member

Adding this seems to be the way to catch the problem (will upload a patch in a moment):

for /f "tokens=2" %%a in ("%~dp0") do (
    echo ERROR: NDK path cannot contain spaces
    exit /b 1
)
@DanAlbert
Copy link
Member

actions/runner-images#1122 (comment)

Thanks! I tried the same thing you did (quote the thing in ndk-build.cmd) and ran into the same problem (that's insufficient). The fix you ended up merging makes sense to me.

Apparently this is already forbidden: https://android.googlesource.com/platform/ndk/+/refs/heads/master/build/ndk-build#53 and https://android.googlesource.com/platform/ndk/+/refs/heads/master/build/core/build-local.mk#43

make can't catch the second case on windows. $(lastword $(MAKEFILE_LIST)) when the NDK's install directory is android-ndk-r25c with spaces is spaces\build\..\build\core\build-local.mk. ndk-build.cmd already passes a quoted path to make, so that really is just make being different on Windows (a bug, but not one I care to chase).

@DanAlbert
Copy link
Member

make can't catch the second case on windows.

This, in case it wasn't clear, is a pretty big blocker for us ever doing better at this. Still probably less problematic than eval, but it's an early enough blocker that I don't think it's even worth trying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 participants