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

Exclude target directory from Time Machine #4386

Merged
merged 1 commit into from
Aug 10, 2017
Merged

Conversation

kornelski
Copy link
Contributor

Fixes #3884

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@kornelski
Copy link
Contributor Author

Tests fail on http_auth_offered, which looks like a flaky test unrelated to this change.

@alexcrichton
Copy link
Member

I don't believe the test is flaky but rather related to the upgrade of libgit2-sys here. Mind either updating the test or leaving the dep as it was?

Cargo.toml Outdated
@@ -53,6 +53,7 @@ url = "1.1"

[target.'cfg(unix)'.dependencies]
openssl = "0.9"
core-foundation = { version = "0.4.4", features = ["mac_os_10_7_support"] }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be in a cfg that only gets pulled in on OSX?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the syntax for this? I've tried [target.'cfg(macos)'.dependencies], but it didn't get picked up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah looks like you found it!

use core_foundation::base::TCFType;

// For compatibility with 10.7 a string is used instead of global kCFURLIsExcludedFromBackupKey
let is_excluded_key: Result<string::CFString, _> = "NSURLIsExcludedFromBackupKey".parse();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can perhaps avoid some indentation issues here with:

let is_excluded_key: string::CFString = match ... {
    Ok(k) => k,
    Err(_) => return,
};
// ...

@kornelski
Copy link
Contributor Author

I didn't notice libgit got updated.

I've tried to commit only lines of cargo.lock that had core-foundation, but it broke the file (parse error), so I committed the whole thing.

@alexcrichton
Copy link
Member

Looks like this fails to compile on stable/beta?

@@ -12,7 +12,7 @@ mod imp {

pub fn read2(mut out_pipe: ChildStdout,
mut err_pipe: ChildStderr,
mut data: &mut FnMut(bool, &mut Vec<u8>, bool)) -> io::Result<()> {
data: &mut FnMut(bool, &mut Vec<u8>, bool)) -> io::Result<()> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a change on master. Github is displaying diff incorrectly. I'll rebase.

Cargo.toml Outdated
@@ -35,7 +35,7 @@ home = "0.3"
ignore = "^0.2.2"
jobserver = "0.1.6"
libc = "0.2"
libgit2-sys = "0.6"
libgit2-sys = "=0.6.12"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't commit this = change

Temporary/derived files outside dedicated system directories should be explicitly excluded from backups to prevent undesirable bloat and churn.
@kornelski
Copy link
Contributor Author

Passes after fixes and rebase

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 10, 2017

📌 Commit 8e0a7ca has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Aug 10, 2017

⌛ Testing commit 8e0a7ca with merge 8c6b080...

bors added a commit that referenced this pull request Aug 10, 2017
Exclude target directory from Time Machine

Fixes #3884
@bors
Copy link
Contributor

bors commented Aug 10, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 8c6b080 to master...

@bors bors merged commit 8e0a7ca into rust-lang:master Aug 10, 2017
@kornelski kornelski deleted the backups branch August 10, 2017 11:51
jstasiak added a commit to jstasiak/cargo that referenced this pull request Jun 18, 2020
This patch follows the lead of rust-lang#4386 (which excludes target directories
from Time Machine backups) and is motived by the same reasons listen
in rust-lang#3884. CACHEDIR.TAG is an OS-independent mechanism supported by Borg,
restic, GNU Tar and other backup/archiving solutions.

See https://bford.info/cachedir/ for more information about the
specification. This has been discussed in Rust Internals earlier this
year[1] and it seems like it's an uncontroversial improvement so I went
ahead with the patch.

[1] https://internals.rust-lang.org/t/pre-rfc-put-cachedir-tag-into-target/12262/11
bors added a commit that referenced this pull request Jul 2, 2020
Exclude the target directory from backups using CACHEDIR.TAG

This patch follows the lead of #4386 (which excludes target directories
from Time Machine backups) and is motived by the same reasons listen
in #3884. CACHEDIR.TAG is an OS-independent mechanism supported by Borg,
restic, GNU Tar and other backup/archiving solutions.

See https://bford.info/cachedir/ for more information about the
specification. This has been discussed in Rust Internals earlier this
year[1] and it seems like it's an uncontroversial improvement so I went
ahead with the patch.

One thing I'm wondering is whether this should maybe cover the whole main target directory (right now it applies to `target/debug`, `target/release` etc. but not to target root).

[1] https://internals.rust-lang.org/t/pre-rfc-put-cachedir-tag-into-target/12262/11
@ehuss ehuss added this to the 1.21.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cargo's temp files cause backup churn on macOS
5 participants