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

CMakeLists: Fix deduplication trap with --preload-file #12944

Merged
merged 1 commit into from
Mar 16, 2024

Conversation

fwcd
Copy link
Member

@fwcd fwcd commented Mar 9, 2024

CMake applies a form of deduplication that, when specifying

target_link_options(mixxx-lib PUBLIC --preload-file a)
target_link_options(mixxx-lib PUBLIC --preload-file b)

would only result in

--preload-file a b

because it considers --preload-file to be a flag that was already set. This leads to very confusing error messages, because the Emscripten compiler would consider b to be a positional argument (i.e. a source file) rather than a value to --preload-file.

While this currently isn't an issue, because we only set this flag once, the current syntax is misleading and could easily lead to issues if we e.g. split up the resources as proposed in the adjacent TODO comment. For this reason, this PR migrates to the --preload-file=... syntax to avoid this.

Side note: Another way to do this would be to use "SHELL:--preload-file ...", but then we would likely have to deal with escaping/quoting the path ourselves to make it robust against spaces. For details, see this upstream CMake issue and this CMake PR.

@JoergAtGithub
Copy link
Member

LGTM! Local build works as before! Thank you!

@JoergAtGithub JoergAtGithub merged commit 51e0827 into mixxxdj:main Mar 16, 2024
13 checks passed
@fwcd fwcd deleted the fix-deduplication-trap branch March 21, 2024 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants