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

tools: fixup the CMake custom command for elf-cleaner to correctly find sources #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tturner
Copy link
Contributor

@tturner tturner commented Jun 11, 2023

This is a tiny change to allow elf-cleaner to build correctly during assembleRelease.

Without this I got failures to find these files with the latest Android Studio (and on the command line) - the glob found no files so the build command was malformed.

@tturner tturner marked this pull request as draft June 11, 2023 14:27
@tturner tturner marked this pull request as ready for review June 11, 2023 14:30
@msfjarvis
Copy link
Member

I haven't run into a build error yet but this is a good change even for consistency sake, merged as 1ba84e0

@msfjarvis msfjarvis closed this Jun 11, 2023
@zx2c4
Copy link
Member

zx2c4 commented Jun 13, 2023

Wondering how to repro this. Do you run gradle from a different CWD or something?

@tturner
Copy link
Contributor Author

tturner commented Jun 13, 2023

I didn't think I was doing anything particularly unusual. Fairly vanilla Debian 12 (no custom shell). Installed latest beta? of the android studio (Hedgehog) because that seemed to be needed for the latest commits on master (I got errors relating to the agp alpha version when I tried the normal android studio release).

I had that download the android SDK and then used ANDROID_HOME env to point to the sdk directory. I then ran the command from the readme in the base directory of the cloned repo.

I'm not sure why it is different from your experience but the general recommendation is to make cmake builds agnostic to the folder they are built in and the old version of that cmake command assumed that the custom command was running in the source folder which isn't guaranteed by cmake.

@zx2c4
Copy link
Member

zx2c4 commented Jun 13, 2023

Well, there are two other GLOBs in there that aren't doing this. So that's a bit puzzling. I think I'd like to see some analysis of what's happening here. AFAICT, what was there before should be correct already and seems standard practice.

@zx2c4 zx2c4 reopened this Jun 13, 2023
@zx2c4
Copy link
Member

zx2c4 commented Jun 21, 2023

No reply from you or further research so I'm going to revert the commit until the right time.

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.

3 participants