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

refactor: remove unnecessary Option in Freshness::Dirty #13361

Merged
merged 5 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/cargo/core/compiler/fingerprint/dirty_reason.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ pub enum DirtyReason {
FsStatusOutdated(FsStatus),
NothingObvious,
Forced,
/// First time to build something.
FreshBuild,
}

trait ShellExt {
Expand Down Expand Up @@ -131,6 +133,11 @@ impl fmt::Display for After {
}

impl DirtyReason {
/// Whether a build is dirty because it is a fresh build being kicked off.
pub fn is_fresh_build(&self) -> bool {
matches!(self, DirtyReason::FreshBuild)
}

fn after(old_time: FileTime, new_time: FileTime, what: &'static str) -> After {
After {
old_time,
Expand Down Expand Up @@ -257,6 +264,7 @@ impl DirtyReason {
s.dirty_because(unit, "the fingerprint comparison turned up nothing obvious")
}
DirtyReason::Forced => s.dirty_because(unit, "forced"),
DirtyReason::FreshBuild => s.dirty_because(unit, "fresh build"),
}
}
}
52 changes: 21 additions & 31 deletions src/cargo/core/compiler/fingerprint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,42 +415,27 @@ pub fn prepare_target(cx: &mut Context<'_, '_>, unit: &Unit, force: bool) -> Car
// information about failed comparisons to aid in debugging.
let fingerprint = calculate(cx, unit)?;
let mtime_on_use = cx.bcx.config.cli_unstable().mtime_on_use;
let compare = compare_old_fingerprint(unit, &loc, &*fingerprint, mtime_on_use);
let dirty_reason = compare_old_fingerprint(unit, &loc, &*fingerprint, mtime_on_use, force);

// If our comparison failed or reported dirty (e.g., we're going to trigger
// a rebuild of this crate), then we also ensure the source of the crate
// passes all verification checks before we build it.
let Some(dirty_reason) = dirty_reason else {
return Ok(Job::new_fresh());
};

// We're going to rebuild, so ensure the source of the crate passes all
// verification checks before we build it.
//
// The `Source::verify` method is intended to allow sources to execute
// pre-build checks to ensure that the relevant source code is all
// up-to-date and as expected. This is currently used primarily for
// directory sources which will use this hook to perform an integrity check
// on all files in the source to ensure they haven't changed. If they have
// changed then an error is issued.
if compare
.as_ref()
.map(|dirty| dirty.is_some())
.unwrap_or(true)
{
let source_id = unit.pkg.package_id().source_id();
let sources = bcx.packages.sources();
let source = sources
.get(source_id)
.ok_or_else(|| internal("missing package source"))?;
source.verify(unit.pkg.package_id())?;
}

let dirty_reason = match compare {
Ok(None) => {
if force {
Some(DirtyReason::Forced)
} else {
return Ok(Job::new_fresh());
}
}
Ok(reason) => reason,
Err(_) => None,
};
let source_id = unit.pkg.package_id().source_id();
let sources = bcx.packages.sources();
let source = sources
.get(source_id)
.ok_or_else(|| internal("missing package source"))?;
source.verify(unit.pkg.package_id())?;

// Clear out the old fingerprint file if it exists. This protects when
// compilation is interrupted leaving a corrupt file. For example, a
Expand Down Expand Up @@ -521,7 +506,7 @@ pub fn prepare_target(cx: &mut Context<'_, '_>, unit: &Unit, force: bool) -> Car
Work::new(move |_| write_fingerprint(&loc, &fingerprint))
};

Ok(Job::new_dirty(write_fingerprint, dirty_reason))
Ok(Job::new_dirty(write_fingerprint, Some(dirty_reason)))
}

/// Dependency edge information for fingerprints. This is generated for each
Expand Down Expand Up @@ -1755,10 +1740,15 @@ fn compare_old_fingerprint(
old_hash_path: &Path,
new_fingerprint: &Fingerprint,
mtime_on_use: bool,
) -> CargoResult<Option<DirtyReason>> {
forced: bool,
) -> Option<DirtyReason> {
let compare = _compare_old_fingerprint(old_hash_path, new_fingerprint, mtime_on_use);
log_compare(unit, &compare);
compare
match compare {
Ok(None) if forced => Some(DirtyReason::Forced),
Ok(reason) => reason,
Err(_) => Some(DirtyReason::FreshBuild),
epage marked this conversation as resolved.
Show resolved Hide resolved
}
}

fn _compare_old_fingerprint(
Expand Down
8 changes: 5 additions & 3 deletions src/cargo/core/compiler/job_queue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1111,9 +1111,11 @@ impl<'cfg> DrainState<'cfg> {
// being a compiled package.
Dirty(dirty_reason) => {
if let Some(reason) = dirty_reason {
config
.shell()
.verbose(|shell| reason.present_to(shell, unit, ws_root))?;
if !reason.is_fresh_build() {
config
.shell()
.verbose(|shell| reason.present_to(shell, unit, ws_root))?;
}
}

if unit.mode.is_doc() {
Expand Down