-
Notifications
You must be signed in to change notification settings - Fork 308
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
fix: incremental compilation always compile the common-version crate #4605
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4605 +/- ##
==========================================
- Coverage 84.93% 84.65% -0.28%
==========================================
Files 1096 1096
Lines 196699 196740 +41
==========================================
- Hits 167060 166545 -515
- Misses 29639 30195 +556 |
@@ -19,7 +19,7 @@ use build_data::{format_timestamp, get_source_time}; | |||
use shadow_rs::{CARGO_METADATA, CARGO_TREE}; | |||
|
|||
fn main() -> shadow_rs::SdResult<()> { | |||
println!("cargo:rerun-if-changed=.git/refs/heads"); | |||
println!("cargo:rerun-if-changed=../../../.git/refs/heads"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is a fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found rust-lang/cargo#4213
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My programmable solution for share:
if let Ok((dir, _)) = gix_discover::upwards(Path::new(env!("CARGO_MANIFEST_DIR"))) {
let git_refs_heads = dir.as_ref().join(".git/refs/heads");
println!("cargo::rerun-if-changed={}", git_refs_heads.display());
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with the gix library and not sure if it will work with git submodules.
It's best to keep it simple. I can improve it once I'm sure it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Record a code snippet for handling submodule:
fn configure_rerun_if_head_commit_changed() {
let mut current = Path::new(env!("CARGO_MANIFEST_DIR")).to_path_buf();
// skip if no valid-looking git repository could be found
while let Ok((dir, _)) = gix_discover::upwards(current.as_path()) {
match dir {
gix_discover::repository::Path::Repository(git_dir) => {
unreachable!(
"build.rs should never be placed in a git bare repository: {}",
git_dir.display()
);
}
gix_discover::repository::Path::WorkTree(work_dir) => {
let git_refs_heads = work_dir.join(".git/refs/heads");
println!("cargo::rerun-if-changed={}", git_refs_heads.display());
break;
}
gix_discover::repository::Path::LinkedWorkTree { work_dir, .. } => {
current = work_dir
.parent()
.expect("submodule's work_dir must have parent")
.to_path_buf();
continue;
}
};
}
}
@Byron hopefully this is the correct way to "discover the git repository path that is not a submodule".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just re-read the API docs and could only see that it can be a submodule, but it could also be another worktree checkout.
gix-discover
can only give an idea of what it probably is, and to know for sure one would have to open the repository and perform more checks.
However, with modern Git repositories, the git_dir
of LinkedWorkTree
should look like …/modules/<name>
, so one could additionally try to find the modules
subdirectory to be reasonably sure it's a submodule worktree you are looking at.
…reptimeTeam#4605) fix: wrong cargo:rerun
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
I found a problem:
Each incremental build compiles the common-version crate and its dependent crates.
This causes each incremental build to take a very long time and puts pressure on my limited disk space.
Checklist