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

Duplicate artifact tracking issue. #6313

Open
Tracked by #4 ...
ehuss opened this issue Nov 13, 2018 · 58 comments
Open
Tracked by #4 ...

Duplicate artifact tracking issue. #6313

ehuss opened this issue Nov 13, 2018 · 58 comments
Assignees
Labels
A-layout Area: target output directory layout, naming, and organization C-tracking-issue Category: A tracking issue for something unstable. S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.

Comments

@ehuss
Copy link
Contributor

ehuss commented Nov 13, 2018

#6308 added a check if multiple jobs produce the same files. To be safe, it is currently a warning. At some point in the future it should be turned into a hard error if no problems are reported. Collisions are almost certainly bad behavior that will cause problems, so rejecting it is probably the right thing to do.

Related issues:

Known situations where collisions may occur:

Notes on implementation issues:

  • Cargo is hard-coded with the outputs that rustc produces. If those outputs change, it will not be able to catch those changes. In particular, Warn on duplicate artifact creation #5524 would not have been caught by these checks.
  • OutputFile is currently not calculated correctly in some cases. Known issues:
  • Not all outputs are checked. Such as:
    • The .d dep info file. This uses the same hash as the main artifact, so is unlikely to be a problem.
    • Incremental files. One hopes that the hashes used in rustc are good enough?
    • Other temp files created by rustc, like rcgu files, which should always include the hash.
    • Anything done by build scripts.
  • cargo doc has a dedicated path for detecting collisions. If cargo doc is ever updated to support multiple crates with the same names, this code path can be removed.
@ehuss ehuss added the C-tracking-issue Category: A tracking issue for something unstable. label Nov 13, 2018
@ehuss ehuss self-assigned this Nov 13, 2018
@kamathba
Copy link

kamathba commented Dec 7, 2018

I have an interesting scenario where we are getting this warning that feels a little like a false positive.

We have a project that we can build with x86_64-unknown-linux-gnu or x86_64-unknown-linux-musl. We use a docker container (rust-musl-cross) typically when building for the musl target, since the dependencies are a little harder to set up on everyone's machine. This is also how we build/package in CI. When building with the gnu taget, our output folders are typically target/debug and target/release. When building for the musl target, the output folders are namespaced under target/x86_64-unknown-linux-musl. This diagnostic considers it a warning if using the gnu target I run:

cargo build --release -Z unstable-options --out-dir target/release

Now the why for us doing this has to do more with either nuances or our misunderstandings of how the cargo-deb and cargo-rpm packages work. Since we only ever package from the musl toolchain, we could probably hard-code the paths to target/x86_64-unknown-linux-musl/release/foo inside our Cargo.toml files, but the above scenario seems like a detectable situation where you've selected the output directory that things were going to end up in anyways. It also manifests as a confusing warning:

(using x86_64-unknown-linux-gnu)
cargo build --release -Z unstable-options --out-dir target/release/ 
warning: `--out-dir` filename collision.
The lib target `foo` in package `foo v0.2.0 (/home/bkamath/foo/foo)` has the same output filename as the lib target `foo` in package `foo v0.2.0 (/home/bkamath/foo/foo)`.
Colliding filename is: /home/bkamath/foo/foo/target/release/libfoo.rlib
The exported filenames should be unique.

The same warnings happen for bin targets as well. The two things seemingly being compared in this case aren't created by different targets/build.

@ehuss
Copy link
Contributor Author

ehuss commented Dec 8, 2018

@kamathba Can you clarify some things about your use case for me? I'm trying to understand why you are using --out-dir at all. Are you manually setting the list of files to be packaged? It looks like with cargo-deb you can just specify target/release paths and it rewrites them when cross compiling, so using --out-dir shouldn't be necessary. Can you explain more why you need it?

@kamathba
Copy link

kamathba commented Dec 8, 2018

