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

Don't use generator expressions #1415

Closed
wants to merge 1 commit into from
Closed

Conversation

kasper93
Copy link

While generator expression is more concise, it is avoidable in this case. This change fixes compatibility with Meson, which currently does not support generator expressions and would incorrectly enable install. While this shouldn't be critical, it exposes other compatibility issues. In short, by changing this basic expression, I'm able to build shaderc with Meson wrap successfully.

While generator expression is more concise, it is avoidable in this
case. This change fixes compatibility with Meson, which currently does
not support generator expressions and would incorrectly enable install.
While this shouldn't be critical, it exposes other compatibility issues.
In short, by changing this basic expression, I'm able to build shaderc
with Meson wrap successfully.
@dneto0
Copy link
Collaborator

dneto0 commented Jun 3, 2024

Sorry, Meson is not one of our supported build systems.
I counted 99 uses of CMake generator expressions in my Shaderc tree (including dependencies)
I don't think it's viable to back out.

I noticed in your upstream example that you turned off Shaderc tests.
mesonbuild/meson#13214
I encourage folks to run the tests. It catches bugs. (I once caught a bad cross-compiler in the Android NDK because it miscompiled some Glslang code, and I caught it with a Shaderc test).

@dneto0 dneto0 closed this Jun 3, 2024
@kasper93
Copy link
Author

kasper93 commented Jun 3, 2024

Sorry, Meson is not one of our supported build systems. I counted 99 uses of CMake generator expressions in my Shaderc tree (including dependencies) I don't think it's viable to back out.

Understandable. The point here was that Meson seems to support generator expressions that are passed down to compilation, as they were intended use-case for those in CMake. It seems to just not parse correctly those simple configuration time conditionals. Either way, thank you for looking at this PR.

I noticed in your upstream example that you turned off Shaderc tests. mesonbuild/meson#13214 I encourage folks to run the tests. It catches bugs. (I once caught a bad cross-compiler in the Android NDK because it miscompiled some Glslang code, and I caught it with a Shaderc test).

I fully agree. This was done to minimize test surface, by disabling as much as possible. Also I don't think Meson can convert ctest to own tests, so that's another problem.

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.

None yet

2 participants