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

Cargo: produce deterministic filenames for `build --test` and `test --no-run` #1924

Open
bruno-medeiros opened this Issue Aug 20, 2015 · 51 comments

Comments

Projects
None yet
@bruno-medeiros
Copy link

bruno-medeiros commented Aug 20, 2015

I'm doing IDE integration, and I want my IDE to be able to debug the test binaries - for which the IDE has to launch those programs itself, it shouldn't use cargo test. However (unlike --binfor example) neither build --test nor test --no-run produce a deterministic filename, but something random like test1-748b2ca97d589628.exe.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Aug 21, 2015

The problem Cargo has to deal with here is that you can test many targets which are all named the same. For example you could have a binary called foo, a library called foo, and an integration test also called foo. All of these cases need to generate binaries with unique names so they can coexist.

I do agree though it's annoying to have to basically do a diff of the output directory before and after a test run to see what binaries were generated.

@bruno-medeiros

This comment has been minimized.

Copy link
Author

bruno-medeiros commented Aug 21, 2015

Why not just put the test binaries in their own directory then, like target/debug/tests/ ?
Or prefix the executable name with test. so that integration test foo would generate target/debug/test.foo.exe ? (the target name can't have a dots, so there can't be a conflict if test. is used as file prefix)

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Aug 21, 2015

The problem is that you can have multiple binaries called foo. We'd need prefixes like test.bin, test.lib, test.test, etc.

@bruno-medeiros

This comment has been minimized.

Copy link
Author

bruno-medeiros commented Aug 24, 2015

The problem is that you can have multiple binaries called foo. We'd need prefixes like test.bin, test.lib, test.test, etc.

Which is a perfectly fine solution, no?
Or alternatively, this minor variant of the above:
test.bin.<binName>, test.lib.<libName>, test.<testName>
which is less verbose for the names of the binaries of the test targets.

@bruno-medeiros

This comment has been minimized.

Copy link
Author

bruno-medeiros commented Aug 24, 2015

Yet another solution would be to have the Cargo build targets all share the same namespace. Seems like a fine solution if Cargo was just starting out, but at this point it would be a breaking change - so not ideal, I reckon...

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Aug 25, 2015

There may be a more ergonomic possibility than test.$type.$name, but overall I think it would work out in terms of disambiguation at least.

@bruno-medeiros

This comment has been minimized.

Copy link
Author

bruno-medeiros commented Sep 15, 2015

There may be a more ergonomic possibility than test.$type.$name, but overall I think it would work out in terms of disambiguation at least.

Then how about just test.$testname, and make bin and lib a reserved/invalid test name for custom Cargo test targets?

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Sep 15, 2015

Hm actually now that I think about it we probably want to continue the pattern of <name>-* so previous scripts which use that kind of glob today will continue to work, and that means the names could be something like <name>-test.<kind>. I think it's fine to leave the <kind> in there as reserving names may be a bit late in the game at this point.

@bruno-medeiros

This comment has been minimized.

Copy link
Author

bruno-medeiros commented Sep 15, 2015

I think it's fine to leave the in there as reserving names may be a bit late in the game at this point.

