cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Environment variable for Cargo Workspace

Open Ygg01 opened this issue 7 years ago • 44 comments

T-cargo notes:

A CARGO_RUSTC_CURRENT_DIR is added as a nightly only environment variable. See https://github.com/rust-lang/cargo/issues/3946#issuecomment-1832514876. Seek for feedback.


Hi, while working on using workspace in html5ever, I've ran into issue of needing the CARGO_WORKSPACE directory, and being unable, to find it. What I resorted to is essentially, &Path(cargo_manifest).join("..") which feels hacky.

Could CARGO_WORKSPACE be added as environment variable? I'm not sure what it should be when there is no workspace defined, I assume it should either return Err or default it to CARGO_MANIFEST_DIR.

Sidenote I'm willing to work on this issue, if I could get quick pointers, to what I need to do.

Ygg01 avatar Apr 24 '17 09:04 Ygg01

when there is no workspace defined

I'd expect the environment variable to not be set at all in that case.

shepmaster avatar Apr 24 '17 13:04 shepmaster

What would adding the CARGO_WORKSPACE entail. From cursory look, I see custom_build.rs has reference to Context, that has reference to Workspace, but same doesn't exist for compilation.rs.

Would having CARGO_WORKSPACE only for custom builds be ok?

Ygg01 avatar Apr 24 '17 14:04 Ygg01

Thanks for the report! I think I may not quite be following what's going on here though? Do you mean accessing the workspace directory from a build script perhaps?

alexcrichton avatar Apr 24 '17 21:04 alexcrichton

@alexcrichton Yes. I was looking for workspace directory in my custom build script. It's related to servo/html5ever#261.

There is a simple workaround of taking CARGO_MANIFEST_DIR and using its parent, but I thought having CARGO_WORKSPACE would be cleaner solution.

Ygg01 avatar Apr 24 '17 21:04 Ygg01

Oh yeah definitely makes sense to me! Seems reasonable to basically enhance this section

alexcrichton avatar Apr 26 '17 15:04 alexcrichton

So if I understand correctly, if I expose workspace.root_manifest that would be workspace dir of all projects in workspace, correct? Then I can just:

if let Some(workspace_dir) = cx.ws.root_manifest() { 
    cmd.env("CARGO_WORKSPACE_DIR", workspace_dir);
}

Ygg01 avatar May 04 '17 10:05 Ygg01

sounds about right! I think you may not want precisely the root_manifest field but rather ws.root().join("Cargo.toml")

alexcrichton avatar May 04 '17 14:05 alexcrichton

@alexcrichton I am total newb, but won't adding Cargo.toml to workspace root, return the location of workspace manifest as opposed to directory of the directory in which workspace manifest is?

Ygg01 avatar May 08 '17 14:05 Ygg01

oh sure yeah, it just depends on the intent of what's being conveyed (the workspace manifest or the directory of the workspace), I'm fine with either.

alexcrichton avatar May 08 '17 16:05 alexcrichton

Hm, while writing tests, I've noticed a peculiarity. I assume I'm using this wrong. But I wanted to double check

