rust3ds / cargo-3ds

Cargo command to work with Nintendo 3DS project binaries.
Apache License 2.0
59 stars 10 forks source link

Set environment variables for CC and CXX to allow building C/C++ wrapper crates #41

Open adryzz opened 1 year ago

adryzz commented 1 year ago

I was trying to build imgui-rs, to make a library that provides an interface between imgui and the 3DS system, to allow for easy creation of GUI applications, and i ran into a problem. (what little code i wrote is here)

imgui-rs, like many crates that wrap a C or C++ library, uses the cc crate to build the C dependency. The problem is that when the C dependency is being built, it targets the host system's architecture, because the CC, CXX and other standard environment variables aren't being overridden by cargo-3ds.

By manually setting the following environment variables, i was able to build the imgui-rs crate, the zstd crate and other C wrappers without any issues, linking or otherwise.

With all of these set, the crates build fine with no extra warnings or anything.

Meziu commented 1 year ago

Nice idea, I didn't know there were crates with C dependencies that are able to run, but I surely wouldn't like to miss a chance at supporting them!

There are just 2 things to clarify before actually making the change (which would take minimal practical effort):

  1. We already have a small issue about changing env variables with #14. That case is exceptional (since we are talking about modifying flags that directly impact the whole Rust workflow) but the case remains that env variables aren't the most solid way to pass information upstream...
  2. Are those flags ever used by other tools/Rust itself? Modifying such generic vars may break other parts (meant for the host system in particular), but maybe that's an issue with cross-compilation in general...
adryzz commented 1 year ago

the case remains that env variables aren't the most solid way to pass information upstream

The problem is that while the cc crate does have a configurable API, the methods are only available to the build script of the package, and thus there isn't really another way from what i found.

Environment variables should be the best thing for this, as they are made to describe the environment a program runs in.

As for whether those flags are used by other code, they should be fine, but i need to check whether build dependencies of packages (that should be built for the host target, as they are part of build scripts) are affected by this change and if so, if there's another way forward.

ian-h-chamberlain commented 1 year ago

Interestingly, this also sort of relates to https://github.com/rust3ds/ctru-rs/pull/133 — In that case, the build script is figuring out the paths to the compilers using the existing DEVKITPRO / DEVKITARM variables.

As for whether those flags are used by other code, they should be fine, but i need to check whether build dependencies of packages (that should be built for the host target, as they are part of build scripts) are affected by this change and if so, if there's another way forward.

This would be my concern too, but luckily cc supports a special format for this purpose: https://github.com/rust-lang/cc-rs#external-configuration-via-environment-variables

I think it should be safe for cargo-3ds to set CC_armv6k-nintendo-3ds and CXX_armv6k-nintendo-3ds (and maybe CRATE_CC_NO_DEFAULTS_armv6k-nintendo-3ds if needed), and the default compilers would still be used for host tools. There is also TARGET_CC, but I'm not sure if that might have issues if it ever was invoked in a proc macro or something like that, so being specific is probably safer.

All that said — does this configuration need to be in cargo-3ds itself? I suppose it already has knowledge of where the toolchain lives, so it might be sensible to have in there, but you could also just add something like this into .cargo/config.toml's [env] section.

Maybe it's a little more convenient to preset the flags, but I don't think we can guarantee that every crate using cc will even build, let alone run on the 3DS, so my instinct would be to require people to set this manually ("you're on your own"). In particular, I'm not sure one set of flags (CXXFLAGS etc.) will be appropriate for every use case, although I guess the upstream 3dsvars.sh is probably a pretty good source.

@Meziu thoughts?

Meziu commented 1 year ago

This would be my concern too, but luckily cc supports a special format for this purpose: https://github.com/rust-lang/cc-rs#external-configuration-via-environment-variables

I wasn't aware of that, that clears my original doubt.

Maybe it's a little more convenient to preset the flags, but I don't think we can guarantee that every crate using cc will even build, let alone run on the 3DS, so my instinct would be to require people to set this manually ("you're on your own"). In particular, I'm not sure one set of flags (CXXFLAGS etc.) will be appropriate for every use case, although I guess the upstream 3dsvars.sh is probably a pretty good source.

@ian-h-chamberlain In my opinion, simplifying the building process (for the most common cases) is the reason why cargo-3ds exists in the first place. cargo-3ds is already a miscellaneous tool that can be easily avoided by whoever prefers to use other forms of setups (like Makefiles or even rust/cargo configuration files). There is nothing cargo-3ds does that can't be done with simple commands: its purpose is only to collect all that information and provide it in a much faster form factor.

Following that logic, I believe cargo-3ds should include those defaults as much as it includes the --target armv6k-nintendo-3ds arguments. Overall, what those changes imply is exactly what we are already doing for the Rust packages, with the only difference that this time it's for C code. We already do not have any sorts of "guarantee" that normal Rust crates will compile, and I believe I've made clear that we can't assure full compatibility over the various docs I've written (like the ctru-rs wiki).

As my original vision for this tool, we should strive for as much compatibility as possible within the boundaries of cargo-3ds, so I'm completely ok with the proposed changes.

adryzz commented 1 year ago

This would be my concern too, but luckily cc supports a special format for this purpose

Oh i didn't see that, that's great then

As for the inclusion, i agree with Meziu, the whole reason cargo-3ds exists is to be convenient

I'm gonna try writing a patch for cargo-3ds then

Meziu commented 1 year ago

By manually setting the following environment variables, i was able to build the imgui-rs crate, the zstd crate and other C wrappers without any issues, linking or otherwise.

With this setup I managed to run imgui, but not zstd (which is arguably a very useful library), because the compiler gave me some errors about int size (basically, it wanted long rather than short integers).

Did I miss something? Could you follow up on this issue?