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

Don't rewrite dep-info files if they don't change #5181

Merged
merged 1 commit into from
Mar 14, 2018

Conversation

alexcrichton
Copy link
Member

Similar to how we treat lock files, read the contents, compare, and if they're
the same don't actually write the file.

Closes #5172

Similar to how we treat lock files, read the contents, compare, and if they're
the same don't actually write the file.

Closes rust-lang#5172
@rust-highfive
Copy link

r? @matklad

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

Copy link
Member

@matklad matklad left a comment

Choose a reason for hiding this comment

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

LGTM, though it might be worth to consider extracting deinfo-related stuff into a separate struct

previous[0].1 == deps {
continue
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps it's time to add a

#[derive(Eq)]
struct DepInfo { ... }

impl DepInfo {
  fn parse(r: Read) -> CargoResult<Self>;
  fn print(w: Write) -> CargoResult<Self>;
}

it would be nice to have printing and the corresponding parsing code alongside each other.

@alexcrichton
Copy link
Member Author

@bors: r=matklad

Makes sense! I don't think we'll be doing this too often though so I'm gonna go ahead and land for now, and we can follow up w/ an abstraction if we get more ambitious

@bors
Copy link
Contributor

bors commented Mar 14, 2018

📌 Commit f605676 has been approved by matklad

@bors
Copy link
Contributor

bors commented Mar 14, 2018

⌛ Testing commit f605676 with merge 1eb4c8f...

bors added a commit that referenced this pull request Mar 14, 2018
Don't rewrite dep-info files if they don't change

Similar to how we treat lock files, read the contents, compare, and if they're
the same don't actually write the file.

Closes #5172
@bors
Copy link
Contributor

bors commented Mar 14, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing 1eb4c8f to master...

@bors bors merged commit f605676 into rust-lang:master Mar 14, 2018
@alexcrichton alexcrichton deleted the selectively-write-dep-info branch April 25, 2018 19:28
@ehuss ehuss added this to the 1.26.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.

depfile generation always rewrites depfiles even for null builds
5 participants