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

local-cache and link-to-sdk don't seem to work well together #518

Open
akien-mga opened this issue Nov 21, 2023 · 10 comments · Fixed by #519
Open

local-cache and link-to-sdk don't seem to work well together #518

akien-mga opened this issue Nov 21, 2023 · 10 comments · Fixed by #519

Comments

@akien-mga
Copy link

akien-mga commented Nov 21, 2023

Hi,

I've tried to use this action to set up NDK r23c in GHA's Ubuntu 20.04's runner, using the link-to-sdk option so that it would be found where it used to be pre-installed (GH removed it from their Ubuntu 20.04 runner recently: actions/runner-images@8bb825b).

This seems to work if local-cache isn't used, but if I use both options, then the NDK doesn't seem to be found in /usr/local/lib/android/sdk/ndk/23.2.8568313.
Example build log: https://github.com/godotengine/godot/actions/runs/6936937595/job/18870055757

With this config:

      - name: Install Android NDK r23c
        uses: nttld/setup-ndk@v1
        with:
          ndk-version: r23c
          link-to-sdk: true
          local-cache: true

Ideally I'd like to use both, so that I can install the NDK once and keep it cached for all subsequent runs.

I would imagine it would restore /home/runner/.setup-ndk/r23c from the cache, and re-create the symlink in /usr/local/lib/android/sdk/ndk/23.2.8568313 each time.

@DmitriySalnikov
Copy link

DmitriySalnikov commented Nov 21, 2023

I fixed it in my repository this way

All files were restored in the correct directory ${{steps.setup-ndk.outputs.ndk-path }}.
The problem was that the symbolic links when restoring from the cache referred to /opt/hostedtoolcache/ndk/r23/x64/ and not to /home/runner/.setup-ndk/r23 (action job)

Changed clang++ from /opt/hostedtoolcache/ndk/r23/x64/toolchains/llvm/prebuilt/linux-x86_64/bin/clang to /home/runner/.setup-ndk/r23/toolchains/llvm/prebuilt/linux-x86_64/bin/clang
Changed llvm-strip from /opt/hostedtoolcache/ndk/r23/x64/toolchains/llvm/prebuilt/linux-x86_64/bin/llvm-objcopy to /home/runner/.setup-ndk/r23/toolchains/llvm/prebuilt/linux-x86_64/bin/llvm-objcopy

I switched to using link-to-sdk: true, but it still requires fixing symbolic links as well. Test with fix/Test without fix

@raftario
Copy link
Member

I'll give this a look later today, should be an easy fix. Cool seeing this used in Godot :D

@raftario
Copy link
Member

raftario commented Nov 21, 2023

I think I have a good idea what is happening, currently the action will silently skip creating the symlink if the directory already exists. Looks like GitHub forgot to remove their symlink and it's still there pointing to non-existent files. I'll probably just change it to overwrite existing ones, shouldn't be a breaking change given it's the exact same version.

I'm assuming you'd get the exact same error without running the action. Someone should probably verify and report the issue but I'm probably gonna be lazy and just fix it here and let someone else run into it and report the issue themselves.

@akien-mga
Copy link
Author

Thanks for the quick update!

I indeed had a similar error without pre-installing the NDK on another Godot repo (godot-cpp): https://github.com/godotengine/godot-cpp/actions/runs/6930951873/job/18859997384
So your findings seem correct.

That didn't break the main godot repo's CI though, presumably because our SCons buildsystem has some logic to check the existence of the NDK and install it if missing. Not sure why it then started failing with using this action with local-cache + link-to-sdk though.

I tested the fix and it doesn't seem to work yet for me:

I may have an invalid cache from a previous attempt that I'd need to clean manually?

@DmitriySalnikov
Copy link

DmitriySalnikov commented Nov 22, 2023

I confirm, without manually fixing the links, the same error appears
https://github.com/DmitriySalnikov/godot_debug_draw_3d/actions/runs/6955475648

And no errors with my Restore Android Symlinks step
https://github.com/DmitriySalnikov/godot_debug_draw_3d/actions/runs/6955529520/job/18924474269

But it is very strange that if the cache has not been used yet and Restore Android Symlinks is active, then this step still fixes exactly the same links as after restoring from the cache.
Probably after saving the cache, the unpacked NDK remains in some folder that is not recreated after recovery from the cache. (the paths in the screenshot are taken from the contents of symbolic links)
image

@DmitriySalnikov
Copy link

@raftario can you check again what the problem is?

@raftario
Copy link
Member

I will during the weekend

@raftario raftario reopened this Nov 24, 2023
@lynxnb
Copy link

lynxnb commented Feb 27, 2024

Hey, any update on this? I was planning on using this on my repo to avoid caching ndk manually.

@raftario
Copy link
Member

The honest answer is that I genuinely have no idea why the issue arises and would need to dig into it more, but didn't have time for it during the holidays and don't have time for it now as I am in the process of moving across the country. I'd be more than happy to accept contributions from someone willing to do the work but in the meantime I'm the only person maintaining this action and fixing a small edge case in it is pretty far down my priority list. I hope you can understand.

@lynxnb
Copy link

lynxnb commented Mar 1, 2024

Thank you for replying, that is completely understandable and I wish you good luck. I might attempt looking into this in the future but my hands are full at the moment, so I don't think that will happen anytime soon.

mariotaku added a commit to mariotaku/libssh-rs that referenced this issue May 15, 2024
Green-Sky added a commit to Green-Sky/tomato that referenced this issue May 30, 2024
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 a pull request may close this issue.

4 participants