Unify build directories. by ehuss · Pull Request #6668 · rust-lang/cargo (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation16 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This unifies the deps, build, incremental, and .fingerprint directories, pulling them out of the debug/release directories. This lays the groundwork so that #6577 can more easily share build artifacts across debug/release. Final artifacts remain linked into debug/release.
For backwards compatibility, tests and benchmarks are still stored in target/{debug,release}/deps. This is because it is not uncommon for tests to use current_exe to find executable binaries.
Misc details:
- Removed the
nativedirectory, I'm fairly certain it is not used. - Removed the naming restriction for
build,incremental, andnative.examplesanddepsare still restricted. - Added
doctoLayoutnow thatLayoutowns the top-level structure. Other things likepackageand.metabuildcould be added later, just to keep everything in one place.
r? @Eh2406
(rust_highfive has picked a reviewer for you, use r? to override)
This may be a disruptive change, so I'd like to make sure this is a good layout for the future, or if a different strategy would be better. There is some flexibility to change things around (for example, we could make it .deps to be hidden, or use some other, more descriptive name). The directories could also be nested together.
As I mentioned earlier, this slightly breaks rustbuild, but the fix is simple.
I feel like the backwards compatibility for tests is a little inelegant. Not sure if there's a better approach.
Continuing discussion from #6577:
directory junctions are supported much further back on Windows, right?
Directory junctions are supported back to Windows 2000. However, they are absolute-paths only, and only work on NTFS. std does not have a way to create them (it has one for internal tests, though). symlink_dir creates symlinks, hard_link creates file-only hardlinks, and directory junctions are a 3rd concept. It would be possible to try directory junctions, but I fear that would cause issues.
I feel like this is a good next step on eventually moving to a separate global cache, if that is the direction the team decides to go.
I see two concerns:
- How much breakage is this going to make? How much shim code to we need for backwards compatibility? How backwards compatible to we want to be? Can we test this somehow? Opt in for testing? A crater run?
- Are we going to have to add shims for this behavior when we make changes in the future? Witch is hard as the team has not clearly defined what we want to go.
I wonder if this is a good moment to define what is "stable" about the file structure and what is "implementation details"?
Difficult questions to answer. I think it will definitely break a few projects. I just did some searching, and I'm unfortunately finding a large number of projects digging in the deps dir (over 40), which is making me reconsider this strategy. ☹️
I'm not sure how to proceed. It would be depressing to have undocumented behavior be perma-stable because people are using it, and it would also be frustrating to break a bunch of projects.
I could change it to hard-link everything into the old deps dir. I worry about filesystems that don't support hard-links, in which case you'd have two copies of everything. I could go with the symlink/directory-junction approach, but that means Windows wouldn't support non-NTFS filesystems.
Another approach would be to segregate build-dependencies, and hard-link the shared ones. That will be more difficult to implement, but might be a good outcome.
Could also just give up on trying to share, and just say build scripts need to be compiled separately. That will make some projects very unhappy, though.
Hmm... 🤔 Anyone with ideas?
Since we're wondering about possible breakage here, I think it'd be most actionable to go from concrete breakage to develop a plan to roll this out. @ehuss you mentioned that you saw some projects digging around in deps, and would it be possible to categorize those accesses into how the folder is currently being used?
I think with that we might be able to develop a strategy to roll this out (for example do we roll it out with warnings on internals? do we have an unstable flag and opt-in for awhile before switching defaults? do we switch immediately, use hard links where possible, and warn on internals? etc/etc).
☔ The latest upstream changes (presumably #6687) made this pull request unmergeable. Please resolve the merge conflicts.
I did a survey of projects that appear to be accessing inside deps: https://gist.github.com/ehuss/7e3de6b732d276bb9639de11bf911af3
This is not a complete survey, but maybe gives an idea of what is being used. It might be interesting to consider these will also likely break with a global, shared cache. How disruptive does that survey look?
I don't see flags as being a good way to transition. An unstable flag would only be usable on nightly, and I feel it would be unlikely that projects would support both modes simultaneously.
I'm considering a different approach of keeping build dependencies separate, but with a fallback mechanism so that it would use the deps in debug or release if they are available and compatible. That might be less disruptive. I could also implement it in such a way to naturally support a global shared cache. One concern is that it would not play nicely if custom profiles are ever added. Can anyone think of other drawbacks or considerations?
I don't have time to dig in at the moment, but I want to make it clear that With some transition plan I like the general goule.
Thanks for collecting that info @ehuss!
I continue to pretty strongly feel that we should continue to find a way forward with this change or something pretty similar. I don't want us to get into a situation where we feel shackled to make changes because you're definitely right that doing something like a global cache is going to cause problems regardless, but I feel like those problems are worth the tradeoff of the benefits we'd otherwise get.
Along those lines I see this as a "game" of figuring out how to make the impact as reasonable as possible. A good number of those crates, for example, are largely just interested in the "final artifact", but something that isn't handled well. For example the "final artifact" may include something via RUSTFLAGS or cargo rustc which is currently just defacto buried away somewhere.
Another category seems to be about executing rustc to do things like more doctests or compiletest. Thankfully though there's a small handful of foundational crates we can fix in one way or another to get them working again.
I wonder if we could take a plan of attack like so?
- Prepare the change in target directory layout, but make it internally conditional on a boolean switch.
- Send a post to internals, detailing:
- A description of the change, as well as motivation
- Expected fallout we expect to see, and how to fix it
- Initial ideas of how to fix the fallout
- On nightly default the boolean switch to "use the new scheme", but provide a very easy (env var, cargo config, etc) way to turn it back to the old scheme.
From there we'd have some clear messaging saying that a change is coming as well as a way to force experimentation with the new scheme (but a way to locally fix builds if necessary). Depending on the fallout and schemes we have for fixing things we could then continue to evaluate. If it breaks everything and everyone's unhappy then we revert the boolean switch and figure out a different strategy. If it breaks lots and there's enthusiasm to fix, then it'll fix itself in time. Or maybe we have very little breakage in practice!
How's that sound?
| } else if unit.target.is_custom_build() { |
|---|
| self.build_script_dir(unit) |
| } else if unit.target.is_example() { |
| self.layout(unit.kind).examples().to_path_buf() |
| } else if unit.mode == CompileMode::Test | |
| self.layout(unit.kind).legacy_deps().to_path_buf() |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that tests & benches should be treated as "public" outputs: it's not uncommon to launch test executable manually, to attach gdb. Should we use the following layout?
target/release
examples/
tests/
benches/
bins & libs
?
for example, we could make it .deps to be hidden, or use some other, more descriptive name
I think we should do this. The main problem with the current situation is that we don't actually say which things inside target are public API and which are private impl details. So a name like .cache or .cargo-private or .internal would be useful: it won't prevent users from poking into internals, if they have two, but they hopefully will see that this is not covered by stability guarantees.
Status: The marked PR is awaiting some action (such as code changes) from the PR author.
label
5 tasks
bors added a commit that referenced this pull request
Layout docs and cleanup.
This extracts some changes from #6668. There shouldn't be any behavior changes other than the native directory will no longer be created.
What needs to be done regarding this? One thing that would be nice to see is the deps having their own folder. That way, I can easily delete everything under target on CI except for the target/deps and then cache it based on Cargo.toml. This will reduce a lot of CI cache problems rust projects have.
Currently clap does this.
13 tasks
I'm not sure how to proceed. It would be depressing to have undocumented behavior be perma-stable because people are using it, and it would also be frustrating to break a bunch of projects.
With some transition plan I like the general goal
Could cargo set a DEPS or TARGET_DEPS variable in the build script that shows where the artifacts will be output to? That can be rolled out on nightly for a while before the change, allowing cargo projects to transition without breaking any crates. Then once most crates have transitioned, cargo could switch the directory to the new format.
I'm going to close this PR for now. I'm still interested in pursuing this, but it will take more work than I have time for now.
Labels
Status: The marked PR is awaiting some action (such as code changes) from the PR author.