Work work

.pre-commit-config.yaml

@@ -87,7 +87,7 @@ repos:
         types: [rust]
         language: system
         pass_filenames: false
-        stages: [pre-push]
+        stages: [manual]
 
   # Prettier
   - repo: https://github.com/rbubley/mirrors-prettier

crates/by_stage/src/project.rs

@@ -5,22 +5,22 @@
 //! project reports errors for files the project deliberately excludes — and, when
 //! this was first written, wrote nothing for them either. So the file set is the
 //! project's own (`Project::files`), narrowed only by things a *build* has to
-//! exclude and a check does not: a hidden directory holding copies, and the last
-//! build's own output.
+//! exclude and a check does not: a hidden directory holding copies, and the
+//! directory this build writes to. Every tree a build already wrote carries a
+//! manifest, and the project leaves those out itself — see
+//! [`ty_project::build_output`].
 //!
 //! It lives here rather than in the `by` binary because the language server needs
 //! the same answers. A single file re-staged into a build tree has to land where
 //! the whole-tree build would have put it and be transpiled the way the whole-tree
 //! 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::Db as _;
-use ruff_db::system::{OsSystem, System, SystemPath, SystemPathBuf};
+use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf};
 use ty_project::{Db, ProjectDatabase, ProjectMetadata};
 
 /// Everything needed to build a project db a second time.
