feldera icon indicating copy to clipboard operation
feldera copied to clipboard

Don't clear the full environment for compiler server invocation

Open gz opened this issue 8 months ago • 2 comments

https://github.com/feldera/feldera/commit/de249990a7792fd789e8d38be1510296d045cb3a

we clear the environment for the compiler server with this change. but now I think it would be better to just filter out the problematic variables instead of removing the entire env.

e.g., we have already this special case https://github.com/feldera/feldera/commit/f48e0ab787fc136122c3124b011e968fd79ba9b3 but there is also RUSTC_WRAPPER which I'd like to override and pass other variables to the process like SCCACHE_DIR etc. I think it's easier to filter out the bad ones rather than special casing all potential ones we'd like to include

gz avatar Apr 03 '25 00:04 gz

The filter is difficult because the environment variables used by Cargo are not easily identifiable -- , you'd think it would just be "CARGO_", but that is not the case: https://doc.rust-lang.org/cargo/reference/environment-variables.html

  • Some do start with CARGO_ and are set by Cargo (e.g., CARGO_PKG_VERSION)
  • Some do start with CARGO_ and are not set by Cargo (e.g., CARGO_MANIFEST_DIR)
  • Some don't start with CARGO_ and are set by Cargo (e.g., OUT_DIR)
  • Some don't start with CARGO_ and are not set by Cargo (e.g., NUM_JOBS)

The package originally causing the recompilation is ring, you can see what it gets from the environment: https://github.com/briansmith/ring/blob/main/build.rs -- Changing any of them, even if Cargo overrides by setting itself, will trigger recompilation. Technically this could be described as a bug with Cargo.

Could you describe the filtering behavior that would work for you?

Alternatively, I would propose the following: we add a special environment variable structure FELDERA_RUST_COMPILATION_<environment variable> where we trim away the prefix FELDERA_RUST_COMPILATION_ and set the <environment variable> as-is.

snkas avatar Apr 03 '25 09:04 snkas

maybe we filter from this list those individually that are a concern? https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-crates

alternatively maybe we do need to specify the env for the rust compiler via command line flags or renaming env variables like you suggest .. command line flags are kinda stupid though because it would mean overriding the docker entry point for CI caching which is something I can't do in github actions

gz avatar Apr 03 '25 21:04 gz