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

Under certain conditions, the CMAKE environment variable is not included in TemplateVariableInfo #1161

Closed
ghost opened this issue Jan 19, 2024 · 1 comment · Fixed by #1163

Comments

@ghost
Copy link

ghost commented Jan 19, 2024

Hello,

I've noticed that under some certain conditions, the CMAKE environment variable is not included in the TemplateVariableInfo object owned by the current_cmake_toolchain.

For instance, if CMake is built from source, then the variable is set:

https://github.com/bazelbuild/rules_foreign_cc/blob/c2e097455d2bbf92b2ae71611d1261ba79eb8aa8/toolchains/private/BUILD.bazel#L62-L65

However, when it comes to prebuilt CMake bundle, the variable isn't set.

https://github.com/bazelbuild/rules_foreign_cc/blob/c2e097455d2bbf92b2ae71611d1261ba79eb8aa8/toolchains/prebuilt_toolchains.py#L333-L342

Note that this issue does not affect prebuilt Ninja bundles:

https://github.com/bazelbuild/rules_foreign_cc/blob/c2e097455d2bbf92b2ae71611d1261ba79eb8aa8/toolchains/prebuilt_toolchains.py#L449

I'm trying to compile a rust crate which uses the cmake crate in its build.rs build script. I can include the CMake toolchain in the build script processing using the build_script_toolchains annotation, but since the CMAKE environment variable isn't necessary set, the build script fails to find the CMake binary.

Do you think this is a real issue?

I've written a quick-but-unsound fix locally, but before trying to open a PR to fix it I wanted to make sure that it's a real issue.

@jsharpe
Copy link
Member

jsharpe commented Jan 19, 2024

I think this is just an oversight rather than intentional design - feel free to put together a PR to make this consistent!

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.

1 participant