features and fixes

.github/workflows/ci.yaml

@@ -1227,7 +1227,7 @@ jobs:
           echo '```console' > "$GITHUB_STEP_SUMMARY"
           # Enable color output for prek and remove it for the summary
           # Use --hook-stage=manual to enable slower hooks that are skipped by default
-          SKIP=rustfmt uv run --only-dev --locked prek run --all-files --show-diff-on-failure --color always --hook-stage manual | \
+          SKIP=rustfmt,cargo-doc uv run --only-dev --locked prek run --all-files --show-diff-on-failure --color always --hook-stage manual | \
             tee >(sed -E 's/\x1B\[([0-9]{1,2}(;[0-9]{1,2})*)?[mGK]//g' >> "$GITHUB_STEP_SUMMARY") >&1
           exit_code="${PIPESTATUS[0]}"
           echo '```' >> "$GITHUB_STEP_SUMMARY"

.pre-commit-config.yaml

@@ -79,14 +79,15 @@ repos:
       - id: cargo-doc
         name: check `cargo doc` and `cargo test --doc`
         entry: >
-          bash -c 'RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --document-private-items -p ty_python_semantic
-          -p ty_python_core -p ty_module_resolver -p ty_site_packages -p ty_combine -p ty_project -p ty_ide -p ty_wasm
-          -p ty_vendored -p ty_static -p ty -p ty_test -p ruff_db -p ruff_python_formatter
-          && cargo test --all-features --doc'
+          bash -c 'RUSTDOCFLAGS="-D warnings" cargo doc --all --no-deps
+            && RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --document-private-items -p ty_python_semantic
+              -p ty_python_core -p ty_module_resolver -p ty_site_packages -p ty_combine -p ty_project -p ty_ide
+              -p ty_wasm -p ty_vendored -p ty_static -p ty -p ty_test -p ruff_db -p ruff_python_formatter
+            && cargo test --all-features --doc'
         types: [rust]
         language: system
         pass_filenames: false
-        priority: 0
+        stages: [pre-push]
 
   # Prettier
   - repo: https://github.com/rbubley/mirrors-prettier

crates/by_build/src/lib.rs

@@ -22,8 +22,18 @@ pub(crate) mod annotate;
 pub struct Artifact {
     /// the generated C, kept for `--annotate` and for debugging
     pub source: PathBuf,
-    /// the loadable extension
-    pub extension: PathBuf,
+    /// the header every generated `.c` includes, at the root of the tree
+    pub header: PathBuf,
+    /// the loadable extension, or `None` when only the C was emitted
+    ///
+    /// a caller that records what the build wrote — so that a later one can take
+    /// a stale file back — has to be told the difference. naming the extension an
+    /// `--emit-c-only` run *would* have built is not harmless: the name is
+    /// `<module>.so`, which is not where any real build puts it (that is
+    /// `Toolchain::extension_path`, tagged with the interpreter's abi), so a
+    /// caller checking the file system for it finds nothing, records nothing, and
+    /// prunes the extension a previous real build left
+    pub extension: Option<PathBuf>,
     /// the `--annotate` report, when one was asked for
     pub annotation: Option<PathBuf>,
 }
@@ -113,7 +123,8 @@ fn emit_verified(module: &ModuleIr, out_dir: &Path, options: &Options) -> Result
     }
     fs::create_dir_all(out_dir)
         .with_context(|| format!("could not create {}", out_dir.display()))?;
-    fs::write(out_dir.join(by_rt::BY_H_NAME), by_rt::BY_H)?;
+    let header = out_dir.join(by_rt::BY_H_NAME);
+    fs::write(&header, by_rt::BY_H)?;
 
     let source_path = out_dir.join(module.name.relative_path(".c"));
     create_parent(&source_path)?;
