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 OUT_DIR and other environment variables with cargo wraps #13752

Open
bobbens opened this issue Oct 4, 2024 · 6 comments
Open

rust: set OUT_DIR and other environment variables with cargo wraps #13752

bobbens opened this issue Oct 4, 2024 · 6 comments

Comments

@bobbens
Copy link

bobbens commented Oct 4, 2024

Describe the bug
Meson can wrap Rust cargo files, however, it does not provide the same environment variables that cargo does. In particular, for my case (compiling sdl2), OUT_DIR is not provided and the build.rs which uses OUT_DIR fails to compile.

Although this is related to #10030 , it is a bit different as it focuses on cargo wraps which should be able compile like with cargo itself, and thus the environment variables shouldn't necessarily be set in the main meson.build. If #10030 is merged, whatever solution is used there at the end could be used when generating the meson.build from the Cargo.toml.

To Reproduce
Either use the small provided project cargo-fail.tar.gz or using the following file as subprojects/sdl2-rs.wrap, it will fail to company when used as dependency('sdl2-0.37-rs'):

[wrap-file]
method = cargo
directory = sdl2-0.37.0
source_url = https://crates.io/api/v1/crates/sdl2/0.37.0/download
source_filename = sdl2-0.37.0.tar.gz
source_hash = 3b498da7d14d1ad6c839729bd4ad6fc11d90a57583605f3b4df2cd709a9cd380
[provide]
dependency_names = sdl2-0.37-rs

The meson.build would be:

project('cargo-fail', 'rust', version : '0.1' )
exe = executable('cargo-fail', 'main.rs', dependencies: dependency('sdl2-0.37-rs') )

and main.rs would be:

fn main () {}

Expected behavior
It should compile like it does when compiled with OUT_DIR='..' meson compile. By default it will error out with:

$ meson compile
INFO: autodetecting backend as ninja
INFO: calculating backend command to run: /usr/bin/ninja
[4/6] Compiling Rust source ../subprojects/sdl2-sys-0.37.0/src/lib.rs
FAILED: subprojects/sdl2-sys-0.37.0/libsdl2_sys.rlib
rustc -C linker=cc --color=always -C debug-assertions=yes -C overflow-checks=no --crate-type rlib --edition=2018 -g --crate-name sdl2_sys --emit dep-info=subprojects/sdl2-sys-0.37.0/sdl2_sys.d --emit link=subprojects/sdl2-sys-0.37.0/libsdl2_sys.rlib --out-dir subprojects/sdl2-sys-0.37.0/libsdl2_sys.rlib.p -C metadata=a4e236e@@sdl2_sys@sta --cfg 'feature="default"' --extern libc=subprojects/libc-0.2.155/liblibc.rlib -Lsubprojects/libc-0.2.155 ../subprojects/sdl2-sys-0.37.0/src/lib.rs
error: environment variable `OUT_DIR` not defined at compile time
  --> ../subprojects/sdl2-sys-0.37.0/src/lib.rs:14:18
   |
14 | include!(concat!(env!("OUT_DIR"), "/sdl_bindings.rs"));
   |                  ^^^^^^^^^^^^^^^
   |
   = help: Cargo sets build script variables at run time. Use `std::env::var("OUT_DIR")` instead
   = note: this error originates in the macro `env` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 1 previous error

ninja: build stopped: subcommand failed.

system parameters

  • native build
  • linux
  • Python 3.12.6
  • meson 1.5.2
  • ninja 1.12.1
@dcbaker
Copy link
Member

dcbaker commented Oct 4, 2024

We don't support build.rs, and it's basically impossible for us to do so in an automated way because build.rs is an escape hatch, you can programmatically do anything you want in it: generate code, emit new arguments to rust and/or cargo, completely rewrite source files, find dependencies. If you need to use a crate that has a build.rs there is not choice but to hand write a meson.build to replace the build.rs file.

Which reminds me, I really need to get back to my crate to allow putting some stuff in the cargo.toml instead of the build.rs...

@bobbens
Copy link
Author

bobbens commented Oct 4, 2024

We don't support build.rs, and it's basically impossible for us to do so in an automated way because build.rs is an escape hatch, you can programmatically do anything you want in it: generate code, emit new arguments to rust and/or cargo, completely rewrite source files, find dependencies. If you need to use a crate that has a build.rs there is not choice but to hand write a meson.build to replace the build.rs file.

I perfectly understand not wanting to write a full replacement for cargo and the likes, however, I would strongly recommend at least setting some of the environment variables, as that would likely get quite good coverage over most build.rs packages. In particular, for sdl2 and likely many others, just adding OUT_DIR would be enough as doing OUT_DIR='..' meson compile is enough to get it working. This would save a lot of effort having to rewrite meson.builds given that meson gives no way to set environment variables directly.

