Parcourir la source

Only re-archive the build artifact when the build tree changed

archive_build re-archived unconditionally whenever a build directory sat
next to build.tar.xz - the common incremental case (archive restored, build
runs, cleanup) - rewriting byte-identical archives and churning mtimes. That
also broke archive byte-identity for workflows where an artifact was
downloaded as build.tar.xz and the package rebuilt nothing (the DCBA case).

A build-tree digest now records the tree state that produced an archive in a
build.tar.xz.digest sidecar next to it:

- Util.tree_digest walks a tree deterministically (byte-sorted relative
  paths, file type, mode, symlink targets, full file bytes) into a base64
  SHA-512; unreadable entries throw instead of being skipped.
- archive_build compares the current tree digest with the sidecar: a match
  keeps the existing archive byte-identical (mtime untouched), drives the
  progress delegate to completion and only cleans the build directory. A
  mismatch or a missing (legacy) sidecar re-archives as before and writes a
  fresh sidecar.
- get_build_archive writes the sidecar when it archives a bare build
  directory; unarchive/get_build_directory leave it alone.
- clean_build_cache and 'usm clean builds' delete the sidecar together with
  the archive.

Tests: unchanged tree keeps archive bytes and mtime; modified, added,
removed, mode-changed and retargeted-symlink trees re-archive; missing
sidecar re-archives; digest determinism and path sensitivity; sidecar
lifecycle across get_build_archive/unarchive/clean_build_cache.
saddle-Clanker-Saddle il y a 1 mois
Parent
commit
50b92000c5
4 fichiers modifiés avec 414 ajouts et 6 suppressions
  1. 6 3
      src/cli/Clean.vala
  2. 61 2
      src/lib/State/CachedPackage.vala
  3. 113 1
      src/lib/Util.vala
  4. 234 0
      src/tests/TestMain.vala

+ 6 - 3
src/cli/Clean.vala

@@ -117,15 +117,18 @@ private uint64 clean_lists(Usm.SystemState state) throws Error {
 }
 
 /**
- * Deletes build directories and archives. Without {@link name} every
- * package's; with it, only {@link name}'s version directories (other
- * packages sharing the name prefix are never touched).
+ * Deletes build directories and archives together with the
+ * build.tar.xz.digest sidecars describing the trees that produced them.
+ * Without {@link name} every package's; with it, only {@link name}'s
+ * version directories (other packages sharing the name prefix are never
+ * touched).
  */
 private uint64 clean_builds(Usm.SystemState state, string? name) throws Error {
     uint64 freed = 0;
     foreach(var package_dir in clean_package_directories(state, name)) {
         freed += clean_delete(Path.build_filename(package_dir, "build"));
         freed += clean_delete(Path.build_filename(package_dir, "build.tar.xz"));
+        freed += clean_delete(Path.build_filename(package_dir, "build.tar.xz.digest"));
     }
     return freed;
 }

+ 61 - 2
src/lib/State/CachedPackage.vala

@@ -83,9 +83,21 @@ namespace Usm {
             }
 
             Util.archive(dir_path, path);
+            write_build_digest(Util.tree_digest(dir_path));
             return path;
         }
 
