Skip to content

build: allow the cleanup step to be non-fatal #505

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

Merged
merged 1 commit into from
Aug 11, 2025

Conversation

davvid
Copy link
Contributor

@davvid davvid commented Jul 10, 2025

3e8962c (Clean up dep-info files from OUT_DIR, 2024-10-22) made the cleanup step a fatal operation when cleanup fails. Some filesystems (e.g. NFS v3) can run into timing issues under load where .nfs* dummy files can still exist for a short window of time after the probe command completes. This window is just large enough for a failure to occur in remove_all().

Add an environment variable that allows users to make the cleanup operation non-fatal.

@davvid davvid force-pushed the ignore-cleanup branch 2 times, most recently from a07d529 to 82aa3b7 Compare July 10, 2025 19:59
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Could you find out the specific std::io::ErrorKind or std::io::Error::raw_os_error that is being produced by the filesystem in this scenario? If it is something reasonably unique to this failure mode, I would prefer to ignore just that kind of error (without an environment variable).

3e8962c (Clean up dep-info files from OUT_DIR, 2024-10-22)
made the cleanup step a fatal operation when cleanup fails.

Some filesystems (e.g. NFS v3) can run into timing issues under load
where .nfs* dummy files can still exist for a short window of time after
the probe command completes. One way this can happen is when other
processes, such as a malware scanner, holds the files open in the
background for a short period of time. This window of time is just long
enough for a failure to occur in remove_all().

Ignore DirectoryNotEmpty errors to account for this scenario.
@davvid
Copy link
Contributor Author

davvid commented Aug 11, 2025

I've updated this branch to specifically check for ErrorKind::DirectoryNotEmpty now. Thanks for reviewing.

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks!

@dtolnay dtolnay merged commit ad59158 into dtolnay:master Aug 11, 2025
12 of 14 checks passed
@@ -203,7 +203,7 @@ fn compile_probe(rustc_bootstrap: bool) -> bool {
// file in OUT_DIR, which causes nonreproducible builds in build systems
// that treat the entire OUT_DIR as an artifact.
if let Err(err) = fs::remove_dir_all(&out_subdir) {
if err.kind() != ErrorKind::NotFound {
if err.kind() != ErrorKind::NotFound && err.kind() != ErrorKind::DirectoryNotEmpty {
Copy link
Owner

Choose a reason for hiding this comment

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

#512 adds an explanation comment and updates this to work with standard libraries that do not contain DirectoryNotEmpty.

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.

2 participants