@dcbaker
Copy link
Member

dcbaker commented Oct 4, 2024

Except it isn't. Because Meson has no idea that build.rs is writing files, what their names are, where they'll be located, when then need to be updated, and whether they need to be installed.

Where you're going to run into problems in that path is the sdl-sys crate, it's 1000 LOC, it makes decisions like whether to invoke pkg-config, vcpkg, or invoke cmake to get sdl2; it calls bindgen to generate rust sources, and it emits linker and compiler flags. the sdl2 crate itself only sets a linker search path on some of the BSDs, which Meson would ignore.

That crate in particular is the sort that you need a hand written meson.build, because there is no way that we would be able to handle everything there. build.rs is the thing that we simply cannot solve, because it is so deeply integrated into cargo implementation details. The only real solutions we have are to either provide tools to offload some of the information from build.rs into the cargo.toml (which would could then handle), or simply re-implement build.rs files as meson.build files.

@BrknRobot
Copy link

BrknRobot commented Oct 4, 2024

Because Meson has no idea that build.rs is writing files, what their names are, where they'll be located, when then need to be updated, and whether they need to be installed.

Would it make sense to support describing the output of build.rs in a way that answers these questions for Meson? This type of approach to external scripts is already supported for things like custom_target

@bobbens
Copy link
Author

bobbens commented Oct 5, 2024

Except it isn't. Because Meson has no idea that build.rs is writing files, what their names are, where they'll be located, when then need to be updated, and whether they need to be installed.

I understand that writing a custom meson.build would be the ideal solution for dependency tracking and the likes, However, I may be naive, but shouldn't meson understand OUT_DIR and have some sort of idea where the package output should go? Alternatively, if meson just does not want to support build.rs, refusing to work with the package when detected would be another potential solution. It just feels rather weird that it tries to compile with build.rs yet doesn't define OUT_DIR which seems to be quite prevalent.

Currently, given that it is not possible to set env vars with meson in general or for build_targets, it is not even possible to write a custom meson.build to solve this package, short of using custom_targets or an auxiliary script to do it. Even assuming that setting OUT_DIR fails, it is unlikely something would break because it is defined, while if it isn't defined, anything that relies on it will fail to compile.

@eli-schwartz
Copy link
Member

Would it make sense to support describing the output of build.rs in a way that answers these questions for Meson? This type of approach to external scripts is already supported for things like custom_target

No, custom_target doesn't operate the same way at all.

custom_target allows you to exit the meson build graph, enter an external command, and expect that external command to produce predefined output artifacts. Some people use this today with rust: they run cargo build as a custom_target. There are various shortcomings to this approach, e.g. you cannot tie artifacts together, but running cargo will, indeed, set OUT_DIR. And run build.rs exactly as-is.

custom targets cannot "answer questions for meson". Meson knows the custom_target command, the input files, and the output files -- all of them are written in meson.build itself. That command line then acts similar to the gcc/clang compiler command line from an executable(). It is precomputed, written into the build graph, has meson-defined inputs and meson-defined outputs, and is expected to keep quiet and do what it is told.

There is exactly one question that a custom_target command can be asked and respond with an answer to -- and it is the same question that gcc / clang can be asked, and which rustc can be asked. It is: "can you provide a depfile containing a list of headers that when modified should trigger a recompile".

I understand that writing a custom meson.build would be the ideal solution for dependency tracking and the likes, However, I may be naive, but shouldn't meson understand OUT_DIR and have some sort of idea where the package output should go? Alternatively, if meson just does not want to support build.rs, refusing to work with the package when detected would be another potential solution. It just feels rather weird that it tries to compile with build.rs yet doesn't define OUT_DIR which seems to be quite prevalent.

Does it indeed try to compile with build.rs? ;)

It's difficult to know when a source file contains OUT_DIR without opening every source file for scanning, which isn't actually possible to do on every git pull.

Also note that setting environment variables at all will cause every single compiler invocation to become really really slow. That's why in the linked PR, the discussion became linked to the proposal to stabilize --env-set in rust itself. Which unfortunately appears to have died on the rustc side of things.

If it was low-cost to set environment variables with rust then it might be practical to set OUT_DIR automatically. There is simply no way whatsoever we are going to make every single rustc invocation in meson go through a python wrapper to set environment variables just on the off chance that they are needed. Even if we merge the PR to add an env: kwarg to rust executables that does use a python wrapper, it would have to be opt-in precisely because of that performance hit.

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

No branches or pull requests

4 participants