@@ -310,8 +310,7 @@ pub(crate) fn project_sources(
     root: &Path,
     output: Option<&Path>,
 ) -> Vec<(PathBuf, ruff_db::files::File)> {
-    let candidates: Vec<(PathBuf, ruff_db::files::File)> = db
-        .project()
+    db.project()
         .files(db)
         .into_iter()
         .filter(|file| {
@@ -327,79 +326,16 @@ pub(crate) fn project_sources(
         // dependencies, not this project's sources — emitting them would write
         // a parallel tree nobody asked for
         .filter(|(path, _)| !is_hidden_within(path, root))
-        // nor is the last build's output. it holds a copy of every `.by` source
-        // this build is about to read, and reading those instead would build the
-        // project into itself, one directory deeper each time
+        // nor is the directory this build writes to. it holds a copy of every
+        // `.by` source this build is about to read, and reading those instead
+        // would build the project into itself, one directory deeper each time.
+        // a tree a build finished carries a manifest and is never among the
+        // project's files to begin with, whatever `--out` called it; this is for
+        // the one this run was told to write to, which may not have one yet
         .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
@@ -420,60 +356,9 @@ pub fn module_roots(db: &ProjectDatabase, cwd: &Path) -> Vec<PathBuf> {
 
 #[cfg(test)]
 mod tests {
-    use super::{inside_build_output, is_hidden_within};
-    use ruff_db::system::{OsSystem, SystemPath};
-    use std::collections::HashMap;
+    use super::is_hidden_within;
     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");

crates/by_stage/src/staging.rs

@@ -8,6 +8,7 @@
 
 use std::collections::{BTreeMap, BTreeSet};
 use std::fs;
+use std::io::Write as _;
 use std::path::{Path, PathBuf};
 
 use anyhow::Context;
@@ -20,7 +21,13 @@ use anyhow::Context;
 /// until one shadows a module that moved. The manifest is what makes the output a
 /// mirror rather than a pile: what the previous build wrote and this one did not
 /// is deleted.
-pub(crate) const MANIFEST_FILENAME: &str = ".by-manifest";
+///
+/// Being there is also what marks the tree as a build output: the project the build
+/// was made from leaves it out of its own files — see [`ty_project::build_output`].
+const MANIFEST_FILENAME: &str = ty_project::build_output::BUILD_MANIFEST;
+
+/// the line a manifest starts with, before the paths it records
+const MANIFEST_HEADER: &str = "# written by `by build`; delete it and stale output stays\n";
 
 /// An output tree being written.
 pub struct Staging {
@@ -30,6 +37,8 @@ pub struct Staging {
     /// destinations another writer produced, held apart from `written` so a
     /// source that would land on one is reported rather than overwriting it
     recorded: BTreeSet<PathBuf>,
+    /// whether the tree is known to carry a manifest already — see [`Self::mark_as_output`]
+    marked: bool,
 }
 
 impl Staging {
@@ -38,9 +47,46 @@ impl Staging {
             out: out.to_path_buf(),
             written: BTreeMap::new(),
             recorded: BTreeSet::new(),
+            marked: false,
         }
     }
 
+    /// mark the tree as a build output before the first file lands in it
+    ///
+    /// the manifest [`Self::finish`] writes is also what tells the project this tree is not
+    /// part of it. a first build writes into a directory that has none yet, and a language
+    /// server watching the project would otherwise see a copy of every source appear, take them
+    /// for the project's own, and check each one until the build finished and the manifest
+    /// arrived. an empty manifest says the same as a missing one to the next build (nothing
+    /// was written here before), and one already there is left for `finish` to read
+    ///
+    /// writing and copying through the staging mark it themselves, and so does [`Self::record`].
+    /// a writer that lays files out itself, as `by compile` does, calls this before it writes
+    pub fn mark_as_output(&mut self) -> anyhow::Result<()> {
+        if self.marked {
+            return Ok(());
+        }
+        let manifest = self.out.join(MANIFEST_FILENAME);
+        create_parent(&manifest)?;
+        match fs::OpenOptions::new()
+            .write(true)
+            .create_new(true)
+            .open(&manifest)
+        {
+            Ok(mut file) => {
+                file.write_all(MANIFEST_HEADER.as_bytes())
+                    .with_context(|| format!("could not write {}", manifest.display()))?;
+            }
+            Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => {}
+            Err(error) => {
+                return Err(error)
+                    .with_context(|| format!("could not write {}", manifest.display()));
+            }
+        }
+        self.marked = true;
+        Ok(())
+    }
+
     pub(crate) fn out(&self) -> &Path {
         &self.out
     }
@@ -76,6 +122,7 @@ impl Staging {
         contents: &str,
     ) -> anyhow::Result<()> {
         self.claim(relative, source)?;
+        self.mark_as_output()?;
         let destination = self.out.join(relative);
         create_parent(&destination)?;
         fs::write(&destination, contents)
@@ -104,13 +151,15 @@ impl Staging {
                 claimant(source.as_deref()),
             );
         }
+        self.mark_as_output()?;
         self.recorded.insert(relative.to_path_buf());
         Ok(())
     }
 
     /// Copy `source` to `relative` verbatim.
     pub(crate) fn copy(&mut self, relative: &Path, source: &Path) -> anyhow::Result<()> {
         self.claim(relative, Some(source))?;
+        self.mark_as_output()?;
         let destination = self.out.join(relative);
         create_parent(&destination)?;
         fs::copy(source, &destination).with_context(|| {
@@ -190,8 +239,7 @@ impl Staging {
         }
 
         create_parent(&manifest)?;
-        let mut rendered =
-            String::from("# written by `by build`; delete it and stale output stays\n");
+        let mut rendered = String::from(MANIFEST_HEADER);
         for path in current {
             rendered.push_str(&portable(path));
             rendered.push('\n');
@@ -426,6 +474,69 @@ mod tests {
         assert!(!directory.path().join("mine.py").exists());
     }
 
+    /// the manifest is what keeps the tree out of the project it was built from, so it is
+    /// there before the first file is, not only once the build is done
+    #[test]
+    fn a_first_build_marks_its_output_before_writing_into_it() {
+        let directory = tempfile::tempdir().expect("tempdir");
+        let out = directory.path().join("build");
+        let mut staging = Staging::new(&out);
+        staging
+            .write(Path::new("a.py"), None, "x = 1\n")
+            .expect("write");
+        assert!(
+            ty_project::build_output::is_build_output(
+                &ruff_db::system::OsSystem::new(
+                    ruff_db::system::SystemPath::from_std_path(directory.path()).expect("utf-8")
+                ),
+                ruff_db::system::SystemPath::from_std_path(&out).expect("utf-8"),
+            ),
+            "a tree being written is an output before the build finishes"
+        );
+        staging.finish().expect("finish");
+        assert!(read_manifest(&out.join(MANIFEST_FILENAME)).contains(Path::new("a.py")));
+    }
+
+    /// a file another writer put into the tree marks it too
+    #[test]
+    fn recording_an_artifact_marks_the_output() {
+        let directory = tempfile::tempdir().expect("tempdir");
+        let out = directory.path().join("build");
+        let mut staging = Staging::new(&out);
+        staging.record(Path::new("a.so")).expect("record");
+        assert!(
+            ty_project::build_output::is_build_output(
+                &ruff_db::system::OsSystem::new(
+                    ruff_db::system::SystemPath::from_std_path(directory.path()).expect("utf-8")
+                ),
+                ruff_db::system::SystemPath::from_std_path(&out).expect("utf-8"),
+            ),
+            "a tree with an artifact recorded in it is an output before the build finishes"
+        );
+    }
+
+    /// marking a tree the last build left does not forget what that build wrote
+    #[test]
+    fn marking_an_output_keeps_the_previous_manifest() {
+        let directory = tempfile::tempdir().expect("tempdir");
+        let mut first = Staging::new(directory.path());
+        first
+            .write(Path::new("gone.py"), None, "x = 1\n")
+            .expect("write");
+        first.finish().expect("finish");
+
+        let mut second = Staging::new(directory.path());
+        second
+            .write(Path::new("kept.py"), None, "x = 1\n")
+            .expect("write");
+        assert!(
+            read_manifest(&directory.path().join(MANIFEST_FILENAME)).contains(Path::new("gone.py")),
+            "the previous build's record survives until this one replaces it"
+        );
+        second.finish().expect("finish");
+        assert!(!directory.path().join("gone.py").exists());
+    }
+
     #[test]
     fn a_manifest_round_trips_through_its_portable_form() {
         let directory = tempfile::tempdir().expect("tempdir");