Implictly enable -Zbuild-dir-new-layout when -Zrustdoc-mergeable-info

[ranger-ross]

What does this PR try to resolve?

Originally reported in: #16306 (comment)

-Zrustdoc-mergeable-info only supports the new build-dir layout. Currently just passing -Zrustdoc-mergeable-info alone will use the new layout while the rest of the build uses the old layout.

This commit changes the the behavior to implictly enable -Zbuild-dir-new-layout when -Zrustdoc-mergable-info is enabled.

Note that paths in the tests have also changed from foo-[HASH] to foo/[HASH].
The previous behavior of <build-dir>/<profile>/build/foo-[HASH]/deps was incorrect and not valid for both new and old layouts.

tracking issue: #16306

How to test and review this PR?

See the updated tests

r? @weihanglo

[weihanglo]

What was the actual issue / invalid state we found here?
I can see that there is some inconsistency, but not sure whether this is actually a bug (for example, cargo clean might be questionable here since the support is not yet implemented).

Also like to point out that -Zrustdoc-mergeable-info may already be used extensively by some projects due to the perf regression in rustdoc rust-lang/rust#146895. It's not clear whether those users are ready for the new layout. If we see this as a way to expose the new layout to more users, then probably fine.

[ranger-ross]

What was the actual issue / invalid state we found here?

There are 2 things:

1. For me I'd expect Cargo to either use the new or old layout for an entire invocation. It seems odd that an invocation would use both types at the same time.
2. More importantly, the current behavior is using $pkgname-$HASH in the new layout location instead of $pkgname/$HASH due to
   

   cargo/src/cargo/core/compiler/build_runner/compilation_files.rs

   Lines 248 to 251 in a793cd1

   	let separator = match self.ws.gctx().cli_unstable().build_dir_new_layout {
   	true => "/",
   	false => "-",
   	};

[weihanglo]

I see. So there is no bug but inconsistency. It doesn't do the wrong thing when interacting with the new layout, right?. It is just abusing the old build-script layout that makes us feel weird when -Zbuild-dir-new-layout is off.

Putting crate-info JSON file under new layout is intentional, but not the initial motivation. What the feature needs is a per-unit basis location so that crate info JSON file name won't collide: #16309 (comment).

What I try to avoid here is coupling two nightly features, as Zrustdoc-mergeable-info is currently the only way to mitigate the unforunate 10x slowness perf issue. If there is any reason we must do this (e.g. it complicates the crater test process or future integration) then yeah we should consider merge this.

[ranger-ross]

What I try to avoid here is coupling two nightly features

I think this is fair enough.

I suppose to re-frame things, if we decide to stabilize -Zrustdoc-mergable-info before -Zbuild-dir-new-layout we would effectively be stabilizing <build-dir>/<profile>/build/foo-[HASH]/deps as part of the old layout.

Also another motivation for this PR was in preparation for renaming deps in the new layout as mentioned in #16502 (comment).
Since -Zrustdoc-mergable-info relies on -Zbuild-dir-new-layout it means directory names would change for -Zrustdoc-mergable-info when we change them. (I actually made these changes locally and it felt weird to update the tests for -Zrustdoc-mergable-info even though the tests are not passing -Zbuild-dir-new-layout)

[weihanglo]

It is fine changing -Zrustdoc-mergable-info behavior, as it is still unstable, and we haven't yet added any cargo clean support 😆.

I know it is weird, but if no technical blocker I slightly lean towards not coupling them.

And its hard to say that which will be stabilized first. Me and Ed both want rustdoc to provide a true isolated rustdoc invocation for each unit #16309 (comment), but not sure if it is feasible for rustdoc.