let p = project("foo")
    .file("Cargo.toml", r#"
        [project]
        name = "foo"
        version = "0.5.0"
        authors = []


        [workspace]
        members = ["a"]
    "#)
    .file("src/lib.rs", "")
    .file("build.rs", r#"
        fn main() {
            //panic!("WILL FAIL");
        }
    "#)
    .file("a/Cargo.toml", r#"
        [project]
        name = "a"
        version = "0.5.0"
        authors = []
        links = "foo"
        build = "build.rs"
    "#)
    .file("a/src/lib.rs", "")
    .file("a/build.rs", r#"
        fn main() {
                panic!("PASSES?");
        }
    "#);
assert_that(p.cargo_process("build").arg("-v"),
                execs().with_status(0));

The panic in a/build.rs never happens, and panic in build.rs always happens, even if workspace doesn't have a build= "build.rs" line.

Idea behind tests was to verify that each member build.rs has appropriate value for environment variables.

Ygg01 avatar May 08 '17 22:05 Ygg01

@Ygg01 oh cargo build on a workspace doesn't execute all build scripts, and build.rs is inferred to be a build script if otherwise not specified.

alexcrichton avatar May 09 '17 15:05 alexcrichton

@alexcrichton Is there an alternative way to test env. variables are properly set in each member build script?

Ygg01 avatar May 09 '17 15:05 Ygg01

@Ygg01 I think you'd just cargo build in both directories, right? And then have an assert in the build script the env var is correct?

alexcrichton avatar May 09 '17 18:05 alexcrichton

Yes, I think that is correct (one call to cargo build in member directory and one call in workspace directory). Are there any examples of such code? I can only see adding files to ProjectBuilder file.

Ygg01 avatar May 10 '17 10:05 Ygg01

Oh you'll just want to call p.cargo multiple times basically, there's some other tests I believe which run cargo more than once.

alexcrichton avatar May 10 '17 18:05 alexcrichton

This was assigned to me to summarize why we didn't merge my PR which would have closed it #4787.

We decided to punt on this feature because of the question about what happens when building a crate downloaded from crates.io, which is no longer in a workspace in that form, but might have been originally produced in a workspace and have a build script that expects to have this env var set.

Its also unclear what the motivation for this variable is; my motivation was to find the lockfile, but I concluded that the best way to get the information I was getting from the lockfile was to run cargo metadata rather than actually read from Cargo.lock.

withoutboats avatar Jan 23 '18 00:01 withoutboats

Its also unclear what the motivation for this variable is;

One motivation would be to find the absolute path of the resulting binary executable. How I'm currently doing it. EDIT: added the above.

run cargo metadata rather than

hey that's pretty cool:

$ pwd
/home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage/recompile_self
$ time cargo metadata --format-version 1 | json_reformat | grep workspace_root
    "workspace_root": "/home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage"

real	0m1.054s
user	0m0.847s
sys	0m0.214s

ghost avatar Jan 23 '18 07:01 ghost

@withoutboats my original motivation for this feature was when html5ever, was moving from one project per workspace to multiple. Namely some tests that were specific, became shared and not in the same directory they were left.

However fact that almost no one needed this feature, and it was easily implementable by other means, kinda made me think it's not needed.

I did forgot about it completely.

Ygg01 avatar Jan 23 '18 12:01 Ygg01

I also have another use case for this feature: I'm trying to get the absolute path to the source file being compiled. I embed this as metadata from a procedural macro invocation so that source can be copied during a subsequent cargo run into a particular directory structure to integrate it into a third party, language-agnostic user interface.

When using a workspace, the file!() macro expands to be workspace directory relative (I was expecting it to be CARGO_MANIFEST_DIR relative, though, to be honest). Without a way to get the workspace directory, I have to rely on a hacky method of walking up the file!() path until I hit root, popping sub-directories off of CARGO_MANIFEST_DIR as I go.

Of course, there could easily be a better way to get the source file's absolute path that I'm completely ignorant of, so please let me know if that's the case. Thanks!

peterhuene avatar Aug 12 '18 18:08 peterhuene

I also ran into this problem where file! returns a workspace relative path but I cannot absolutize it which I need for some test related situations.

mitsuhiko avatar Jan 15 '19 09:01 mitsuhiko

This is the workaround i have in place now which is pretty ugly: https://github.com/mitsuhiko/insta/blob/b113499249584cb650150d2d01ed96ee66db6b30/src/runtime.rs#L67-L88

mitsuhiko avatar Jan 15 '19 10:01 mitsuhiko

Seems like calling cargo metadata is the only way currently. Basically what @mitsuhiko done above is

  1. Call cargo metadata --format-version=1
  2. Parsing it as JSON
  3. Retrieving field workspace_root

So, in a ~nut~shell that would look like

cargo metadata --format-version=1 | jq .workspace_root

I'm a bit concerned about output size of cargo metadata though. It's ~1.6MB on my project:

$ cargo metadata --format-version=1 | wc -c
 1717575

folex avatar Oct 29 '20 11:10 folex

This lack of envvar also came back to my mine when investigating this regression: https://github.com/rust-lang/cargo/issues/8992

mitsuhiko avatar Dec 17 '20 20:12 mitsuhiko

So should we just make CARGO_WORKSPACE_DIR be convenience for workspace_root from cargo metadata --format-version=1?

Ygg01 avatar Dec 22 '20 16:12 Ygg01

Seems like calling cargo metadata is the only way currently.

I think this also works:

dirname "$(cargo locate-project --workspace --message-format plain)"

The --workspace flag is from #8712.

I'm a bit concerned about output size of cargo metadata though. It's ~1.6MB on my project:

$ cargo metadata --format-version=1 | wc -c
 1717575

cargo locate-project --workspace --message-format plain has the benefit that it prints only the path in plain text format (JSON format can be printed by removing --message-format plain), without additional redundant informations.

ghost avatar Dec 22 '20 16:12 ghost

I think people understand that they can't rely on CARGO_WORKSPACE_DIR in build.rs because of the reason specified above. But there are lot of other scenarios other than build.rs where people want this env var. Would be nice if one of the PRs get accepted and in documentation, we note that using it in build.rs would not work.

pksunkara avatar Mar 13 '21 17:03 pksunkara

Is there any reason that an environment variable for workspace dir never happened? It seems pretty aligned with the rest of the environment variable and is already exposed via the CLI.

wycats avatar Sep 14 '21 14:09 wycats

Here's a bit of a silly workaround. Create a '.cargo/config.toml' file at the root of your workspace and set this as an environment variable:

[env]
CARGO_WORKSPACE_DIR = { value = "", relative = true }

I don't think this is a good excuse to not have this built in, but at least it's a stop-gap for those who need it. This only works because of the "relative = true" which prepends the path that the ".config" folder is in to the beginning of the variable.

RandomInsano avatar Nov 18 '21 18:11 RandomInsano

Another use case for this is the expect-test crate which has problems in some set ups without this env var: https://github.com/rust-analyzer/expect-test/issues/33

Veykril avatar Jul 19 '22 10:07 Veykril

Any updates on this?

jjant avatar Dec 03 '22 16:12 jjant