Skip to content

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
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!

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?

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
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