Generate the rolling snapshots under the record root they are frozen from

script/Build.sol

@@ -89,6 +89,7 @@ contract Build is BuildScript, RegistryDeploySuites {
         for (uint256 i = 0; i < contracts.length; i++) {
             LibRainDeploySnapshot.writeSnapshot(
                 vm,
+                recordRoot(),
                 LibRainDeploySnapshot.CANDIDATE,
                 contracts[i].contractName,
                 contracts[i].candidate.sourceCreationCode,

src/lib/LibRainDeploySnapshot.sol

@@ -49,6 +49,15 @@ error EmptyRelease(string tag);
 /// @param newestFrozenTag The newest tag already in the record.
 error NonMonotonicRelease(string tag, string newestFrozenTag);
 
+/// Thrown when a record root is not a path this library can place a record at.
+/// A root is interpolated into every snapshot path a caller hands it, so one
+/// that climbs out of the tree, starts at `/` or is empty makes the paths this
+/// library returns paths to somewhere else entirely — and `fs_permissions`, a
+/// consuming repo's config rather than this library's argument, the only thing
+/// standing between a generated file and an arbitrary location on disk.
+/// @param root The rejected root.
+error InvalidRecordRoot(string root);
+
 /// @title LibRainDeploySnapshot
 /// @notice Which release is being built, where its record lives, and how it is
 /// frozen. Release machinery, not code generation.
@@ -213,6 +222,51 @@ library LibRainDeploySnapshot {
     /// assertion was standing in for.
     string constant LIB_FS_ROOT = GENERATED_DIR;
 
+    /// Reverts unless `root` is a path of record root segments: at least one
+    /// segment, separated by single `/`, each of them at least one character
+    /// and every character an ASCII letter, a digit, `_`, `$` or `-`.
+    ///
+    /// That is `LibFs.requireTag`'s alphabet with `-` admitted as well, and a
+    /// root is held to it segment by segment for the reason `requireTag` states
+    /// of a tag: no character in the set is a path separator and none of them is
+    /// `.`, so no segment is `.` or `..` and none reaches past the single
+    /// directory it names. Refusing the empty segment is what carries that from
+    /// a segment to a path — it takes the leading `/` of an absolute path, the
+    /// trailing one, the doubled one, and the empty root itself.
+    ///
+    /// `requireTag` cannot be asked this, because the separators that make a
+    /// path a path are exactly what it refuses; the alphabet BETWEEN them is a
+    /// copy of its, held to it byte for byte by
+    /// `testRecordRootSegmentIsTheTagAlphabetPlusHyphen`. The `-` is the whole
+    /// of the widening and it is what this repo's own roots need: `src/generated`
+    /// is tag segments already, while every fixture root the tests build is
+    /// `test/generated-<something>` or `test/fixture-record`.
+    /// @param root The record root to check.
+    function requireRecordRoot(string memory root) internal pure {
+        bytes memory rootBytes = bytes(root);
+        uint256 segmentLength = 0;
+        for (uint256 i = 0; i < rootBytes.length; i++) {
+            bytes1 char = rootBytes[i];
+            if (char == "/") {
+                if (segmentLength == 0) {
+                    revert InvalidRecordRoot(root);
+                }
+                segmentLength = 0;
+                continue;
+            }
+            bool isLetter = (char >= 0x41 && char <= 0x5A) || (char >= 0x61 && char <= 0x7A);
+            bool isDigit = char >= 0x30 && char <= 0x39;
+            bool isUnderscoreOrDollar = char == 0x5F || char == 0x24;
+            if (!(isLetter || isDigit || isUnderscoreOrDollar || char == "-")) {
+                revert InvalidRecordRoot(root);
+            }
+            segmentLength++;
+        }
+        if (segmentLength == 0) {
+            revert InvalidRecordRoot(root);
+        }
+    }
+
     /// The directory holding a snapshot, rolling or frozen, under a record
     /// root.
     ///
@@ -221,11 +275,18 @@ library LibRainDeploySnapshot {
     /// that admitted a name the writer refuses is a reader pointed at a path
     /// nothing can ever have written, and a fixture record that admitted one
     /// would be a fixture of a layout the real record cannot hold.
+    ///
+    /// The root is checked here too, and this is where it has to be: it is the
+    /// one place the root becomes a path, and the two halves of that path are
+    /// concatenated caller input. A checked `dir` beside an unchecked root is
+    /// only the shorter half of the path confined.
     /// @param root The record root — `LIB_FS_ROOT` for a repo's real record.
+    /// MUST be a path of record root segments.
     /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`.
     /// MUST be drawn from `LibFs`'s tag alphabet.
     /// @return The directory path.
     function dirForSnapshot(string memory root, string memory dir) internal pure returns (string memory) {
+        requireRecordRoot(root);
         LibFs.requireTag(dir);
         return string.concat(root, "/", dir);
     }
@@ -259,6 +320,7 @@ library LibRainDeploySnapshot {
     /// as on how they are spelled: a reader that accepted what the writer
     /// refuses is the same divergence one step quieter.
     /// @param root The record root — `LIB_FS_ROOT` for a repo's real record.
+    /// MUST be a path of record root segments.
     /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`.
     /// MUST be drawn from `LibFs`'s tag alphabet.
     /// @param contractName The name of the contract. MUST be a Solidity
@@ -352,10 +414,17 @@ library LibRainDeploySnapshot {
     /// - the entry is a file directly inside it. Everything in a release
     ///   directory belongs to that release's record — there is no extension to
     ///   filter on, because nothing else has any business being in there.
+    /// The root is checked before the walk, because a root nothing can be
+    /// written under is not a record that happens to be empty. This is the one
+    /// root-taking entry point that does not reach `dirForSnapshot`, so the two
+    /// together are every way a root gets into this library.
     /// @param vm The Vm instance for file operations.
     /// @param root The record root — `LIB_FS_ROOT` for a repo's real record.
+    /// MUST be a path of record root segments.
     /// @return Every frozen record file.
     function frozenSnapshotPaths(Vm vm, string memory root) internal view returns (string[] memory) {
+        requireRecordRoot(root);
+
         // A repo with no generated directory at all has released nothing. That
         // is a real state — it is this repo's own, before its first release —
         // rather than a missing file to fail on.
@@ -458,20 +527,26 @@ library LibRainDeploySnapshot {
 
     /// Generate one snapshot for one contract.
     ///
-    /// There is no output root to choose. `LibFs.buildFileForTaggedContract`
-    /// derives its directory from `LIB_FS_ROOT` and the snapshot directory it is
-    /// handed, and this is the repo's real deploy record, which belongs under
-    /// that root and nowhere else. This is the one place a snapshot's bytes come
-    /// into existence, and they come from the compiler rather than from another
-    /// tree, so there is nothing for a root to select between.
-    ///
-    /// `freeze` does take a root and that is not the same freedom: it COPIES,
-    /// within one record tree, reading a rolling snapshot under the root it is
-    /// handed and writing the frozen copy under that same root. Pointing a
-    /// copier at a tree of its own is a thing a test genuinely needs, exactly
-    /// as pointing `frozenSnapshotPaths` at one is; GENERATING this repo's
-    /// record anywhere but under `LIB_FS_ROOT` remains something nothing here
-    /// can express.
+    /// The output root is the one `freeze` is handed, and it is required for
+    /// the same reason: `cutRelease()` regenerates and then freezes within ONE
+    /// record tree. A generator that could only write under `LIB_FS_ROOT` would
+    /// leave a release cut under any other root frozen from a rolling snapshot
+    /// its own regeneration never wrote — after writing the real record on the
+    /// way there, which is the tree a `recordRoot()` override exists to keep a
+    /// caller's hands off.
+    ///
+    /// `LibFs.buildFileForContract` takes the directory it writes into, so the
+    /// root reaches the writer through `dirForSnapshot(root, dir)`, which is
+    /// where both halves of the directory are checked: `dir` against `LibFs`'s
+    /// tag alphabet and the root against `requireRecordRoot`. The root is a
+    /// caller's string and it is the half that names where the tree IS, so an
+    /// unchecked one would make the output directory of every write here the
+    /// caller's to place anywhere `fs_permissions` allows — which is a
+    /// consuming repo's config, not an argument this library gets to see. At
+    /// `LIB_FS_ROOT` that directory is
+    /// `LibFs.dirForTag(dir)`, which is where
+    /// `testRootAwareSnapshotPathIsTheWritersAtTheRealRoot` holds the two
+    /// spellings to being one path.
     ///
     /// The dependency list is frozen here with the rest, and it is not
     /// metadata. `RainDeployBroadcast.run` hands a suite's `dependencies` to
@@ -490,6 +565,8 @@ library LibRainDeploySnapshot {
     /// repo's statement. Repos outside this org call this overload; repos
     /// inside it call the one that defaults to the org's values.
     /// @param vm The Vm instance for file operations.
+    /// @param root The record root to generate into — `LIB_FS_ROOT` for a
+    /// repo's real record. MUST be a path of record root segments.
     /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`.
     /// @param contractName The contract the snapshot describes.
     /// @param spdxLicenseIdentifier The SPDX licence identifier the written
@@ -501,6 +578,7 @@ library LibRainDeploySnapshot {
     /// @return The path written.
     function writeSnapshot(
         Vm vm,
+        string memory root,
         string memory dir,
         string memory contractName,
         string memory spdxLicenseIdentifier,
@@ -513,18 +591,21 @@ library LibRainDeploySnapshot {
         address deployed = LibRainDeploy.deployZoltu(creationCode);
         string memory constants = snapshotConstants(vm, deployed, creationCode, dependencies);
 
-        // The directory is created by the writer, from the same tag this path is
-        // derived from, so there is no `createDir` here to disagree with it.
-        LibFs.buildFileForTaggedContract(
-            vm, deployed, dir, contractName, spdxLicenseIdentifier, copyrightText, constants
+        // The directory is created by the writer, from the same root and tag
+        // this path is derived from, so there is no `createDir` here to
+        // disagree with it.
+        LibFs.buildFileForContract(
+            vm, deployed, dirForSnapshot(root, dir), contractName, spdxLicenseIdentifier, copyrightText, constants
         );
 
-        return pathForSnapshot(dir, contractName);
+        return pathForSnapshot(root, dir, contractName);
     }
 
     /// `writeSnapshot` applied to `RAIN_SPDX_LICENSE_IDENTIFIER` and
     /// `RAIN_COPYRIGHT_TEXT`, for a repo this org owns.
     /// @param vm The Vm instance for file operations.
+    /// @param root The record root — `LIB_FS_ROOT` for a repo's real record.
+    /// MUST be a path of record root segments.
     /// @param dir The snapshot directory name — a release tag, or `CANDIDATE`.
     /// @param contractName The contract the snapshot describes.
     /// @param creationCode That contract's creation code.
@@ -533,13 +614,14 @@ library LibRainDeploySnapshot {
     /// @return The path written.
     function writeSnapshot(
         Vm vm,
+        string memory root,
         string memory dir,
         string memory contractName,
         bytes memory creationCode,
         address[] memory dependencies
     ) internal returns (string memory) {
         return writeSnapshot(
-            vm, dir, contractName, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT, creationCode, dependencies
+            vm, root, dir, contractName, RAIN_SPDX_LICENSE_IDENTIFIER, RAIN_COPYRIGHT_TEXT, creationCode, dependencies
         );
     }
 

test/concrete/BuildRecordRootHarness.sol

@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: LicenseRef-DCL-1.0
+// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd
+pragma solidity =0.8.25;
+
+import {BuildScript} from "../../src/abstract/BuildScript.sol";
+import {BuildHarness} from "./BuildHarness.sol";
+
+/// @title BuildRecordRootHarness
+/// @notice `Build` with its record root overridden — the one thing
+/// `BuildScript` documents the root as being overridable for — and the
+/// regeneration `cutRelease()` freezes from reachable from a test.
+///
+/// The regeneration alone. `run()` and `cutRelease()` also regenerate the libs,
+/// and those are written into `LIB_DIR`, which no override moves, so either
+/// entry point would rewrite committed libs that other test contracts read
+/// while forge runs them in parallel.
+contract BuildRecordRootHarness is BuildHarness {
+    string internal sRoot;
+
+    constructor(string memory root) {
+        sRoot = root;
+    }
+
+    /// @inheritdoc BuildScript
+    function recordRoot() internal view override returns (string memory) {
+        return sRoot;
+    }
+
+    function externalRegenerateSnapshots() external {
+        regenerateSnapshots();
+    }
+}

test/script/BuildRecordRoot.t.sol

@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: LicenseRef-DCL-1.0
+// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd
+pragma solidity =0.8.25;
+
+import {Test} from "forge-std-1.16.2/src/Test.sol";
+import {LibRainDeploySnapshot} from "../../src/lib/LibRainDeploySnapshot.sol";
+import {BuildRecordRootHarness} from "../concrete/BuildRecordRootHarness.sol";
+
+/// @title BuildRecordRootTest
+/// @notice Where `script/Build.sol` writes when `recordRoot()` is overridden —
+/// the one thing `BuildScript` documents the root as being overridable for.
+///
+/// A contract of its own because this one WRITES, and `BuildTest` states that
+/// nothing in it does. What is asserted here is which tree the write lands in,
+/// so the write is the subject rather than a side effect: it goes under this
+/// contract's own fixture root, and the committed record it would otherwise
+/// have gone into is read and left alone.
+contract BuildRecordRootTest is Test {
+    string constant FIXTURE_ROOT = "test/generated-build-record-root";
+
+    /// A cheatcode write is not undone by a revert, so a failure leaves
+    /// generated sources on disk and the next run reads THOSE.
+    function resetFixture(string memory root) internal {
+        if (vm.exists(root)) {
+            //forge-lint: disable-next-line(unsafe-cheatcode)
+            vm.removeDir(root, true);
+        }
+    }
+
+    /// PROPERTY: `Build`'s regeneration writes every rolling snapshot under
+    /// `recordRoot()`.
+    ///
+    /// `cutRelease()` freezes each contract from `pathForSnapshot(recordRoot(),
+    /// CANDIDATE, name)`, so a regeneration that ignores the root hands the
+    /// freeze a record nothing wrote — `NothingToFreeze` for any repo that
+    /// overrides the root — and rewrites the real `src/generated/candidate/` on
+    /// the way to that revert, which is the tree the override exists to keep a
+    /// caller's hands off.
+    ///
+    /// The bytes are the committed candidate's, read from the real record: the
+    /// regeneration is a function of what this repo compiles and the committed
+    /// snapshot is what it last compiled to, so an equal file under the fixture
+    /// root is the whole snapshot having moved rather than a file having been
+    /// created there.
+    function testRegenerateSnapshotsWritesUnderTheRecordRoot() external {
+        resetFixture(FIXTURE_ROOT);
+        BuildRecordRootHarness harness = new BuildRecordRootHarness(FIXTURE_ROOT);
+        string[] memory names = harness.externalSnapshotContractNames();
+
+        harness.externalRegenerateSnapshots();
+
+        // Read while the fixture is still there, asserted once it is gone.
+        bool[] memory written = new bool[](names.length);
+        string[] memory regenerated = new string[](names.length);
+        string[] memory committed = new string[](names.length);
+        for (uint256 i = 0; i < names.length; i++) {
+            string memory path =
+                LibRainDeploySnapshot.pathForSnapshot(FIXTURE_ROOT, LibRainDeploySnapshot.CANDIDATE, names[i]);
+            written[i] = vm.exists(path);
+            regenerated[i] = written[i] ? vm.readFile(path) : "";
+            committed[i] = vm.readFile(LibRainDeploySnapshot.pathForSnapshot(LibRainDeploySnapshot.CANDIDATE, names[i]));
+        }
+
+        resetFixture(FIXTURE_ROOT);
+
+        assertTrue(names.length > 0, "no contract is generated, so nothing was asserted");
+        for (uint256 i = 0; i < names.length; i++) {
+            assertTrue(written[i], string.concat("regeneration wrote nothing under the record root: ", names[i]));
+            assertEq(
+                regenerated[i],
+                committed[i],
+                string.concat("snapshot under the record root is not the committed one: ", names[i])
+            );
+        }
+    }
+}