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 README.md from cargo change tracking #995

Merged
merged 1 commit into from
Jul 17, 2020

Conversation

tjkirch
Copy link
Contributor

@tjkirch tjkirch commented Jul 17, 2020

In packages using cargo-readme, README.md files are touched by build.rs, which
triggers cargo's change tracking in the next build. This means cargo is
rebuilding the package every time.

This change adds README.md to the exclude list in Cargo.toml files so that
cargo doesn't think a README change requires a rebuild. Most of our packages
use cargo-readme to generate README.md, and for packages that don't, this still
isn't harmful; manual updates to README.md don't need a Rust build. (Special
cases can just remove this exclude line.)

The downside is that accidental changes to README.md, like removing it, won't
be automatically corrected with a rebuild if you haven't touched anything else.
This should be rare, and will correct itself upon any other change. (And a
developer is unlikely to commit such an accidental change.)


Thanks to @bcressey for finding this!

Testing done:

Without this, I observed crates with cargo-readme (and their local dependencies) rebuilding every cargo build.

With this, cargo build finishes immediately in successive attempts.

Also, at least for me and Zac, cargo make was rebuilding the os package every time, which is not a quick one. With this change, the build-packages step finishes very quickly and the only work we're redoing is image builds.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

In packages using cargo-readme, README.md files are touched by build.rs, which
triggers cargo's change tracking in the next build.  This means cargo is
rebuilding the package every time.

This change adds README.md to the exclude list in Cargo.toml files so that
cargo doesn't think a README change requires a rebuild.  Most of our packages
use cargo-readme to generate README.md, and for packages that don't, this still
isn't harmful; manual updates to README.md don't need a Rust build.  (Special
cases can just remove this exclude line.)

The downside is that accidental changes to README.md, like removing it, won't
be automatically corrected with a rebuild if you haven't touched anything else.
This should be rare, and will correct itself upon any other change.  (And a
developer is unlikely to commit such an accidental change.)
@tjkirch tjkirch requested review from bcressey and zmrow July 17, 2020 22:42
Copy link
Contributor

@webern webern left a comment

Choose a reason for hiding this comment

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

Nice find.

@tjkirch tjkirch merged commit f64dcd4 into bottlerocket-os:develop Jul 17, 2020
@tjkirch tjkirch deleted the exclude-readme branch July 17, 2020 23:13
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.

4 participants