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

[Rust] set CC and CFLAGS envvars #18

Open
yutannihilation opened this issue Jan 25, 2024 · 6 comments
Open

[Rust] set CC and CFLAGS envvars #18

yutannihilation opened this issue Jan 25, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@yutannihilation
Copy link
Contributor

yutannihilation commented Jan 25, 2024

(This issue is mainly for sharing the information, so feel free to close. But I hope this helps somehow.)

Probably you already know, the Rust compiler sometimes invokes the C compiler inside.

https://doc.rust-lang.org/cargo/reference/build-scripts.html

rwasm overrides the CC and CFLAGS Makevars, but I found these settings are not propagated to the C compiler unless the same name of envvars are set.

https://github.com/rust-lang/cc-rs#external-configuration-via-environment-variables

I'm not sure if this is what rwasm should take care of or the package author should, but I actually had to specify it in my package, which compiles a bit of C code in the crate.

yutannihilation/savvy-webr-test@5ee66d7

@yutannihilation yutannihilation changed the title [Rust] set CC and CLAGS envvars [Rust] set CC and CFLAGS envvars Jan 25, 2024
@georgestagg
Copy link
Member

georgestagg commented Jan 25, 2024

A great find, thank you for sharing! I think we should take care of this in rwasm if we can.

We could tweak webr-vars.mk to export some of the make variables so that they're available in the environment with changes such as,

- CC = emcc
- CXX = em++
- FC = $(EMFC)
- CFLAGS = $(WASM_CFLAGS)
- CPPFLAGS = $(WASM_CPPFLAGS)
- CXXFLAGS = -std=gnu++11 $(WASM_CXXFLAGS)
+ export CC = emcc
+ export CXX = em++
+ export FC = $(EMFC)
+ export CFLAGS = $(WASM_CFLAGS)
+ export CPPFLAGS = $(WASM_CPPFLAGS)
+ export CXXFLAGS = -std=gnu++11 $(WASM_CXXFLAGS)

With this, Rust should pick up the emcc compiler without package authors having to add CC=$(CC) manually.

One thing I am not sure about is how this change could potentially affect other R packages. I can think of a situation where this might break things: if some package is building an executable as an intermediate step, then running it on the host machine to generate some further output to be included in the package. I don't actually know if any R packages work like that - @jeroen, can you think of any? Or think of any other issues with simply exporting these variables to the build environment?

Either way, I suppose any packages doing something like that could be made to use the compiler given in HOST_CC by a configure step instead:

$ emconfigure env | grep CC
CC=/Users/georgestagg/emsdk/upstream/emscripten/emcc
HOST_CC=/Users/georgestagg/emsdk/upstream/bin/clang

@yutannihilation
Copy link
Contributor Author

Sounds great!

To be fair, the use of Rust is not majority and probably only few of them invokes C compiler. So, it might not be really worth the risk. At the moment, we anyway need to use Makevars.webr for overriding settings, so it might be reasonable to leave it for the author to specify it.

Also, to be precise, CC=$(CC) might not be strictly necessary. It seems choosing the wasm32-unknown-emscripten target determines the C compiler is emcc. (But, since I couldn't figured out the mechanism, I specify CC for safety).

@georgestagg
Copy link
Member

At the moment, we anyway need to use Makevars.webr for overriding settings

We shim uname in rwasm to return "Emscripten" when cross-compiling, so you might be able to combine things into a single Makevars by testing for that:

UNAME := $(shell uname -s)
ifeq ($(UNAME),Emscripten)
	[...]
endif

@georgestagg georgestagg added the enhancement New feature or request label Jan 25, 2024
@yutannihilation
Copy link
Contributor Author

Oh, good to know, thanks.

@yutannihilation
Copy link
Contributor Author

FYI, I chose to use configure instead of relying on ifeq GNU make extension. I'm not sure how it's meaningful, but it seems it's the preference of the CRAN maintainers to avoid extensions for the maximum portability. YMMV.

yutannihilation/savvy-webr-test@257f8fb

@jeroen
Copy link
Contributor

jeroen commented Jan 30, 2024

One thing I am not sure about is how this change could potentially affect other R packages. I can think of a situation where this might break things: if some package is building an executable as an intermediate step, then running it on the host machine to generate some further output to be included in the package. I don't actually know if any R packages work like that - @jeroen, can you think of any?

The maps package is one example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants