Rollup merge of #154110 - lambdageek:fix/incr-compile-note, r=wesleywiser

Change "error finalizing incremental compilation" text and emit it as a note, not a warning

As mentioned in #151181 (comment) and #151181 (comment) the current message could be improved:

1. Right now it displays as "warning: error ..." which is confusing (is it an error or a warning)
2. It doesn't give the user a clear indication of what the consequences are
3. The _current_ build is successful. The _next_ build might be slower

The new message is now

```text
note: did not finalize incremental compilation session directory ...
  |
  = help: the next build will not be able to reuse work from this compilation
```

I started a zulip thread [#t-compiler/incremental > Ergonomics of "error finalizing incremental session"](https://rust-lang.zulipchat.com/#narrow/channel/241847-t-compiler.2Fincremental/topic/Ergonomics.20of.20.22error.20finalizing.20incremental.20session.22/with/580191447)

Author: Zalathar

compiler/rustc_incremental/src/errors.rs

@@ -192,7 +192,8 @@ pub(crate) struct DeleteFull<'a> {
 }
 
 #[derive(Diagnostic)]
-#[diag("error finalizing incremental compilation session directory `{$path}`: {$err}")]
+#[diag("did not finalize incremental compilation session directory `{$path}`: {$err}")]
+#[help("the next build will not be able to reuse work from this compilation")]
 pub(crate) struct Finalize<'a> {
     pub path: &'a Path,
     pub err: std::io::Error,

compiler/rustc_incremental/src/persist/fs.rs

@@ -366,7 +366,7 @@ pub fn finalize_session_directory(sess: &Session, svh: Option<Svh>) {
         }
         Err(e) => {
             // Warn about the error. However, no need to abort compilation now.
-            sess.dcx().emit_warn(errors::Finalize { path: &incr_comp_session_dir, err: e });
+            sess.dcx().emit_note(errors::Finalize { path: &incr_comp_session_dir, err: e });
 
             debug!("finalize_session_directory() - error, marking as invalid");
             // Drop the file lock, so we can garage collect

tests/run-make/incremental-finalize-fail/foo.rs

@@ -0,0 +1,5 @@
+poison::poison_finalize!();
+
+pub fn hello() -> i32 {
+    42
+}

tests/run-make/incremental-finalize-fail/poison/lib.rs

@@ -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());
+}

tests/run-make/incremental-finalize-fail/rmake.rs

@@ -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}`");
+}