@ehuss I think its mostly because of us using both the x86_64-unknown-linux-gnu and x86_64-unknown-linux-musl targets and we want everything to "just work". Also, we use cargo-rpm, which doesn't seem as smart about picking up cross-compile settings and has limited options (which we could probably fix).

All that said, I'm more wondering why cargo can't elide when the --out-dir is the same as the directory it was going to place things in anyways?

As I'm digging into this answer, I'm wondering why our builds default to building for x86_64-unknown-linux-musl even though we don't specify it in .cargo/config. We'd like to not use --out-dir, as its one of the only reasons we aren't on stable now that clippy/rustfmt landed.

qnighy added a commit to qnighy/honeybadger-rs that referenced this issue Feb 2, 2019
wehlutyk added a commit to wehlutyk/infuse that referenced this issue Feb 12, 2019
@jjpe
Copy link

jjpe commented Feb 20, 2019

As described here I have a crate in which I'd like to produce both a dylib and a cdylib file.

This seems impossible at the moment. Specifically, Cargo writes both dylibs to the same filename i.e. it seems to write one and then it seems to overwrite it with the other (can't tell you which is which without digging into the final file though).
If it is possible to have Cargo produce both cdylib and dylib, I've yet to find out how, as adding
crate-type = ["cdylib", "dylib"] to the [lib] section of the crate gives a compile warning leading to this issue.

@Lokathor
Copy link

Lokathor commented Apr 4, 2019

I'm trying to produce rlib, staticlib, and cdylib just to make sure that all modes are buildable (because I've had some tricky build problems once or twice already) and I'm getting a warning from cargo test that told me to come here.

Also, cargo test fails to run if I don't run cargo build first because it tries to link to target/debug/deps/my_lib.dll and fails to find it.

@ehuss
Copy link
Contributor Author

ehuss commented Apr 4, 2019

@Lokathor Can you share a minimum example to demonstrate? I'm not sure how to reproduce your issue. Also, which platform are you on?

@Lokathor
Copy link

Lokathor commented Apr 4, 2019

how about a build log?
here's Linux, https://travis-ci.org/Lokathor/thorium/jobs/515790083

the same general warning happens for the mac build too, which you can also find there

on windows, https://ci.appveyor.com/project/Lokathor/thorium/builds/23603097/job/sia07tfchtthym5a (im not 100% sure this is the exact same commit, but it should be)

I don't know how to make a more minimal example. I actually don't even care except for the scary warning that it might become an error later.

@ehuss
Copy link
Contributor Author

ehuss commented Apr 4, 2019

@Lokathor OK, I see what is happening. It is a consequence of having cdylib and panic="abort" with an integration test and a binary. For reasons, cdylibs don't have a unique hash added to their filename. In this case, the "thorium" library needs to be built twice (once with panic="abort" for main.rs, and once without for the test which doesn't allow panic="abort"), and both of those end up with the same filename.

I can't think of an easy workaround for you, unfortunately.

I'm not sure what the best approach here is. Perhaps dylibs could be placed in unique directory names. Also, maybe Cargo could be more conservative what it builds for tests, since it is only linking against one lib type.

@Lokathor
Copy link

Lokathor commented Apr 4, 2019

I only need the dylib for debug and/or release, and even then only if the dynamic_link feature is enabled. It's just a development ability, not meant to be shipped.

Unfortunately, cargo features are currently quite boring and cannot control much. What I would like is if features could control more.

bors added a commit that referenced this issue Jun 3, 2019
Catch filename output collisions in rustdoc.