Well, Rust may be 1.0, but Cargo is only version 0.2, so it can be argued that a minor breaking change would be acceptable. But I don't know how strict your policy aims to be with regards to breaking changes in Cargo.
(personally, I'm not that fussed about it either way, as long as it becomes a deterministic naming scheme)

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Sep 16, 2015

Despite Cargo's version it actually needs to be quite stable today, so I'd prefer to keep at least roughly the same pattern that we have today.

@FlorentBecker

This comment has been minimized.

Copy link

FlorentBecker commented Nov 29, 2015

How about (additionnally?) making cargo run --no-test output the name of the executables it builds on stdout? This way, cargo test --no-run -q gives the path to the executable (rebuilding it if needed).

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Nov 30, 2015

@FlorentBecker my preference here would be to just have deterministic filenames for now, but that's a possible alternative if it doesn't work out!

@adrianheine

This comment has been minimized.

Copy link

adrianheine commented Jan 4, 2016

I'd have to say that I would prefer to have a way to get the file name from cargo instead of being able to guess it myself.

@posborne

This comment has been minimized.

Copy link

posborne commented Feb 24, 2016

I have to agree with @adrianheine and @FlorentBecker. It would be very nice to have cargo give the output of test discovery. To make the output parseable in scripts and the like, I would propose that something like a cargo test --print or cargo test --discover be added that simply prints the relative path to each test executable. This option could be used along with others to limit the scope of the discovery.

Currently, I am working on adding infrastructure to nix based on libc. Unlike libc, nix does not currently have most of its tests built separately in a single executable. Right now, after cross compiling the executables there is no great way to get the paths to the test executables and pass execute them in qemu. I can glob but even then it involves a repetition of the individual tests that are in the root Cargo.toml.

@bruno-medeiros

This comment has been minimized.

Copy link
Author

bruno-medeiros commented Feb 24, 2016

@alexcrichton If compatibility with existing build scripts is important (implying the continuing of the pattern of <name>-*) , why not simply generate the deterministic filenames when an additional flag (--simple-filenames?) is passed to cargo test? This way existing build scripts work the same, and only new tools use the additional flag.

@posborne 's additional suggestion of enabling Cargo to output the filenames is good as well. It enables tools to figure out the executables without having to be able to parse Cargo.toml and figure out the Cargo targets beforehand. But this suggestion should be an addition, not the main solution, there should still be a way to generate static/deterministic filenames.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Feb 25, 2016

Yeah at this point I think that we should probably just invent some scheme that has predictable names but lacks hashes. Along those lines I'd be fine with basically anything that kept the convention of <name>-* and then the * was just replaced by something deterministic.

@bruno-medeiros

This comment has been minimized.

Copy link
Author

bruno-medeiros commented Mar 1, 2016

Hum, so like <name>-test for integration-style tests, <name>-test.lib for lib tests and <name>-test.bin for bin tests?

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 1, 2016

That sounds pretty reasonable to me, yeah!

@jan-hudec

This comment has been minimized.

Copy link

jan-hudec commented Mar 17, 2016

<name>-test.lib, <name>-test.bin

I would perhaps avoid the ., which might be somewhat confusing on Windows, and suggest <name>-lib-test and <name>-bin-test. With -test always at end—or beginning—so that *-test (test-*) matches all test binaries.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 17, 2016

Unfortunately using -test as a suffix or prefix can conflict with other targets. For example if you have binaries called test-foo-lib and foo-lib-test, then we can't generate a binary for a corresponding library called foo (because - is valid in crate names). What's the confusion on Windows you're thinking of though?

@jan-hudec

This comment has been minimized.

Copy link

jan-hudec commented Mar 17, 2016

Then <name>-test for the tests under tests/ wouldn't work either, right?

The confusion under Windows I feared was that Windows by default hide the extension in various places and don't do it in a particularly consistent way. And because .bin and .lib are commonly used as suffixes, sometimes you might be looking at foo-test.lib.exe and confuse it for foo-test.lib or vice versa.

If you want to have . in the names to avoid conflict, I would then suggest: test.name(.exe), test.bin.name(.exe) and test.lib.name(.exe). These would keep the advantage that test.* is all tests, the . should still prevent conflicts and the name at the end would be unlikely to resemble a real extension.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 18, 2016

Hm this actually gives me an idea! So I'd like to keep the same names we have today in case any scripts are relying on them, but we could perhaps do something like:

  1. Generate all executables into a tests/ directory with deterministic names
  2. Hard-link all those executables up one level to where they are today with today's names

That way we could slowly phase out the old locations and we could continue to use the new ones today!

@bruno-medeiros

This comment has been minimized.

Copy link
Author

bruno-medeiros commented Mar 18, 2016

The confusion under Windows I feared was that Windows by default hide the extension in various places and don't do it in a particularly consistent way.

If that's confusing, Windows users should disable the Windows Explorer option that hides file extensions. I suspect most Windows developers and power users do that already anyways (I certainly do). And IDEs certainly don't hide extensions.

@bruno-medeiros

This comment has been minimized.

Copy link
Author

bruno-medeiros commented Mar 18, 2016

@alexcrichton Yeeeesh, hard links... you sure that's a good idea? Sounds like a really heavy handed solution. Do all the OSes that Rust targets have filesystems that support hard links? What if one wants to use a filesystem that doesn't support hard links? Imagine for example someone has a Rust project on a portable FAT32 USB stick or something like that. Or a network share? It's a really odd and rare scenario, but I think it might realistically happen.
What library would you even use to create hard links in a portable way, does such a thing exist in Rust?

@kamalmarhubi

This comment has been minimized.

Copy link
Contributor

kamalmarhubi commented Mar 18, 2016

Re library to use, std::fs::hard_link() exists :-)

@bruno-medeiros

This comment has been minimized.

Copy link
Author

bruno-medeiros commented Mar 18, 2016

@kamalmarhubi Oh cool, didn't know about that.
I still think it's not a clean solution though, it's better to fix this in a way that works independently of the underlying file system.

@kamalmarhubi

This comment has been minimized.

Copy link
Contributor

kamalmarhubi commented Mar 18, 2016

What if one wants to use a filesystem that doesn't support hard links? Imagine for example someone has a Rust project on a portable FAT32 USB stick or something like that.

I haven't thought about if hardlinks are the right approach, but this is a real concern. I think between FS support and confusing Explorer defaults, I'd prefer the confusion over reduced FS support.

@DemiMarie

This comment has been minimized.

