Warn on duplicate artifact creation

[Mark-Simulacrum]

With a workspace such as in this repository: https://github.com/Mark-Simulacrum/cargo-issue-5524, running Cargo check on stable results in the following metadata being generated.

I believe that Cargo should try to detect the situation where duplicate artifacts will be generated and stop compilation; however, I'm not sure it's possible to do this well. I think even a heuristic that checks for lib/bin entries with the same name across the workspace would be sufficient for most of the benefits, though.

It's worth noting that I think this has only become an issue (at least for cargo check) since rust-lang/rust#49289, which made rustc --emit=metadata emit metadata for bin targets. It might be enough to make change to rustc to disambiguate binary and lib metadata in the filename. In a way, this might be a regression, but I'm not sure.

$ find target -type f -name '*rmeta' | xargs ls -trl # stable 1.26
-rw-r--r-- 2 mark mark 1058 May 11 21:35 target/debug/libfoo.rmeta
-rw-r--r-- 2 mark mark 1058 May 11 21:35 target/debug/deps/libfoo-a9f370a27c2df2eb.rmeta

However, on beta, the following occurs:

$ find target -type f -name '*rmeta' | xargs ls -trl # beta 1.27
-rw-r--r-- 1 mark mark 1221 May 11 21:36 target/debug/deps/libfoo-11803c32bdc69a09.rmeta
-rw-r--r-- 2 mark mark    0 May 11 21:36 target/debug/libfoo.rmeta
-rw-r--r-- 2 mark mark    0 May 11 21:36 target/debug/deps/libfoo-f0bc89da5bcecba3.rmeta

cc @alexcrichton -- do you think this ought to be marked as a regression? It's confusing behavior that has only become a problem since 1.27, but I don't know whether it really warrants regression-like tracking.

[matklad]

I wonder why we uplink rmeta files at all? Presumably, their format is an implementation detail, and only rust-lang tools like RLS could make use of it, and those tools should be able to read those files out of ./deps folder?

[alexcrichton]

Yeah sort of like rlibs I think we could just avoid uplinking these altogether

[dwijnand]

I think if someone were to detail how to "avoid uplinking these altogether" it might expedite a fix here. 🙂

[ehuss]

@dwijnand It's not clear (to me) how safe it is to change these behaviors. Like #6131 where someone was manually running rustc to link against rlibs created by cargo, the location of the rlibs is not easy to determine (generally need to slurp the json output to get the correct deps location). If someone has some tool which looks at the rmeta files, removing them might make things more difficult for them.

I think rls-analysis does the correct thing, but it would be good to double-check if it would be impacted as it is the primary user. In fact, I would imagine rls-analysis is the only user, so if it still works it should be fine. It would be good to know if any other tool expects those files to exist. EDIT: Oops, was thinking of something else.

The decision on what to hardlink is done here. The code relevant to rmeta and bin files is here. Essentially link_stem should return None if unit.mode.is_check(). Also, the comment about "non-lib targets" in calc_outputs should be removed since it is no longer true (although a new comment should be added that it uses the term lib even for binary targets, which is somewhat confusing).

[ehuss]

@nrc I was under the impression that rls read the rmeta files somewhere, but I don't see that anywhere. Are they used anywhere by rls? I believe rustdoc reads them, but does anything else?

[ehuss]

Oh, and as a side note, it might be nice if calc_outputs also checked if any of the paths it computes are duplicated and warn or error, since that would likely indicate a bug in cargo.

[nrc]

@ehuss it doesn't. It does cargo check and leaves everything to Cargo/rustc. It does read the save-analysis output which are JSON files in the target/deps directory