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

CACHEDIR.TAG is not created if the target already exists #12441

Open
tklajnscek opened this issue Aug 3, 2023 · 5 comments
Open

CACHEDIR.TAG is not created if the target already exists #12441

tklajnscek opened this issue Aug 3, 2023 · 5 comments
Labels
A-caching Area: caching of dependencies, repositories, and build artifacts C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@tklajnscek
Copy link

tklajnscek commented Aug 3, 2023

Problem

If a target directory already exists, even if empty, cargo will not create CACHEDIR.TAG in it.
This will break any tools that depend on CACHEDIR.TAG being in there.

This fails for both the default ./target dir as well as custom --target-dir.

EDIT:

Here's our specific case where this fails all the time:
When I don't have the target dir created and first open my project in VSCode it's setup to re-route all rust-analyzer target dirs to stop the RA builds & checks and regular builds constantly trampling on each other like so in settings.json:

"rust-analyzer.check.extraArgs": ["--target-dir=target/rust-analyzer"],
"rust-analyzer.cargo.buildScripts.overrideCommand": ["cargo", "check", "--quiet", "--workspace", "--message-format=json", "--all-targets", "--target-dir=target/rust-analyzer"],

So when RA runs, it creates the ./target/rust-analyzer dir, which means that the next time that cargo runs it doesn't create the CACHEDIR.TAG because /target already exists...

Steps

  1. Pick any cargo project (just the default init crate should do)
  2. Create the target dir manually, or delete it's contents after a build manually
  3. Build
  4. Observe lack of CACHEDIR.TAG

Possible Solution(s)

If there's no specific reason for this just make sure to create CACHEDIR.TAG if it doesn't exist already.

Notes

We currently use this to find the target directory of the root package when a subpackage is being built. We need to get some version info from there so that we can download the matching C libraries dependencies. Is there a better way to do this than scanning up the tree for a known file such as CACHEDIR.TAG?

Version

cargo 1.71.0 (cfd3bbd8f 2023-06-08)
release: 1.71.0
commit-hash: cfd3bbd8fe4fd92074dfad04b7eb9a923646839f
commit-date: 2023-06-08
host: aarch64-apple-darwin
libgit2: 1.6.4 (sys:0.17.1 vendored)
libcurl: 7.88.1 (sys:0.4.61+curl-8.0.1 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1t  7 Feb 2023
os: Mac OS 13.4.0 [64-bit]
@tklajnscek tklajnscek added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Aug 3, 2023
@weihanglo
Copy link
Member

weihanglo commented Aug 3, 2023

From my understanding, for an existing target directory, Cargo doesn't know if it is safe to be excluded from backup. Hence avoid adding CACHEDIR.TAG.

We currently use this to find the target directory of the root package when a subpackage is being built. We need to get some version info from there so that we can download the matching C libraries dependencies. Is there a better way to do this than scanning up the tree for a known file such as CACHEDIR.TAG?

This looks like a misuse of CACHEDIR.TAG. It is not for marking a directory is target directory, nor Cargo guarantees that. If you just want to know the path of the target directory, perhaps cargo metadata is a better option. Something like cargo metadata | jq -r .target_directory.

@weihanglo weihanglo added S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-triage Status: This issue is waiting on initial triage. labels Aug 3, 2023
@tklajnscek
Copy link
Author

tklajnscek commented Aug 3, 2023

From my understanding, for an existing target directory, Cargo doesn't know if it is safe to be excluded from backup. Hence avoid adding CACHEDIR.TAG.

It still seems a strange choice since it's very easy for an unsuspecting user to clear out the contents of the target directory, leaving the directory itself alive, then do another build, which doesn't produce the same set of file as the first build. It will also break any tools that depend on it (e.g. it may start getting backed up to the cloud etc). The intent of CACHEDIR.TAG was to make it work with those things in the first place :)

This looks like a misuse of CACHEDIR.TAG. It is not for marking a directory is target directory, nor Cargo guarantees that. If you just want to know the path of the target directory, perhaps cargo metadata is a better option. Something like cargo metadata | jq -r .target_directory.

I looked into this a bit and unfortunately it won't work. I need the path to the target directory that the user specified on the root package when they started the build - the child package that's being built is a rust library that also needs to download certain other non-rust dependencies into a specific folder in the target directory where the linker will pick them up.

There doesn't seem to be a good way of doing this... I've dump the environment and it doesn't contain anything useful. I scoured a bunch of gitlab and stack overflow posts and haven't found anything useful.

The (ugly) alternative is to go and move the non-rust dependency download code into the client application and require each client to run it as part of their build script even though they don't really need to be aware or care about those as they are required by our crate which they just depend on.... overally not very user-friendly and I'm trying to avoid it...

EDIT: But this is besides the point of this post - it may still be worth doing something with CACHEDIR.TAG behavior as I don't think it's great like it is right now.

I don't see a case where the target-dir is not a cache/temp directory and should be included in backup. Cargo will dump a bunch of stuff in there which I doubt anyone will want kept around...

Is there a known use case for such a thing? Setting --target-dir to be /some/important/existing/dir?

If not, it's probably worth adding a "Hey, your target directory already exists and doesn't seem to be a cache directory. Are you sure you want to build into it? We don't want to spam a random directory on your hard drive..." or something along those lines. Or just "making" it a cache directory...

@weihanglo
Copy link
Member

I need the path to the target directory that the user specified on the root package when they started the build

I am sorry cargo metadata didn't work. What did you mean by "user-specified"? Could we give more details about your use case?

it's very easy for an unsuspecting user to clear out the contents of the target directory, leaving the directory itself alive

It doesn't look easy to me, since a user is encouraged to use cargo clean but manual delete. I believe it could go either way (intentionally deleted CACHEDIR.TAG but Cargo added it back v.s. unintentionally), so your example also makes sense.

However, before diving into the solution space, maybe we can focus on problem space more, so that we can fix the actual problem you have :)

(or focus on CACHEDIR.TAG usage discussion, but that's not a Cargo guarantees your infra can rely on)

@weihanglo
Copy link
Member

See also #14125, which separates intermediate and final artifacts directories. Therefore we might always create CACHEDIR.TAG for intermediate artifact caches.

@weihanglo weihanglo added S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. and removed S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. labels Jul 21, 2024
@problame
Copy link

Another case where the ./target exists was pointed out in #14281 : there, rust-analyzer is configured to use a separate target directory (./target/analyzer) to reduce contention (Rationale, VSCode snippet)

If you open your VSCode on a clean checkout, chances are rust-analyzer will run before the first cargo build, hence rust-analyzer will create the ./target/ and ./target/analyzer directories. Subsequent cargo build will find the ./target dir to already exist and not create the ./target/CACHEDIR.TAG file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-caching Area: caching of dependencies, repositories, and build artifacts C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

No branches or pull requests

3 participants