Copy link

DemiMarie commented Mar 29, 2016

One approach is for Cargo to output all of the filenames that it generates in a JSON file.

@jsgf

This comment has been minimized.

Copy link
Contributor

jsgf commented Sep 24, 2016

I'm working on #3111 to address #3109 but it looks like it would help this use case too.

cbiffle added a commit to cbiffle/cargo that referenced this issue Sep 25, 2016

Fall back to fs::copy when hard_link fails.
Some filesystems don't allow hard links.  Since Cargo's use of hard
links is an optimization, and not necessary for correctness, it can fall
back to a file copy when hard linking is not available.

This is one possible solution to rust-lang#1924.

Caveat: this will try to copy if the hard link fails *for any reason*.
It's not clear that there's a more surgical way of handling this; Unix
tends to indicate the condition as "permission denied," not with a
granular "links not supported by filesystem" error.
@jan-hudec

This comment has been minimized.

Copy link

jan-hudec commented Sep 27, 2016

@jsgf, this is a bit different. At least shared libraries must contain the metadata part for versioning. But the examples and tests don't and it would be much more useful if they didn't, which is what this issue is asking for.

The use-case I have in mind is testing examples. A test would be written that would execute the example and check its output. For which it can use cargo run --example in native builds, but in cross-compiles cargo is unreachable—so cargo manifest does not help.

@dbrgn

This comment has been minimized.

Copy link
Contributor

dbrgn commented Mar 28, 2017

I also ran into this when trying to collect test binaries for coverage testing.

I used this command to find the binaries after cargo test --no-run:

find target/debug/deps/ -maxdepth 1 -type f -executable -regextype sed -regex '.*/[a-z_]*-[0-9a-f]*'

The thing is, this returns multiple binaries, since i have a crate that provides both a lib.rs and a main.rs. In addition to that, I have integration tests that also produce a binary.

My workaround currently is to do the following:

  • Run cargo build
  • Run the find command above, store the found binaries
  • Run cargo test --no-run
  • Run the find command again
  • Remove the first set of binaries from the second set of binaries

Better support for this would be welcome.

@ssokolow

This comment has been minimized.

Copy link

ssokolow commented Mar 28, 2017

Mine runs cargo clean and then uses cargo test --no-run, so coverage makes Rust's compile times all the more painful and iterating on test coverage a big hassle.

@matklad

This comment has been minimized.

Copy link
Member

matklad commented Mar 28, 2017

@dbrgn, @ssokolow cargo test --no-run --message-format=json produces a list just built binaries in JSON format.

@ssokolow

This comment has been minimized.

Copy link

ssokolow commented Mar 28, 2017

Hmm. I'll have to think about that.

I don't mind using a Python shebang in my justfile for my own projects, but, for the authoritative copy of the boilerplate, the only external scripting language it currently uses for tasks is bash and I'm reluctant to add to the list of dependencies just to parse JSON.

@dbrgn

This comment has been minimized.

Copy link
Contributor

dbrgn commented Mar 28, 2017

Oh, that is awesome in combination with jq!

$ cargo test --no-run --message-format=json | jq -r "select(.profile.test == true) | .filenames[]"

But I agree with @ssokolow about the added dependencies.

@matklad

This comment has been minimized.

Copy link
Member

matklad commented Mar 28, 2017

Yeah, Cargo currently has some awesome support for dumping all sorts of interesting info as JSON (http://doc.crates.io/external-tools.html), but it's not very convenient to use from bash. Of course it's relatively easy to create and install custom subcommands to deal with JSON, if you are OK with adding dependencies.

By the way, @bruno-medeiros, the original problem of "I need a way to run cargo run / cargo test" from the IDE is going to be solved by #3670 via #3866 soon.

@raphlinus

This comment has been minimized.

Copy link
Contributor

raphlinus commented Apr 20, 2017

We (Fuchsia) really want a solution to this, so we can generate cross-compiled test binaries using a GN build and, and then use other (non-cargo) mechanism to copy the binaries over to a device and run them there.

My sense, having explored a bunch of options, is that the best way to do this is to add a flag to cargo rust that allows overriding the -extra-filename option (default -). I'll start putting together a PR for this now, but in the meantime would love feedback about whether this seems like a reasonable approach.

@jan-hudec

This comment has been minimized.

Copy link

jan-hudec commented Apr 25, 2017

@raphlinus, just keep in mind that there can be targets of different kinds, but same name, so the suffixes have to be type-specific.

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Jan 22, 2018

The Cargo team discussed this issue briefly in our meeting today, and we'd be happy to take a PR for it!

@ArtemGr

This comment has been minimized.

Copy link

ArtemGr commented Oct 29, 2018

Just a note that JSON isn't a solution for VSCode launch.json / "program", AFAIK. We'd really need a fixed binary path there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.