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

Provide access to project paths for tests #2841

Closed
dhardy opened this issue Jul 8, 2016 · 15 comments
Closed

Provide access to project paths for tests #2841

dhardy opened this issue Jul 8, 2016 · 15 comments
Labels
A-documenting-cargo-itself Area: Cargo's documentation

Comments

@dhardy
Copy link

dhardy commented Jul 8, 2016

I need a test binary to read some data files from the project's repository and write some new files to a temporary location.

I've done this before with CMake, which provides variables like CMAKE_SOURCE_DIR, CMAKE_CURRENT_SOURCE_DIR, CMAKE_BINARY_DIR (doc) which can be substituted into "configured" source code/script files or passed to tests. The distinction between source and binary directories is important especially since the source directory may not be writable; is this also the case with Cargo?

It is possible for Cargo tests to get the path to the current binary as well as the working directory, but I'd prefer a documented way of getting the project's root directory (e.g. via an environment variable or optional command-line argument), and some recommendations on where to put temporary files.

@alexcrichton
Copy link
Member

The root directory may be accessible somehow through the file!() macro, and temporary files should probably go into env!("OUT_DIR") or ideally something managed by the test itself, Cargo doesn't really ensure that there's a temporary directory for tests. Cargo itself just shoves data into the target/ directory.

Does that help?

@dhardy
Copy link
Author

dhardy commented Jul 10, 2016

file!() just gives the path of the source relative to the project root. Doesn't help.

OUT_DIR doesn't appear to be set when the test is built:

$ cargo test --test test-suite -- --nocapture
   Compiling pippin v0.1.0 (file:///home/dhardy/other/pippin)
test-suite.rs:37:29: 37:44 error: environment variable `OUT_DIR` not defined

Same for most other environment variables which might contain useful paths, the exception being CARGO_MANIFEST_DIR which points to the "target" directory. Perhaps Cargo should define CARGO_TARGET_DIR and CARGO_PROJECT_DIR (to point to the project's top directory) when building?

I was able to get the paths I want anyway, but it's a horrible hack.

// Get absolute path to the "target" directory ("build" dir)
fn get_target_dir() -> PathBuf {
    let bin = env::current_exe().expect("exe path");
    let mut target_dir = PathBuf::from(bin.parent().expect("bin parent"));
    while target_dir.file_name() != Some(OsStr::new("target")) {
        target_dir.pop();
    }
    target_dir
}
// Get absolute path to the project's top dir, given target dir
fn get_top_dir<'a>(target_dir: &'a Path) -> &'a Path {
    target_dir.parent().expect("target parent")
}

@alexcrichton
Copy link
Member

I'm somewhat wary of just keeping to pile on tons of environment variables, in general strategies like this are pretty brittle because there's no guarantee these directories exist when the binary is actually run as any number of events could have happened between compile time and execution time.

Ah but yeah I believe OUT_DIR is only defined when there's a build script in play, and yes file!() doesn't get you absolute paths.

@dhardy
Copy link
Author

dhardy commented Jul 13, 2016

I wouldn't want to compile the output directory into a release binary, but paths are needed somehow and test binaries don't usually move. The question is which is the least brittle assumption?

  1. Do what I did in the above code: look for a parent directory named target from the executable's path
  2. Assume something about the directory structure inside target and go up a fixed number of levels from the executable's argument path
  3. Hard-code project / build directory paths (via environment variables set during compilation, or other less straightforward means)

What I'm saying is that Cargo docs should cover this somehow. I don't see anything too evil about adding a few environment variables either, so long as the documentation is kept up-to-date.

@alexcrichton
Copy link
Member

Ah yeah of those assumptions I think (2) is the least brittle, and I'd be totally fine documenting it!

@alexcrichton alexcrichton added the A-documenting-cargo-itself Area: Cargo's documentation label Jul 14, 2016
@dhardy
Copy link
Author

dhardy commented Aug 11, 2016

Since Cargo supports out-of-source builds neither options (1) or (2) will work reliably.

IMO "trying not to pile on a ton of environment variables" sounds a bit like Cargo is trying to bind the hands of developers, rather than help them. Cargo is just a tool and should include the features users need.

@ghost
Copy link

ghost commented Jan 3, 2018

EDIT: I had a look at the code, things are more complicated than I thought, so nevermind :)
DEBUG:cargo::ops::cargo_rustc: linking /home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage/target/debug/deps/recompile_self-3768b8c8127fd518 to /home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage/target/debug/recompile_self

