-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
Change "error finalizing incremental compilation" text and emit it as a note, not a warning #154110
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
+201
−2
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
384f363
change "error finalizing incremental compilation" from warning to note
lambdageek 4845f78
Reword the incremental finalize diagnostic
lambdageek 0cc4946
add a test to make rustc_incremental finalize_session_directory renam…
lambdageek 0c05f6c
Add ignore-cross-compile to incremental-finalize-fail
lambdageek File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| poison::poison_finalize!(); | ||
|
|
||
| pub fn hello() -> i32 { | ||
| 42 | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| //! A proc macro that sabotages the incremental compilation finalize step. | ||
| //! | ||
| //! When invoked, it locates the `-working` session directory inside the | ||
| //! incremental compilation directory (passed via POISON_INCR_DIR) and | ||
| //! makes it impossible to rename: | ||
| //! | ||
| //! - On Unix: removes write permission from the parent (crate) directory. | ||
| //! - On Windows: creates a file inside the -working directory and leaks | ||
| //! the file handle, preventing the directory from being renamed. | ||
|
|
||
| extern crate proc_macro; | ||
|
|
||
| use std::fs; | ||
| use std::path::PathBuf; | ||
|
|
||
| use proc_macro::TokenStream; | ||
|
|
||
| #[proc_macro] | ||
| pub fn poison_finalize(_input: TokenStream) -> TokenStream { | ||
| let incr_dir = std::env::var("POISON_INCR_DIR").expect("POISON_INCR_DIR must be set"); | ||
|
|
||
| let crate_dir = find_crate_dir(&incr_dir); | ||
| let working_dir = find_working_dir(&crate_dir); | ||
|
|
||
| #[cfg(unix)] | ||
| poison_unix(&crate_dir); | ||
|
|
||
| #[cfg(windows)] | ||
| poison_windows(&working_dir); | ||
|
|
||
| TokenStream::new() | ||
| } | ||
|
|
||
| /// Remove write permission from the crate directory. | ||
| /// This causes rename() to fail with EACCES | ||
| #[cfg(unix)] | ||
| fn poison_unix(crate_dir: &PathBuf) { | ||
| use std::os::unix::fs::PermissionsExt; | ||
| let mut perms = fs::metadata(crate_dir).unwrap().permissions(); | ||
| perms.set_mode(0o555); // r-xr-xr-x | ||
| fs::set_permissions(crate_dir, perms).unwrap(); | ||
| } | ||
|
|
||
| /// Create a file inside the -working directory and leak the | ||
| /// handle. Windows prevents renaming a directory when any file inside it | ||
| /// has an open handle. The handle stays open until the rustc process exits. | ||
| #[cfg(windows)] | ||
| fn poison_windows(working_dir: &PathBuf) { | ||
| let poison_file = working_dir.join("_poison_handle"); | ||
| let f = fs::File::create(&poison_file).unwrap(); | ||
| // Leak the handle so it stays open for the lifetime of the rustc process. | ||
| std::mem::forget(f); | ||
| } | ||
|
|
||
| /// Find the crate directory for `foo` inside the incremental compilation dir. | ||
| /// | ||
| /// The incremental directory layout is: | ||
| /// {incr_dir}/{crate_name}-{stable_crate_id}/ | ||
| fn find_crate_dir(incr_dir: &str) -> PathBuf { | ||
| let mut dirs = fs::read_dir(incr_dir).unwrap().filter_map(|e| { | ||
| let e = e.ok()?; | ||
| let name = e.file_name(); | ||
| let name = name.to_str()?; | ||
| if e.file_type().ok()?.is_dir() && name.starts_with("foo-") { Some(e.path()) } else { None } | ||
| }); | ||
|
|
||
| let first = | ||
| dirs.next().unwrap_or_else(|| panic!("no foo-* crate directory found in {incr_dir}")); | ||
| assert!( | ||
| dirs.next().is_none(), | ||
| "expected exactly one foo-* crate directory in {incr_dir}, found multiple" | ||
| ); | ||
| first | ||
| } | ||
|
|
||
| /// Find the session directory ending in "-working" inside the crate directory | ||
| fn find_working_dir(crate_dir: &PathBuf) -> PathBuf { | ||
| for entry in fs::read_dir(crate_dir).unwrap() { | ||
| let entry = entry.unwrap(); | ||
| let name = entry.file_name(); | ||
| let name = name.to_str().unwrap().to_string(); | ||
| if name.starts_with("s-") && name.ends_with("-working") { | ||
| return entry.path(); | ||
| } | ||
| } | ||
| panic!("no -working session directory found in {}", crate_dir.display()); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| //@ ignore-cross-compile | ||
| //@ needs-crate-type: proc-macro | ||
|
|
||
| //! Test that a failure to finalize the incremental compilation session directory | ||
| //! (i.e., the rename from "-working" to the SVH-based name) results in a | ||
| //! note, not an ICE, and that the compilation output is still produced. | ||
| //! | ||
| //! Strategy: | ||
| //! 1. Build the `poison` proc-macro crate | ||
| //! 2. Compile foo.rs with incremental compilation | ||
| //! The proc macro runs mid-compilation (after prepare_session_directory | ||
| //! but before finalize_session_directory) and sabotages the rename: | ||
| //! - On Unix: removes write permission from the crate directory, | ||
| //! so rename() fails with EACCES. | ||
| //! - On Windows: creates and leaks an open file handle inside the | ||
| //! -working directory, so rename() fails with ERROR_ACCESS_DENIED. | ||
| //! 3. Assert that stderr contains the finalize failure messages | ||
|
|
||
| use std::fs; | ||
| use std::path::{Path, PathBuf}; | ||
|
|
||
| use run_make_support::rustc; | ||
|
|
||
| /// Guard that restores permissions on the incremental directory on drop, | ||
| /// to ensure cleanup is possible | ||
| struct IncrDirCleanup; | ||
|
|
||
| fn main() { | ||
| let _cleanup = IncrDirCleanup; | ||
|
|
||
| // Build the poison proc-macro crate | ||
| rustc().input("poison/lib.rs").crate_name("poison").crate_type("proc-macro").run(); | ||
|
|
||
| let poison_dylib = find_proc_macro_dylib("poison"); | ||
|
|
||
| // Incremental compile with the poison macro active | ||
| let out = rustc() | ||
| .input("foo.rs") | ||
| .crate_type("rlib") | ||
| .incremental("incr") | ||
| .extern_("poison", &poison_dylib) | ||
| .env("POISON_INCR_DIR", "incr") | ||
| .run(); | ||
|
|
||
| out.assert_stderr_contains("note: did not finalize incremental compilation session directory"); | ||
| out.assert_stderr_contains( | ||
| "help: the next build will not be able to reuse work from this compilation", | ||
| ); | ||
| out.assert_stderr_not_contains("internal compiler error"); | ||
| } | ||
|
|
||
| impl Drop for IncrDirCleanup { | ||
| fn drop(&mut self) { | ||
| let incr = Path::new("incr"); | ||
| if !incr.exists() { | ||
| return; | ||
| } | ||
|
|
||
| #[cfg(unix)] | ||
| restore_permissions(incr); | ||
| } | ||
| } | ||
|
|
||
| /// Recursively restore write permissions so rm -rf works after the chmod trick | ||
| #[cfg(unix)] | ||
| fn restore_permissions(path: &Path) { | ||
| use std::os::unix::fs::PermissionsExt; | ||
| if let Ok(entries) = fs::read_dir(path) { | ||
| for entry in entries.filter_map(|e| e.ok()) { | ||
| if entry.file_type().map_or(false, |ft| ft.is_dir()) { | ||
| let mut perms = match fs::metadata(entry.path()) { | ||
| Ok(m) => m.permissions(), | ||
| Err(_) => continue, | ||
| }; | ||
| perms.set_mode(0o755); | ||
| let _ = fs::set_permissions(entry.path(), perms); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Locate the compiled proc-macro dylib by scanning the current directory. | ||
| fn find_proc_macro_dylib(name: &str) -> PathBuf { | ||
| let prefix = if cfg!(target_os = "windows") { "" } else { "lib" }; | ||
|
|
||
| let ext: &str = if cfg!(target_os = "macos") { | ||
| "dylib" | ||
| } else if cfg!(target_os = "windows") { | ||
| "dll" | ||
| } else { | ||
| "so" | ||
| }; | ||
|
|
||
| let lib_name = format!("{prefix}{name}.{ext}"); | ||
|
|
||
| for entry in fs::read_dir(".").unwrap() { | ||
| let entry = entry.unwrap(); | ||
| let name = entry.file_name(); | ||
| let name = name.to_str().unwrap(); | ||
| if name == lib_name { | ||
| return entry.path(); | ||
| } | ||
| } | ||
|
|
||
| panic!("could not find proc-macro dylib for `{name}`"); | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 like that this focuses the issue directly on the impact the user might care about 👍