Skip to content

Conversation

roblabla
Copy link
Contributor

@roblabla roblabla commented Jun 7, 2021

This would help cross-compilation a bit, allowing the user to have some global environment variables set up for each of their cross-compilation toolchain.

The get_target_env_var function is taken from cc-rs::get_var function.

@roblabla roblabla marked this pull request as draft June 7, 2021 09:58
@roblabla roblabla force-pushed the per-target-cmake-toolchain branch from 841056b to b3efe67 Compare June 7, 2021 10:03
@roblabla roblabla marked this pull request as ready for review June 7, 2021 10:06
@alexcrichton
Copy link
Member

Thanks! Could this be done for most other env var accesses as well? Additionally, like cc-rs, could this print out the results of what env vars are inspected (sometimes very helpful for debugging failures)

@roblabla
Copy link
Contributor Author

roblabla commented Jun 7, 2021

Could this be done for most other env var accesses as well?

Right, so here are the two other environment variables where I think it'd make sense to make them per-target:

  • CMAKE_GENERATOR: Makes sense, you may want to use mingw makefiles or msbuild depending on the target for instance.
  • CMAKE_INSTALL_PREFIX: This is used by find_package and company.

The other two, I'm not so sure:

  • DEP_{dep}_ROOT: I'm not sure what this does.
  • CMAKE: This just points to the cmake binary. I don't think it makes sense to make it per-target, as I believe the same cmake binary is used when cross-compiling (at least that's what I do!).

@alexcrichton
Copy link
Member

For DEP_* those are cargo-specific things so they should avoid this function (although it might be nice to print out that they're being accessed like cc-rs does), but for CMAKE I'd just throw it into the same function for consistency, but I agree that it's pretty doubtful anyone will use a target-specific variable for that.

@roblabla roblabla force-pushed the per-target-cmake-toolchain branch from b3efe67 to 27f166e Compare June 9, 2021 11:52
@roblabla roblabla force-pushed the per-target-cmake-toolchain branch 3 times, most recently from 5764ed1 to 344cff1 Compare June 9, 2021 12:27
@roblabla
Copy link
Contributor Author

roblabla commented Jun 9, 2021

How does this look? It should now get the per-target environment variable for CMAKE, CMAKE_GENERATOR, CMAKE_PREFIX_PATH and CMAKE_TOOLCHAIN_FILE, and print on access. The same env_cache logic found in cc-rs was used here to make sure we only print once per env variable.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me!

@roblabla roblabla force-pushed the per-target-cmake-toolchain branch from af53561 to 657055a Compare June 9, 2021 15:59
@roblabla roblabla force-pushed the per-target-cmake-toolchain branch from 657055a to f9002a6 Compare June 9, 2021 16:06
@alexcrichton
Copy link
Member

Thanks!

@alexcrichton alexcrichton merged commit 5f89f90 into rust-lang:master Jun 9, 2021
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.

2 participants