+        /**
+         * Archives the build directory as the package's build.tar.xz,
+         * re-archiving only when the build tree actually changed since
+         * the archive was created. The tree state that produced an
+         * archive is recorded next to it in build.tar.xz.digest (see
+         * {@link write_build_digest}); a matching digest means the
+         * existing archive is already authoritative, so it is kept
+         * byte-identical (mtime untouched) and only the build directory
+         * is cleaned. A missing sidecar (an archive from before digests
+         * were recorded) or a different tree re-archives as before.
+         */
         public void archive_build(Usm.ProgressDelegate? progress = null) throws Error {
             // An archive with no build directory is already the
             // authoritative artifact: extracting it just to repack the
@@ -95,14 +107,27 @@ namespace Usm {
             }
 
             var build_dir = get_build_directory();
+            var archive_path = build_archive_path();
+            var archive_file = File.new_for_path(archive_path);
 
-            // Delete existing archive, if it exists
-            var archive_file = File.new_for_path(build_archive_path());
             if(archive_file.query_exists()) {
+                var digest = Util.tree_digest(build_dir);
+                if(read_build_digest() == digest) {
+                    // Nothing changed since this archive was created:
+                    // re-tarring would rewrite identical bytes. The
+                    // progress delegate still completes so callers'
+                    // progress bars finish.
+                    if(progress != null) {
+                        progress(1.0f);
+                    }
+                    clean_build_directory();
+                    return;
+                }
                 archive_file.delete();
             }
 
             Util.archive_with_progress(build_dir, archive_file.get_path(), progress);
+            write_build_digest(Util.tree_digest(build_dir));
             clean_build_directory();
         }
 
@@ -129,6 +154,10 @@ namespace Usm {
             if(archive_file.query_exists()) {
                 archive_file.delete();
             }
+            var digest = File.new_for_path(build_digest_path());
+            if(digest.query_exists()) {
+                digest.delete();
+            }
         }
 
         /**
@@ -183,6 +212,36 @@ namespace Usm {
             return Path.build_filename(state_path, "build.tar.xz");
         }
 
+        private string build_digest_path() {
+            return Path.build_filename(state_path, "build.tar.xz.digest");
+        }
+
+        /**
+         * Records {@link digest} — the build-tree digest of the tree that
+         * produced the current archive — in the build.tar.xz.digest
+         * sidecar next to it. Writing it is part of every archive
+         * creation (see {@link archive_build}).
+         */
+        private void write_build_digest(string digest) throws Error {
+            FileUtils.set_contents(build_digest_path(), digest + "\n");
+        }
+
+        /**
+         * The digest recorded for the current archive, or null when no
+         * sidecar exists (an archive from before digests were recorded,
+         * or a cache with no archive at all).
+         */
+        private string? read_build_digest() {
+            string contents = null;
+            try {
+                FileUtils.get_contents(build_digest_path(), out contents);
+            }
+            catch(Error e) {
+                return null;
+            }
+            return contents != null ? contents.strip() : null;
+        }
+
         private string source_directory_path() {
             return Path.build_filename(state_path, "package");
         }

+ 113 - 1
src/lib/Util.vala

@@ -140,4 +140,116 @@ namespace Usm.Util {
         proc.wait_check();
     }
 