**tl;dr:** would be nice to have: - `CARGO_WORKSPACE_DIR` - which could be a parent dir of `CARGO_MANIFEST_DIR` for example. - `CARGO_TARGET_DIR` - fullpath pointer to `target/` or to `target/debug/` ? unsure (assuming former) **EDIT:** there's already a user setable relative-path [one](https://doc.crates.io/environment-variables.html#environment-variables-cargo-reads) which is empty if unset! if it were always set and thus available, could mix with CARGO_MANIFEST_DIR then, to get fullpath. - `CARGO_PKG_OUTFILE` - output exe filename full path (or just fname, if `CARGO_TARGET_DIR` is also available)

long read:
From what I can tell, CARGO_MANIFEST_DIR and CARGO_PROJECT_DIR would be the same(if the latter were to be implemented, that is).
It's CARGO_TARGET_DIR which could differ significantly, under a workspace.
For example:
if CARGO_MANIFEST_DIR=CARGO_PROJECT_DIR=/home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage/recompile_self
then you would think that CARGO_TARGET_DIR would be equal to $CARGO_MANIFEST_DIR/target/$PROFILE (assuming you look at it from within a running build.rs, else you don't have $PROFILE it seems)
which resolves to /home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage/recompile_self/target/debug.
However, since this dir /home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage/ is a workspace dir (with its own Cargo.tml file, example here), then in actuality
CARGO_TARGET_DIR has the value $CARGO_WORKSPACE_DIR/target/$PROFILE aka /home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage/target/debug/ and NOT /home/xftroxgpx/build/2nonpkgs/rust.stuff/rustlearnage/recompile_self/target/debug
(of course CARGO_WORKSPACE_DIR does NOT exist currently as an env. var (ie. not implemented, just like CARGO_TARGET_DIR) )

If CARGO_TARGET_DIR would exist, then I could hardcode(in the exe) the exe file name like: $CARGO_TARGET_DIR/target/$PROFILE/$CARGO_PKG_NAME (though it feels hacky!), unless there's some way to specify the exe filename in Cargo.toml (which I'm sure there is, but I forgot how, currently) in which case I would kindly request a way to get that one too, at compile time. Or perhaps even better, a CARGO_PKG_OUTFILE = fullpath to exe, at compile time.

Basically CARGO_PKG_OUTFILE would be the same value as the resulting binary filename that cargo run executes.
Any pointers on where to look in the source code for implemeting this? Even if a PR won't be accepted.

As to why would I need this for my particular case: I need to hardcode the compiledexe fullpath filename so that when I detect hardlinks to this exe(by other means), I can then show a message pointing to the real exe which would be the only updated(recompiled) one. (the hardlink binary would be the outdated one)
This means std::env::current_exe will point to the same name as the hardlink (in Linux) which is why I need the hardcoded fullpath of the executable at compile time.

EDIT: ok this is what I'm gonna be using locally:

diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs
index ea9294fd..3e1f2595 100644
--- a/src/cargo/ops/cargo_rustc/mod.rs
+++ b/src/cargo/ops/cargo_rustc/mod.rs
@@ -889,6 +902,36 @@ fn build_base_args<'a, 'cfg>(cx: &mut Context<'a, 'cfg>,
     opt(cmd, "-C", "linker=", cx.linker(unit.kind).map(|s| s.as_ref()));
     cmd.args(&cx.incremental_args(unit)?);
 
+        let mut bin_outfile_full_path=Option::None;
+        for &(ref dst, ref link_dst, file_type) in cx.target_filenames(&unit)?.iter() {
+            if file_type != TargetFileType::Normal {
+                continue
+            }
+            let bindst = match *link_dst {
+                Some(ref link_dst) => link_dst,
+                None => dst,
+            };
+
+            if bin_outfile_full_path.is_some() {
+                error!("!! Already set bin_outfile_full_path='{}' This is unexpected. Should've been set only once! FIXME: investigate what caused this!",bin_outfile_full_path.clone().unwrap());
+                assert!(bin_outfile_full_path.is_none());
+            } else {
+                bin_outfile_full_path=Option::Some(bindst.display().to_string());
+                cmd.env("CARGO_TARGET_BINFILE_FULLPATH",bindst.display().to_string()); //alright this makes it work in src/main.rs with env!() macro! eg. const OUTPUT_EXE_AT_COMPILETIME: &'static str = env!("CARGO_TARGET_BINFILE_FULLPATH");
+                //the following is needed specifically for build.rs availability of the same env. var:
+                {
+                    let a=cx.compilation.extra_env.entry(
+                        unit.pkg.package_id().clone()
+                        )
+                        .or_insert_with(Vec::new);
+                    a.push(("CARGO_TARGET_BINFILE_FULLPATH".to_string(), bin_outfile_full_path.clone().unwrap_or("".to_string()))); //for build.rs availability! via eg. println!("Here's CARGO_TARGET_BINFILE_FULLPATH={}", env::var("CARGO_TARGET_BINFILE_FULLPATH").unwrap());
+                }
+                debug!("CARGO_TARGET_BINFILE_FULLPATH={}",bin_outfile_full_path.clone().unwrap_or("".to_string()));
+            }
+            //debug!("!! 3333333333333333333 {}", bindst.display());
+        }//for
+
+
     Ok(())
 }
 