@@ -123,7 +134,8 @@ fn emit_verified(module: &ModuleIr, out_dir: &Path, options: &Options) -> Result
     Ok(Built {
         artifact: Artifact {
             source: source_path,
-            extension: out_dir.join(module.name.relative_path(".so")),
+            header,
+            extension: None,
             annotation: write_annotation(module, out_dir, options)?,
         },
         declined: module.declined.clone(),
@@ -364,7 +376,8 @@ pub fn build_module(module: &ModuleIr, toolchain: &Toolchain, out_dir: &Path) ->
 
     Ok(Artifact {
         source,
-        extension,
+        header,
+        extension: Some(extension),
         annotation: None,
     })
 }

crates/by_build/tests/end_to_end.rs

@@ -81,7 +81,10 @@ fn built(module: &ModuleIr, toolchain: &Toolchain, tag: &str) -> Option<PathBuf>
     let _ = std::fs::remove_dir_all(&dir);
     match build_module(module, toolchain, &dir) {
         Ok(artifact) => {
-            assert!(artifact.extension.exists(), "the extension was written");
+            assert!(
+                extension_of(&artifact).exists(),
+                "the extension was written"
+            );
             Some(dir)
         }
         Err(error) => {
@@ -218,6 +221,18 @@ fn fib_module() -> ModuleIr {
     }
 }
 
+/// The extension a build wrote.
+///
+/// `Artifact::extension` is `None` only for an `--emit-c-only` emit, which
+/// nothing in this file does — every case here goes through `build_module` or
+/// `build_lowered`, both of which invoke the C compiler.
+fn extension_of(artifact: &by_build::Artifact) -> &std::path::Path {
+    artifact
+        .extension
+        .as_deref()
+        .expect("this build compiled an extension, so it has to name one")
+}
+
 #[test]
 fn integer_arithmetic_computes_the_same_answers_as_python() {
     let Some((python, toolchain)) = environment() else {
@@ -931,7 +946,7 @@ fn an_unchanged_module_is_not_recompiled() {
             .and_then(|meta| meta.modified())
             .expect("the artifact exists")
     };
-    let before = stamp(&first.artifact.extension);
+    let before = stamp(extension_of(&first.artifact));
 
     let second = build_source(
         source,
@@ -941,7 +956,11 @@ fn an_unchanged_module_is_not_recompiled() {
         &Options::default(),
     )
     .expect("the toolchain already worked");
-    assert_eq!(stamp(&second.artifact.extension), before, "it recompiled");
+    assert_eq!(
+        stamp(extension_of(&second.artifact)),
+        before,
+        "it recompiled"
+    );
 
     // and a real change does rebuild
     let changed = "def double(a: int) -> int:\n    return a * 3\n";
@@ -954,7 +973,7 @@ fn an_unchanged_module_is_not_recompiled() {
     )
     .expect("the toolchain already worked");
     assert_ne!(
-        stamp(&third.artifact.extension),
+        stamp(extension_of(&third.artifact)),
         before,
         "it skipped a change"
     );
@@ -980,7 +999,7 @@ fn a_stale_extension_is_rebuilt_even_when_the_c_is_unchanged() {
         eprintln!("skipping: no working C toolchain");
         return;
     };
-    std::fs::remove_file(&built.artifact.extension).expect("the artifact exists");
+    std::fs::remove_file(extension_of(&built.artifact)).expect("the artifact exists");
     let again = build_source(
         source,
         "by_e2e_rebuild_stale",
@@ -989,7 +1008,7 @@ fn a_stale_extension_is_rebuilt_even_when_the_c_is_unchanged() {
         &Options::default(),
     )
     .expect("the toolchain already worked");
-    assert!(again.artifact.extension.exists());
+    assert!(extension_of(&again.artifact).exists());
 }
 
 /// a class on a base from outside the module, holding storage of its own
@@ -1251,10 +1270,10 @@ fn a_package_is_built_as_a_tree_and_imports_under_its_dotted_names() {
             return;
         };
         assert!(
-            built.artifact.extension.exists(),
+            extension_of(&built.artifact).exists(),
             "{} was written to {}",
             name.dotted(),
-            built.artifact.extension.display()
+            extension_of(&built.artifact).display()
         );
     }
 

crates/by_stage/src/project.rs

@@ -14,11 +14,13 @@
 //! build would have transpiled it, and the only way to guarantee that is for there
 //! to be one implementation of each.
 
+use std::collections::HashMap;
 use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 
 use anyhow::Context;
-use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf};
+use ruff_db::Db as _;
+use ruff_db::system::{OsSystem, System, SystemPath, SystemPathBuf};
 use ty_project::{Db, ProjectDatabase, ProjectMetadata};
 
 /// Everything needed to build a project db a second time.
@@ -308,7 +310,8 @@ pub(crate) fn project_sources(
     root: &Path,
     output: Option<&Path>,
 ) -> Vec<(PathBuf, ruff_db::files::File)> {
-    db.project()
+    let candidates: Vec<(PathBuf, ruff_db::files::File)> = db
+        .project()
         .files(db)
         .into_iter()
         .filter(|file| {
@@ -328,9 +331,75 @@ pub(crate) fn project_sources(
         // this build is about to read, and reading those instead would build the
         // project into itself, one directory deeper each time
         .filter(|(path, _)| output.is_none_or(|output| !path.starts_with(output)))
+        .collect();
+    // and nor is any *other* build's output. `--out` can name any directory, and
+    // a project can have several, so the one this run was given is not the only
+    // one that has to be turned away
+    let mut known = HashMap::new();
+    candidates
+        .into_iter()
+        .filter(|(path, _)| !inside_build_output(db.system(), path, root, &mut known))
         .collect()
 }
 
+/// Whether `path` sits inside a tree that some build already wrote.
+///
+/// A build output is self-describing: it holds a `.by-manifest` naming everything
+/// that build put there. That marker is what identifies it, rather than the
+/// directory's name. [`NON_SOURCE_DIRS`] already turns away the two default names,
+/// but `--out` can say anything, and only the tree this run was *given* is known
+/// to be an output from the arguments — a project that builds into two of them has
+/// to have the other recognised on sight.
+///
+/// It has to be turned away because the tree holds a *copy* of the project: every
+/// hand-written `.py`, and every `.by` too when `build.sources` is on. Reading one
+/// back means each of those modules is claimed twice — once where it was written
+/// and once where it was copied — and the build stops, telling the author that two
+/// files they never put side by side would compile to the same module.
+///
+/// The project root is never itself an output, whatever it holds: a `.by-manifest`
+/// left at the root would otherwise empty the project of every file it has.
+fn inside_build_output(
+    system: &dyn System,
+    path: &Path,
+    root: &Path,
+    known: &mut HashMap<PathBuf, bool>,
+) -> bool {
+    let Some(parent) = path.parent() else {
+        return false;
+    };
+    let mut current = parent;
+    while current != root && current.starts_with(root) {
+        let is_output = match known.get(current) {
+            Some(answer) => *answer,
+            None => {
+                let answer = holds_manifest(system, current);
+                known.insert(current.to_path_buf(), answer);
+                answer
+            }
+        };
+        if is_output {
+            return true;
+        }
+        match current.parent() {
+            Some(next) => current = next,
+            None => return false,
+        }
+    }
+    false
+}
+
+/// Whether `directory` carries the manifest that marks it a build output.
+///
+/// Asked of the db's [`System`] rather than of `std::fs`, so that it is the same
+/// file system every other question in a build is asked — the language server's
+/// re-stage runs this on a warm db, and an in-memory test system has to be able
+/// to answer it.
+pub(crate) fn holds_manifest(system: &dyn System, directory: &Path) -> bool {
+    SystemPath::from_std_path(directory)
+        .is_some_and(|path| system.is_file(&path.join(crate::staging::MANIFEST_FILENAME)))
+}
+
 /// The project's first-party module roots, longest first, as absolute paths.
 ///
 /// These are the directories a module name is resolved against — for a
@@ -351,9 +420,60 @@ pub fn module_roots(db: &ProjectDatabase, cwd: &Path) -> Vec<PathBuf> {
 
 #[cfg(test)]
 mod tests {
-    use super::is_hidden_within;
+    use super::{inside_build_output, is_hidden_within};
+    use ruff_db::system::{OsSystem, SystemPath};
+    use std::collections::HashMap;
     use std::path::Path;
 
+    /// `--out` can name any directory and a project can hold several, so a build
+    /// output is recognised by the manifest it carries rather than by its name.
+    #[test]
+    fn a_directory_holding_a_manifest_is_a_build_output() {
+        let directory = tempfile::tempdir().expect("tempdir");
+        let root = directory.path();
+        let output = root.join("anything");
+        std::fs::create_dir_all(output.join("pkg")).expect("create");
+        std::fs::write(output.join(".by-manifest"), "").expect("write");
+        let plain = root.join("src");
+        std::fs::create_dir_all(&plain).expect("create");
+
+        let system =
+            OsSystem::new(SystemPath::from_std_path(root).expect("the temp directory is utf-8"));
+        let mut known = HashMap::new();
+        assert!(inside_build_output(
+            &system,
+            &output.join("a.by"),
+            root,
+            &mut known
+        ));
+        assert!(
+            inside_build_output(&system, &output.join("pkg").join("a.by"), root, &mut known),
+            "a file deeper inside the output is inside it too"
+        );
+        assert!(
+            !inside_build_output(&system, &plain.join("a.by"), root, &mut known),
+            "a directory with no manifest is ordinary source"
+        );
+    }
+
+    /// A manifest at the root would otherwise empty the project of every file.
+    #[test]
+    fn the_project_root_is_never_itself_an_output() {
+        let directory = tempfile::tempdir().expect("tempdir");
+        let root = directory.path();
+        std::fs::write(root.join(".by-manifest"), "").expect("write");
+
+        let system =
+            OsSystem::new(SystemPath::from_std_path(root).expect("the temp directory is utf-8"));
+        let mut known = HashMap::new();
+        assert!(!inside_build_output(
+            &system,
+            &root.join("a.by"),
+            root,
+            &mut known
+        ));
+    }
+
     #[test]
     fn a_hidden_directory_is_not_project_source() {
         let root = Path::new("/p");