-}
+    /**
+     * A deterministic SHA-512 digest of the directory tree rooted at
+     * `root`, base64-encoded: a canonical description of every entry —
+     * sorted relative paths (byte order), the entry's file type, its
+     * mode where significant (permissions of files and directories, the
+     * target string of a symbolic link) and the full bytes of every
+     * regular file — folded into one checksum. The relative path is part
+     * of each record, so boundaries are unambiguous and a file's
+     * location always contributes to the result.
+     *
+     * The digest is only ever compared with other digests of the same
+     * tree: it identifies *change*, not content. An entry that cannot be
+     * read throws rather than being silently skipped, because a skipped
+     * entry would make an out-of-date archive look up to date.
+     */
+    public static string tree_digest(string root) throws Error {
+        var checksum = new Checksum(ChecksumType.SHA512);
+        digest_tree(root, "", checksum);
+        var buffer = new uint8[ChecksumType.SHA512.get_length()];
+        size_t buffer_length = buffer.length;
+        checksum.get_digest(buffer, ref buffer_length);
+        return Base64.encode(buffer);
+    }
+
+    /**
+     * Feeds {@link checksum} the canonical record of every entry below
+     * `root` at {@link relative}, depth-first and in byte-sorted path
+     * order. Depth-first keeps sibling records contiguous; sorting the
+     * children of each directory yields a deterministic total order.
+     */
+    private static void digest_tree(string root, string relative, Checksum checksum) throws Error {
+        var names = new Vector<string>();
+        var enumerator = File.new_build_filename(root, relative).enumerate_children(
+            FileAttribute.STANDARD_NAME, FileQueryInfoFlags.NOFOLLOW_SYMLINKS);
+        FileInfo child_info = null;
+        while((child_info = enumerator.next_file()) != null) {
+            names.add(child_info.get_name());
+        }
+        // GLib.CompareFunc<string> sorts strcmp-style, i.e. by byte value
+        names.sort((a, b) => { return strcmp(a, b); });
+
+        foreach(var name in names) {
+            var child_relative = relative.length == 0 ? name : Path.build_filename(relative, name);
+            digest_entry(root, child_relative, checksum);
+        }
+    }
+
+    /**
+     * Feeds {@link checksum} one entry's record: a header naming the
+     * relative path, the type tag and the type-significant data (mode
+     * digits for files and directories, the link target string for
+     * symlinks, then the file bytes for a regular file). Every field is
+     * newline-terminated so a name or target containing the separator
+     * cannot be confused with a field boundary.
+     */
+    private static void digest_entry(string root, string relative, Checksum checksum) throws Error {
+        var file = File.new_build_filename(root, relative);
+        var info = file.query_info(
+            FileAttribute.STANDARD_TYPE + "," + FileAttribute.STANDARD_SYMLINK_TARGET + "," + FileAttribute.UNIX_MODE,
+            FileQueryInfoFlags.NOFOLLOW_SYMLINKS);
+
+        var record = new StringBuilder();
+        record.append("path\n");
+        record.append(relative);
+        record.append("\n");
+
+        var type = info.get_file_type();
+        switch(type) {
+            case FileType.DIRECTORY:
+                record.append_printf("directory %04o\n", info.get_attribute_uint32(FileAttribute.UNIX_MODE) & 0777);
+                digest_string(checksum, record.str);
+                digest_tree(root, relative, checksum);
+                return;
+            case FileType.SYMBOLIC_LINK:
+                record.append("symlink\n");
+                record.append(info.get_attribute_byte_string(FileAttribute.STANDARD_SYMLINK_TARGET) ?? "");
+                record.append("\n");
+                digest_string(checksum, record.str);
+                return;
+            case FileType.REGULAR:
+                record.append_printf("file %04o\n", info.get_attribute_uint32(FileAttribute.UNIX_MODE) & 0777);
+                digest_string(checksum, record.str);
+                digest_file(checksum, file);
+                return;
+            default:
+                // FIFOs, sockets and devices carry no portable
+                // description; hash them by name alone
+                record.append_printf("special %04o\n", info.get_attribute_uint32(FileAttribute.UNIX_MODE) & 0777);
+                digest_string(checksum, record.str);
+                return;
+        }
+    }
+
+    /** Feeds one string record to {@link checksum}. */
+    private static void digest_string(Checksum checksum, string record) {
+        checksum.update(record.data, record.data.length);
+    }
+
+    /** Feeds the full bytes of `file` to {@link checksum}. */
+    private static void digest_file(Checksum checksum, File file) throws Error {
+        var stream = file.read();
+        const int chunk_size = 1024 * 1024;
+        var buffer = new uint8[chunk_size];
+        while(true) {
+            var read = stream.read(buffer);
+            if(read == 0) {
+                break;
+            }
+            checksum.update(buffer, read);
+        }
+    }
+
+}

+ 234 - 0
src/tests/TestMain.vala

@@ -1761,6 +1761,234 @@ exit 1
         Usm.Util.delete_tree(scratch);
     }
 