any new updates will be here or permalink to old one

@stale
Copy link

stale bot commented Sep 18, 2018

As there hasn't been any activity here in over 6 months I've marked this as stale and if no further activity happens for 7 days I will close it.

I'm a bot so this may be in error! If this issue should remain open, could someone (the author, a team member, or any interested party) please comment to that effect?

The team would be especially grateful if such a comment included details such as:

  • Is this still relevant?
  • If so, what is blocking it?
  • Is it known what could be done to help move this forward?

Thank you for contributing!

(The cargo team is currently evaluating the use of Stale bot, and using #6035 as the tracking issue to gather feedback.)

If you're reading this comment from the distant future, fear not if this was closed automatically. If you believe it's still an issue please leave a comment and a team member can reopen this issue. Opening a new issue is also acceptable!

@stale stale bot added the stale label Sep 18, 2018
@dhardy
Copy link
Author

dhardy commented Sep 18, 2018

This issue is definitely stale (no active interest), but I don't believe it was solved. There are a host of environment variables available, but I don't see a documented way of accessing the source path or creating temporary files for tests.

Build scripts do have documented access to the source (the working directory) and the output (OUT_DIR), but there is no indication that this applies to test resources.

And now I've just spend five minutes summarizing a stale issue because of this bot; not sure whether that's a good or bad thing.

@stale stale bot removed the stale label Sep 18, 2018
@dwijnand
Copy link
Member

I've found summaries help move issues forward, so thank you!

@khoek
Copy link

khoek commented Apr 30, 2020

Because this was quite a while ago now: has anything changed since this issue was created?

Since Cargo has tests for itself and (presumably) modifies the filesystem during them, how does Cargo handle it?

@ehuss
Copy link
Contributor

ehuss commented Apr 30, 2020

Reading files from the project can be done by accessing env!("CARGO_MANIFEST_DIR").

For writing, using a temp directory is probably the safest choice. The tempfile crate can be useful for that.

Cargo's testsuite writes to the target directory by looking at the location of the test executable (this is an approach mentioned above). That is not necessarily the safest thing to do, and is not explicitly documented.

robyoung added a commit to robyoung/dicom-test-files that referenced this issue May 30, 2020
This is a hack which doesn't work for doc-tests but the previous
approach didn't work for workspaces. Given that the expectation is that
these files will most likely be used in integration tests I'm going with
this approach.

See: rust-lang/cargo#2841
@rubik
Copy link

rubik commented Aug 10, 2020

Has there been any progress on this? I've just hit the same issue: my tests need external data files and I need to access the path to those files from the tests.

bors added a commit that referenced this issue May 7, 2021
Add CARGO_TARGET_TMPDIR env var for integration tests & benches

Hi.
Recently I [ran into](https://github.com/vojtechkral/bard/blob/main/tests/util/mod.rs#L32) the problem that integration tests don't have a good way to figure out where `target` dir is.

Knowing where `target` is is useful for integration tests that need to setup some filesystem structure for test cases. In fact `cargo` itself does this too (and figures out the path rather clumsily).

Another popular way of doing this is to create a directory in `/tmp` (or quivalent on other systems), however, I believe using subdirectory in `target` is better as testcases are easier to debug that way and temporary  locations aren't polluted.

I think this would also address some concerns in #2841

Another solution might be to provide a dedicated subdirectory in `target` for this, something like `target/scratchpad`, but I'm not convinced this is warranted... Edit: That's what was decided to do, see below...

Let me know what you think 🙂
@vojtechkral
Copy link
Contributor

vojtechkral commented May 8, 2021

Now that #9375 is merged, I'd propose to close this issue, seems like it's been addressed.

  • To access your source files, you can use the CARGO_MANIFEST_DIR env var
  • To access a temporary location suitable for writing test files, use CARGO_TARGET_TMPDIR in integration tests (and benches). This is a compile time env var only.
    CARGO_TARGET_TMPDIR is also brand new, so it will take some weeks before it hits stable toolchains.
    In the meantime, you can use option_env!("CARGO_TARGET_TMPDIR") to detect if it's available. If not, as a fallback you can use something like env::current_exe()/../../tmp or similar as suggested before.

@dhardy
Copy link
Author

dhardy commented Jun 4, 2021

Thanks for the summary @vojtechkral. I'll close as recommended since I currently have no use-case to test with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation
Projects
None yet
Development

No branches or pull requests

7 participants