This does some general cleanup so that rustdoc output is computed correctly.  This allows the collision detection code to work.  There are some known issues with rustdoc creating collisions (rust-lang/rust#61378, rust-lang/rust#56169).  This is related to issue #6313.

This includes a change that `CompileMode::is_doc` no longer returns `true` for doc tests. This has bothered me for a while, as various points in the code was not handling it correctly. Separating it into `is_doc` and `is_doc_test` I think is better and more explicit.

Note for reviewing: There is a big diff in `calc_outputs`, but this is just rearranging code. Everything in `calc_outputs_rustc` is unchanged from the original.

The only functional changes should be:
- A warning is emitted for colliding rustdoc output.
- Don't create an empty fingerprint directory for doc tests.
@martinellison
Copy link

If you are going to make something a terminating error, could you please make sure that the message sets out the steps that we programmers should follow to fix the error. Otherwise it is like "You have been bad. Compilation prohibited. Goodbye." and then you get all these issues and forum posts.

@SimonSapin
Copy link
Contributor

Documenting Servo produces lots of warnings like:

warning: output filename collision.
The lib target `parking_lot` in package `parking_lot v0.8.0` has the same output filename as the lib target `parking_lot` in package `parking_lot v0.7.1`.
Colliding filename is: /repo/target/doc/parking_lot/index.html
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.

This simply becoming a hard error is not acceptable. Having multiple crates with the same name in the dependency graph is a use-case that Cargo needs to support. Even this warning arguably should not exist: this situation is normal, it’s up to Cargo and rustdoc together to figure something out.

@SimonSapin
Copy link
Contributor

Collisions are almost certainly bad behavior that will cause problems, so rejecting it is probably the right thing to do.

In this type of reasoning, please consider: who’s behavior is bad? Who is responsible to fix it? Warnings and errors are only appropriate when the end user is responsible.

Even if the current output is arguably wrong/incomplete, erroring would be a regression.

@ehuss
Copy link
Contributor Author

ehuss commented Jun 25, 2019

@SimonSapin It is a bug. This won't become a hard error until most of the bugs are resolved. Part of the reason of this warning is to ferret out these bugs. Currently it is randomly stomping on files depending on which goes first, so the current behavior is wrong. This warning is just letting you know something is wrong.

We can reword the warning to convey "this is a bug" in the situations where it is a bug. It might be difficult to detect some of the scenarios.

Unfortunately fixing the rustdoc case will be difficult. I think it will require significant changes to rustdoc in order to restructure the directory layout, and have cargo communicate to rustdoc how to link to multiple versions.

bors added a commit that referenced this issue Aug 13, 2019
Adjust warning for rustdoc filename collision.

The existing warning was a little misleading, as this is a known bug, not something we currently plan to reject.

cc #6313 (comment)
@ehuss ehuss added the A-layout Area: target output directory layout, naming, and organization label Dec 30, 2019
@swarnimarun
Copy link

I am curious is there a way to select which one of the colliding packages should be used in the doc gen in case of being in a secondary dependency especially if they are two different versions of the same package?

The logs seem to suggest both packages got documented. But only the first one seems to be there. Is there something more to it?

@SimonSapin
Copy link
Contributor

We can reword the warning to convey "this is a bug"

@ehuss Yes, rewording would be good. As a user of cargo doc when I see this:

The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <https://github.com/rust-lang/cargo/issues/6313>.

This message is very strongly suggesting that I did something wrong and I should consider changing the names of "targets". (What are targets in this context? Do I even have any control over them?)

But there is nothing wrong with having multiple versions of the same crate in a dependency graph. Cargo was designed from the very beginning to support that scenario.

So it is not users who are doing something wrong, but the historical design of rustdoc (or is it cargo doc?) who incorrectly assumes that crate names are unique and can be used as-is as directory names.

For an eventual fix, maybe cargo doc could detect this situation and add a version number the directory names in that case. cargo vendor already does this.

Until then, yes then emitting a message to warn users of the data loss is better the silently overwriting. But blaming users (with a "we’ll break your stuff" threat!) is very much not the right response.

@ehuss
Copy link
Contributor Author

ehuss commented Apr 15, 2020

is there a way to select which one of the colliding packages should be used in the doc

Unfortunately, no. The one that appears in the final output can be random.

@ehuss
Copy link
Contributor Author

ehuss commented Apr 15, 2020

@SimonSapin Can you explain which situation generated that message with cargo doc? I have updated the message for some of the other situations to make it clearer it is a bug in Cargo, and what is wrong (and are hopefully less "blamey"). But I can't think offhand from that message what scenario causes that more generic message with cargo doc. Maybe a workspace with multiple members with the same executable names?

maybe cargo doc could detect this situation and add a version number the directory names in that case

That is the fix, someone just needs to work on it. It is non-trivial, and requires changes to rustdoc. Cargo has to relay the mapping of dependencies to directory names to rustdoc, and that is difficult, and requires some design work for the interface.

0xFable added a commit to White-Whale-Defi-Platform/white-whale-core that referenced this issue Mar 28, 2024
Avoids this problem

warning: output filename collision.
The bin target `schema` in package `vault-manager v0.1.0 (vault-manager)` has the same output filename as the bin target `schema` in package `epoch-manager v0.1.0 (epoch-manager)`.
Colliding filename is:
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <rust-lang/cargo#6313>.
0xFable added a commit to White-Whale-Defi-Platform/white-whale-core that referenced this issue Apr 4, 2024
Avoids this problem

warning: output filename collision.
The bin target `schema` in package `vault-manager v0.1.0 (vault-manager)` has the same output filename as the bin target `schema` in package `epoch-manager v0.1.0 (epoch-manager)`.
Colliding filename is:
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <rust-lang/cargo#6313>.
unStone added a commit to unStone/ssstone-tools that referenced this issue Apr 10, 2024
@Hywan
Copy link

Hywan commented Jul 11, 2024

I don't have this error with rust nightly 2024-06-24, but I've the issue with 2024-07-11. Worst, the warning ends up with an error in a build script because of the filename collision. I don't find any issues mentioning this so far.

@yiranzai

This comment was marked as off-topic.

elBoberido added a commit to elBoberido/iceoryx2 that referenced this issue Jul 15, 2024
elBoberido added a commit to elBoberido/iceoryx2 that referenced this issue Jul 15, 2024
elBoberido added a commit to elBoberido/iceoryx2 that referenced this issue Jul 15, 2024
elBoberido added a commit to elBoberido/iceoryx2 that referenced this issue Jul 15, 2024
github-merge-queue bot pushed a commit to CQCL/hugr that referenced this issue Jul 16, 2024
This avoids the error due to duplicated target names, as the binary
`hugr` has the same name as the main library `hugr`.

`cargo doc` showed this warning:
```
warning: output filename collision.
The bin target `hugr` in package `hugr-cli v0.1.4 (/home/runner/work/hugr/hugr/hugr-cli)` has the same output filename as the lib target `hugr` in package `hugr v0.8.0 (/home/runner/work/hugr/hugr/hugr)`.
Colliding filename is: /home/runner/work/hugr/hugr/target/doc/hugr/index.html
The targets should have unique names.
This is a known bug where multiple crates with the same name use
the same path; see <rust-lang/cargo#6313>.
``` 

This caused the resulting docs to produce non-deterministic output, with
either target overriding the other.
See [cargo#6313](https://www.github.com/rust-lang/cargo/issues/6313).
DanielVoogsgerd added a commit to DanielVoogsgerd/brane that referenced this issue Jul 23, 2024
Having two identically named binaries is a warning at the moment and
will probably become an error at a later point.

See also:
rust-lang/cargo#6313
tarcieri added a commit to RustCrypto/traits that referenced this issue Sep 30, 2024
The `crypto` crate is linking old versions of the trait crates, and
that's causing collisions when building the workspace rustdoc:

    warning: output filename collision.
    The lib target `aead` in package `aead v0.6.0-rc.0 (/Users/tony/src/RustCrypto/traits/aead)` has the same output filename as the lib target `aead` in package `aead v0.5.2`.
    Colliding filename is: /Users/tony/src/RustCrypto/traits/target/doc/aead/index.html
    The targets should have unique names.
    This is a known bug where multiple crates with the same name use
    the same path; see <rust-lang/cargo#6313>.
    warning: output filename collision.
    The lib target `cipher` in package `cipher v0.5.0-pre.7 (/Users/tony/src/RustCrypto/traits/cipher)` has the same output filename as the lib target `cipher` in package `cipher v0.4.4`.
    Colliding filename is: /Users/tony/src/RustCrypto/traits/target/doc/cipher/index.html
    The targets should have unique names.
    This is a known bug where multiple crates with the same name use
    the same path; see <rust-lang/cargo#6313>.

This changes the CI config to exclude it from the rustdoc build for now,
since everything else is on a prerelease series.

We should probably bump `crypto` to link the latest prereleases soon,
but for now this gets CI green again.
tarcieri added a commit to RustCrypto/traits that referenced this issue Sep 30, 2024
The `crypto` crate is linking old versions of the trait crates, and
that's causing collisions when building the workspace rustdoc:

    warning: output filename collision.
The lib target `aead` in package `aead v0.6.0-rc.0
(/Users/tony/src/RustCrypto/traits/aead)` has the same output filename
as the lib target `aead` in package `aead v0.5.2`.
Colliding filename is:
/Users/tony/src/RustCrypto/traits/target/doc/aead/index.html
    The targets should have unique names.
    This is a known bug where multiple crates with the same name use
    the same path; see <rust-lang/cargo#6313>.
    warning: output filename collision.
The lib target `cipher` in package `cipher v0.5.0-pre.7
(/Users/tony/src/RustCrypto/traits/cipher)` has the same output filename
as the lib target `cipher` in package `cipher v0.4.4`.
Colliding filename is:
/Users/tony/src/RustCrypto/traits/target/doc/cipher/index.html
    The targets should have unique names.
    This is a known bug where multiple crates with the same name use
    the same path; see <rust-lang/cargo#6313>.

This changes the CI config to exclude it from the rustdoc build for now,
since everything else is on a prerelease series.

We should probably bump `crypto` to link the latest prereleases soon,
but for now this gets CI green again.
andrei-21 added a commit to getlipa/lipa-lightning-lib that referenced this issue Oct 4, 2024
warning: output filename collision.
The lib target `pocketclient` in package `pocketclient-mock v0.1.0 (~/3l/mock/pocketclient)` has the same output filename as the lib target `pocketclient` in package `pocketclient v0.1.0 (~/3l/pocketclient)`.
Colliding filename is: ~/3l/target/debug/libpocketclient.rlib
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <rust-lang/cargo#6313>.
andrei-21 added a commit to getlipa/lipa-lightning-lib that referenced this issue Oct 7, 2024
warning: output filename collision.
The lib target `pocketclient` in package `pocketclient-mock v0.1.0 (~/3l/mock/pocketclient)` has the same output filename as the lib target `pocketclient` in package `pocketclient v0.1.0 (~/3l/pocketclient)`.
Colliding filename is: ~/3l/target/debug/libpocketclient.rlib
The targets should have unique names.
Consider changing their names to be unique or compiling them separately.
This may become a hard error in the future; see <rust-lang/cargo#6313>.
@RivenSkaye
Copy link

RivenSkaye commented Oct 16, 2024

Just got linked to this issue when running cargo publish for packages in a workspace, right after a final check with cargo clippy --workspace. I presume due to both commands making the same calls on their verification passes?

Edit: in both instances it's for a lib-only package where the manifest lists both "lib" and "rlib" in its crate-types. Makes sense because the linkage docs describe both as a Rust library, but perhaps it'd make sense to have cargo silently ignore the alias pointing to the same thing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout Area: target output directory layout, naming, and organization C-tracking-issue Category: A tracking issue for something unstable. S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.
Projects
None yet
Development

No branches or pull requests