+    // ---- Conditional build-artifact re-archiving --------------------------------
+
+    /**
+     * A CachedPackage over a scratch cache directory holding a build
+     * directory with a fixed tree (a file, a sub-directory with a file
+     * and a symbolic link) — the seed for the re-archive tests.
+     */
+    Usm.CachedPackage make_built_package(string scratch, string name) throws Error {
+        var cache_path = Path.build_filename(scratch, "state", "packages", @"$name-1.0.0");
+        DirUtils.create_with_parents(Path.build_filename(cache_path, "build", "sub"), 0755);
+        FileUtils.set_contents(Path.build_filename(cache_path, "build", "one.txt"), "first\n");
+        FileUtils.set_contents(Path.build_filename(cache_path, "build", "sub", "two.txt"), "second\n");
+        File.new_for_path(Path.build_filename(cache_path, "build", "link"))
+            .make_symbolic_link("one.txt");
+        return new Usm.CachedPackage(cache_path);
+    }
+
+    /** Archives the package's build directory and records its digest sidecar. */
+    void archive_built_package(Usm.CachedPackage package) throws Error {
+        var build_dir = Path.build_filename(package.state_path, "build");
+        var archive_path = Path.build_filename(package.state_path, "build.tar.xz");
+        Usm.Util.archive(build_dir, archive_path);
+        FileUtils.set_contents(Path.build_filename(package.state_path, "build.tar.xz.digest"),
+            Usm.Util.tree_digest(build_dir) + "\n");
+    }
+
+    /** Extracts `archive` into a fresh scratch directory and returns the contents of `member`. */
+    string archived_member(string scratch, string archive, string member) throws Error {
+        var extracted = Path.build_filename(scratch, @"extract-$(Uuid.string_random())");
+        DirUtils.create(extracted, 0755);
+        Usm.Util.unarchive(archive, extracted);
+        string contents = null;
+        FileUtils.get_contents(Path.build_filename(extracted, member), out contents);
+        return contents;
+    }
+
+    /** Modification time of `path` in seconds since the epoch. */
+    int64 file_mtime(string path) {
+        return Stat(path).st_mtime;
+    }
+
+    void test_archive_build_skips_unchanged_tree() throws Error {
+        var scratch = make_scratch();
+        var package = make_built_package(scratch, "skip-pkg");
+        archive_built_package(package);
+
+        var archive_path = Path.build_filename(package.state_path, "build.tar.xz");
+        var digest_path = Path.build_filename(package.state_path, "build.tar.xz.digest");
+        string before = null;
+        FileUtils.get_contents(archive_path, out before);
+        var before_mtime = file_mtime(archive_path);
+
+        var last_fraction = -1.0f;
+        package.archive_build((fraction) => { last_fraction = fraction; });
+
+        string after = null;
+        FileUtils.get_contents(archive_path, out after);
+        check(after == before, "an unchanged build tree keeps the archive bytes");
+        check(file_mtime(archive_path) == before_mtime, "an unchanged build tree keeps the archive mtime");
+        check(!File.new_for_path(Path.build_filename(package.state_path, "build")).query_exists(),
+            "skipping the re-archive still cleans the build directory");
+        check(File.new_for_path(digest_path).query_exists(), "skipping the re-archive keeps the digest sidecar");
+        check(last_fraction == 1.0f, "skipping the re-archive still completes progress");
+
+        Usm.Util.delete_tree(scratch);
+    }
+
+    /** One mutation of a build tree before it is re-archived. */
+    delegate void BuildTreeMutation(string build_dir) throws Error;
+
+    /**
+     * One re-archive-on-change case: seeds a package named <name> with a
+     * build tree, archives it, applies <mutate> (which may throw — a
+     * failed mutation must fail the test, not silently re-test the
+     * unchanged tree), re-archives and asserts the archive was rewritten.
+     */
+    void rearchived_after_change(string scratch, string name, owned BuildTreeMutation mutate) throws Error {
+        var package = make_built_package(scratch, name);
+        archive_built_package(package);
+        var digest_path = Path.build_filename(package.state_path, "build.tar.xz.digest");
+        string digest_before = null;
+        FileUtils.get_contents(digest_path, out digest_before);
+
+        mutate(Path.build_filename(package.state_path, "build"));
+        // Computed before archive_build cleans the directory: the digest
+        // the fresh sidecar has to describe
+        var changed_tree_digest = Usm.Util.tree_digest(Path.build_filename(package.state_path, "build"));
+        package.archive_build(null);
+
+        string digest_after = null;
+        FileUtils.get_contents(digest_path, out digest_after);
+        check(digest_after.strip() != digest_before.strip(),
+            @"a changed build tree ($name) re-archives into a new digest sidecar");
+        check(digest_after.strip() == changed_tree_digest,
+            @"a changed build tree ($name) records its own tree in the fresh sidecar");
+        check(!File.new_for_path(Path.build_filename(package.state_path, "build")).query_exists(),
+            @"a changed build tree ($name) still cleans the build directory");
+
+        Usm.Util.delete_tree(scratch);
+    }
+
+    void test_archive_build_rearchives_changes() throws Error {
+        var scratch = make_scratch();
+
+        rearchived_after_change(scratch, "mod-pkg", (build) => {
+            FileUtils.set_contents(Path.build_filename(build, "one.txt"), "changed\n");
+        });
+        check(true, "a modified file re-archives");
+
+        rearchived_after_change(scratch, "add-pkg", (build) => {
+            FileUtils.set_contents(Path.build_filename(build, "new.txt"), "new\n");
+        });
+        check(true, "an added file re-archives");
+
+        rearchived_after_change(scratch, "rm-pkg", (build) => {
+            File.new_for_path(Path.build_filename(build, "sub", "two.txt")).delete();
+        });
+        check(true, "a removed file re-archives");
+
+        rearchived_after_change(scratch, "mode-pkg", (build) => {
+            FileUtils.chmod(Path.build_filename(build, "one.txt"), 0700);
+        });
+        check(true, "a changed file mode re-archives");
+
+        rearchived_after_change(scratch, "link-pkg", (build) => {
+            File.new_for_path(Path.build_filename(build, "link")).delete();
+            File.new_for_path(Path.build_filename(build, "link")).make_symbolic_link("sub/two.txt");
+        });
+        check(true, "a changed symlink target re-archives");
+    }
+
+    void test_archive_build_rearchives_modified_content() throws Error {
+        var scratch = make_scratch();
+        var package = make_built_package(scratch, "content-pkg");
+        archive_built_package(package);
+
+        var archive_path = Path.build_filename(package.state_path, "build.tar.xz");
+        FileUtils.set_contents(Path.build_filename(package.state_path, "build", "one.txt"), "changed\n");
+        package.archive_build(null);
+
+        check(archived_member(scratch, archive_path, "one.txt") == "changed\n",
+            "a modified file's new content lands in the re-archived archive");
+
+        Usm.Util.delete_tree(scratch);
+    }
+
+    void test_archive_build_rearchives_without_sidecar() throws Error {
+        var scratch = make_scratch();
+        var package = make_built_package(scratch, "legacy-pkg");
+        var build_dir = Path.build_filename(package.state_path, "build");
+        var archive_path = Path.build_filename(package.state_path, "build.tar.xz");
+        Usm.Util.archive(build_dir, archive_path); // legacy: no digest sidecar
+        var tree_digest_before = Usm.Util.tree_digest(build_dir);
+
+        package.archive_build(null);
+
+        string digest = null;
+        FileUtils.get_contents(Path.build_filename(package.state_path, "build.tar.xz.digest"), out digest);
+        check(digest != null && digest.strip() == tree_digest_before,
+            "an archive without a digest sidecar re-archives and records the digest sidecar");
+        check(archived_member(scratch, archive_path, "one.txt") == "first\n",
+            "the re-archived legacy archive holds the build tree's contents");
+
+        Usm.Util.delete_tree(scratch);
+    }
+
+    void test_build_digest_sidecar_lifecycle() throws Error {
+        var scratch = make_scratch();
+        var package = make_built_package(scratch, "cycle-pkg");
+        var cache_path = package.state_path;
+        var archive_path = Path.build_filename(cache_path, "build.tar.xz");
+        var digest_path = Path.build_filename(cache_path, "build.tar.xz.digest");
+
+        // get_build_archive archives a bare build directory
+        var returned = package.get_build_archive();
+        check(returned == archive_path, "get_build_archive creates the archive from the build directory");
+        check(File.new_for_path(digest_path).query_exists(),
+            "get_build_archive records the digest sidecar alongside the archive");
+
+        // An archive-only cache keeps its sidecar; unarchiving leaves it alone
+        FileUtils.set_contents(digest_path, "describes-the-archive-not-the-tree\n");
+        package.unarchive_build();
+        check(File.new_for_path(digest_path).query_exists(),
+            "unarchiving a build artifact leaves the digest sidecar alone");
+
+        // A second archive_build over the restored tree matches the digest
+        var digest = Usm.Util.tree_digest(Path.build_filename(cache_path, "build"));
+        FileUtils.set_contents(digest_path, digest + "\n");
+        var before_mtime = file_mtime(archive_path);
+        package.archive_build(null);
+        check(file_mtime(archive_path) == before_mtime,
+            "an archive restored and re-archived unchanged keeps its bytes");
+
+        // clean_build_cache removes the archive together with the sidecar
+        package.clean_build_cache();
+        check(!File.new_for_path(archive_path).query_exists() && !File.new_for_path(digest_path).query_exists(),
+            "clean_build_cache removes the archive and the digest sidecar together");
+
+        Usm.Util.delete_tree(scratch);
+    }
+
+    void test_tree_digest_determinism() throws Error {
+        var scratch = make_scratch();
+
+        var tree = Path.build_filename(scratch, "tree");
+        DirUtils.create_with_parents(Path.build_filename(tree, "d"), 0755);
+        FileUtils.set_contents(Path.build_filename(tree, "a.txt"), "alpha\n");
+        FileUtils.set_contents(Path.build_filename(tree, "d", "b.txt"), "beta\n");
+        File.new_for_path(Path.build_filename(tree, "d", "link")).make_symbolic_link("../a.txt");
+
+        var first = Usm.Util.tree_digest(tree);
+        check(first == Usm.Util.tree_digest(tree), "hashing the same tree twice yields the same digest");
+
+        // Same content at a different path is a different tree
+        var moved = Path.build_filename(scratch, "moved");
+        DirUtils.create_with_parents(Path.build_filename(moved, "d"), 0755);
+        FileUtils.set_contents(Path.build_filename(moved, "z.txt"), "alpha\n");
+        FileUtils.set_contents(Path.build_filename(moved, "d", "b.txt"), "beta\n");
+        File.new_for_path(Path.build_filename(moved, "d", "link")).make_symbolic_link("../a.txt");
+        check(Usm.Util.tree_digest(moved) != first, "a same-content file at a different path digests differently");
+
+        // A dot-prefixed temp file inside the tree is part of the tree
+        FileUtils.set_contents(Path.build_filename(tree, ".usm-tmp"), "scratch\n");
+        check(Usm.Util.tree_digest(tree) != first, "a temporary file inside the tree changes the digest");
+
+        Usm.Util.delete_tree(scratch);
+    }
+
     void test_build_failure_without_cache_propagates() throws Error {
         var original_dir = Environment.get_current_dir();
         var scratch = make_scratch();
@@ -2292,6 +2520,12 @@ echo "Installing libmeson-pkg.so to /usr/lib"
             test_build_archive_restore_and_clean_retry();
             test_build_archive_restored_incrementally();
             test_build_failure_without_cache_propagates();
+            test_archive_build_skips_unchanged_tree();
+            test_archive_build_rearchives_changes();
+            test_archive_build_rearchives_modified_content();
+            test_archive_build_rearchives_without_sidecar();
+            test_build_digest_sidecar_lifecycle();
+            test_tree_digest_determinism();
         }
         catch(Error e) {
             failures++;