Stamp a release as fiesta's own, publish nothing hidden, and bound the tag check
fiesta release -candidate accepted any directory whose files agreed with a sha256sum-format SHA256SUMS as its own, and the review emptied a foreign checksummed folder with it. Ownership is now claim's rule, the one build and gallery use: the directory must carry fiesta release's stamp at the top, no other stamp below, and no symbolic link, or it is refused by name. The stamp is committed with the release, so a clone can still rebuild an untagged candidate. TestACandidateIsNotAnyChecksummedDirectory builds the reviewer's folder (data.bin, readme.txt, valid sums) in an untagged scratch repo and was red against ad593b2: "fiesta release -candidate emptied .../releases/1.0.0, a checksummed folder that is not fiesta's". The candidate test now also refuses another command's stamp and a missing stamp, and a stray file in a directory the command owns is replaced with the rest, as claim's rule says. site.Release skips exactly one hidden file, the stamp at the top of a release, and publishes nothing else hidden: any other dotfile, or a stamp deeper down, is unlisted and refused by name. site.Build refuses a hidden page or directory under pages/ by name rather than skipping it, since pages/ is committed and a dotfile there is a mistake worth seeing; skipping would also hide a stamp that a wrong -pages pointed at. The stamp name is one constant, site.StampName, which claim.go now uses, so the two cannot drift. TestTheStampIsSkippedAndNeverPublished and TestHiddenPagesAreRefused were red against ad593b2 (with only the constant added so the file compiled). fiesta site writes the tree to ship under -out/public and claims -out itself, so its .fiesta stamp sits above the shipped tree and .build/site/public holds nothing hidden. TestSiteWritesWhatBuildReturns walks the shipped tree for dotfiles and checks -out holds exactly the stamp and public; the scratch mutation that claims -out/public instead reddens it four ways. SHA256SUMS is checked with fs.Lstat before it is read and must be a regular file, so a linked sums file is refused by name rather than read through. TestALinkedSumsFileIsRefused was red against ad593b2. tagExists runs both git calls under a 30 second context and with GIT_TERMINAL_PROMPT=0, so a remote that hangs or asks for credentials is a refusal rather than a wait. TestAnUnreachableOriginRefusesACandidate points origin at a missing path and pins the refusal; the scratch mutation that treats an ls-remote failure as "not tagged" reddens it. The candidate tests still skip without git, but say so on stderr as well as in the skip, and FIESTA_TEST_REQUIRE_GIT turns the skip into a failure. The Makefile sets it for root and the test roles, beside FIESTA_TEST_REQUIRE_NODE. Also: the missing-flag error for fiesta site names the first missing flag in a fixed order rather than map order.
Human
Prompt
Fix round 1: tie -candidate's ownership check to Fiesta (the stamp route), publish no hidden files from site/ or releases/X.Y.Z/ and keep the site stamp out of the shipped tree, pin the unreachable-origin refusal, don't follow a symlinked SHA256SUMS, give ls-remote GIT_TERMINAL_PROMPT=0 and a timeout, and make the git skip loud with FIESTA_TEST_REQUIRE_GIT failing it under make ci.
From this commit's own message. The prompt ledger has no entries for it — which is what a squashed, rebased or amended commit looks like, and what every commit looks like in a repo without a ledger.
Summary
fiesta release now claims its directory with a stamp like build and gallery; site.Build skips that one stamp and refuses every other hidden file; fiesta site writes the shipped tree under -out/public with the stamp above it; tagExists is bounded and never prompts; new tests for a foreign checksummed folder, an unreachable origin, hidden pages, a linked sums file, and the hidden-free shipped tree.
Agent notes
The stamp is committed with each release, so Task 15's freshness check over releases/1.0.0 must ignore .fiesta, and the ship step ships .build/site/public, not .build/site.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
7 files changed,
+469
−120
Makefile+12 −5cmd/fiesta/claim.go+10 −6cmd/fiesta/main.go+74 −43cmd/fiesta/release_foreign_test.go+67 −0cmd/fiesta/release_test.go+154 −62internal/site/site.go+47 −4internal/site/site_test.go+105 −0
diff --git a/Makefile b/Makefile| index e11c0f1..935dd8d 100644 |
| --- a/Makefile |
| +++ b/Makefile |
| @@ -100,17 +100,22 @@ size: |
| # would otherwise go green having checked none of the JavaScript. |
| # Override with FIESTA_TEST_REQUIRE_NODE= to run without it. |
| FIESTA_TEST_REQUIRE_NODE ?= 1 |
| +# FIESTA_TEST_REQUIRE_GIT does the same for git: the tag guard of |
| +# fiesta release -candidate is tested in a scratch repository, and a |
| +# runner without git would otherwise pass with the guard untested. |
| +FIESTA_TEST_REQUIRE_GIT ?= 1 |
| +TEST_ENV := FIESTA_TEST_REQUIRE_NODE=$(FIESTA_TEST_REQUIRE_NODE) FIESTA_TEST_REQUIRE_GIT=$(FIESTA_TEST_REQUIRE_GIT) |
| # root builds, vets and tests every package. The browser drives are |
| # behind the browser tag and run as roles of their own. |
| root: |
| go build ./... |
| go vet ./... |
| - FIESTA_TEST_REQUIRE_NODE=$(FIESTA_TEST_REQUIRE_NODE) go test -count=1 ./... |
| + $(TEST_ENV) go test -count=1 ./... |
| # test.<pkg>[~<k>of<n>]: one package of plain tests as a role of its own, |
| # for a package too slow to sit inside root. |
| test.%: | $(BIN)/tmp |
| - FIESTA_TEST_REQUIRE_NODE=$(FIESTA_TEST_REQUIRE_NODE) ./hack/gotest.sh test '$*' |
| + $(TEST_ENV) ./hack/gotest.sh test '$*' |
| # staticcheck catches what vet does not: deprecated APIs, values that are |
| # never read, and the &*x that looks like a copy and is not. |
| @@ -182,11 +187,13 @@ export FULL_CHROMIUM := check |
| browser.%: | $(BIN)/tmp |
| ./hack/gotest.sh browser '$*' |
| -# site assembles fiesta.rastrillo.org into .build/site from the |
| +# site assembles fiesta.rastrillo.org into .build/site/public from the |
| # committed releases and the pages: each release under its exact |
| # version, _redirects sending /M/* to the newest M.y.z, and site/ at |
| -# the root. What it writes is what gets shipped; nothing in ci depends |
| -# on it, since the releases are committed and checked there. |
| +# the root. .build/site/public is the tree to ship; the command's own |
| +# .fiesta stamp sits above it in .build/site, so nothing hidden is in |
| +# the shipped tree. Nothing in ci depends on this target, since the |
| +# releases are committed and checked there. |
| site: | $(BIN)/tmp |
| go run ./cmd/fiesta site -releases releases -pages site -out .build/site |
diff --git a/cmd/fiesta/claim.go b/cmd/fiesta/claim.go| index 67a2d44..c05bd0c 100644 |
| --- a/cmd/fiesta/claim.go |
| +++ b/cmd/fiesta/claim.go |
| @@ -6,6 +6,8 @@ import ( |
| "os" |
| "path/filepath" |
| "regexp" |
| + |
| + "amadan.net/rastrillo/fiesta/internal/site" |
| ) |
| // The ownership check is rastrillo's dsgen's, with the stamp renamed. |
| @@ -15,12 +17,14 @@ import ( |
| // write, which was safe and left a renamed file published for as long |
| // as the build directory lived. The stamp is how it gets both. |
| -// stampName is the file fiesta build and fiesta gallery leave in a |
| -// directory to say the directory is its own. Dotted so it stays out of |
| -// the way of whatever serves the tree. One name for both commands, and |
| -// the body says which wrote it: a directory one of them wrote is not |
| -// a directory the other may empty. |
| -const stampName = ".fiesta" |
| +// stampName is the file fiesta build, fiesta gallery, fiesta release |
| +// and fiesta site leave in a directory to say the directory is its |
| +// own. Dotted so it stays out of the way of whatever serves the tree. |
| +// One name for every command, and the body says which wrote it: a |
| +// directory one of them wrote is not a directory another may empty. |
| +// The name is site's, because site.Release skips exactly this file in |
| +// a committed release directory and publishes nothing else hidden. |
| +const stampName = site.StampName |
| // stampWriter reads the command a stamp names. |
| var stampWriter = regexp.MustCompile(`written by (fiesta [a-z]+),`) |
diff --git a/cmd/fiesta/main.go b/cmd/fiesta/main.go| index 8dcd035..08019ac 100644 |
| --- a/cmd/fiesta/main.go |
| +++ b/cmd/fiesta/main.go |
| @@ -20,30 +20,32 @@ |
| // fiesta release -version X.Y.Z [-releases DIR] [-candidate] |
| // |
| // writes the same release build makes into DIR/X.Y.Z/ (DIR is |
| -// releases/ by default), to be committed: the releases directory is |
| -// what fiesta site publishes. The version must be fiesta.Version, the |
| -// one in the tree. A directory that already exists is refused, since a |
| -// committed release never changes, unless -candidate is passed and no |
| -// tag vX.Y.Z exists locally or on origin: an unpublished candidate may |
| -// be rebuilt until it is tagged. See releaseCmd. |
| +// releases/ by default), to be committed, stamp included: the |
| +// releases directory is what fiesta site publishes, and the stamp is |
| +// the one file in it the site never copies. The version must be |
| +// fiesta.Version, the one in the tree. A directory that already |
| +// exists is refused, since a committed release never changes, unless |
| +// -candidate is passed and no tag vX.Y.Z exists locally or on origin: |
| +// an unpublished candidate may be rebuilt until it is tagged. See |
| +// release. |
| // |
| // fiesta site -releases DIR -pages DIR -out DIR |
| // |
| // assembles the site from the releases and the pages, exactly what |
| -// internal/site.Build returns, into -out: each release under its exact |
| -// version, _redirects sending /M/* to the newest M.y.z, and the pages |
| -// at the root. |
| +// internal/site.Build returns, into DIR/public/: each release under |
| +// its exact version, _redirects sending /M/* to the newest M.y.z, and |
| +// the pages at the root. DIR/public/ is the tree to ship, and the |
| +// stamp sits above it in DIR, so nothing hidden is ever deployed. |
| // |
| -// build, gallery and site each own the directory they write. They |
| -// empty it before they write, so what is there afterwards is the |
| -// output and only the output, and they will only take a directory that |
| -// is absent, empty, or marked with their own stamp (.fiesta); anything |
| -// else they refuse, changing nothing. See claim. release writes no |
| -// stamp, since its output is committed and published as is; its |
| -// ownership check is the release's own SHA256SUMS. |
| +// Each command owns the directory it writes. It empties it before it |
| +// writes, so what is there afterwards is the output and only the |
| +// output, and it will only take a directory that is absent, empty, or |
| +// marked with its own stamp (.fiesta); anything else it refuses, |
| +// changing nothing. See claim. |
| package main |
| import ( |
| + "context" |
| "errors" |
| "flag" |
| "fmt" |
| @@ -53,6 +55,7 @@ import ( |
| "path/filepath" |
| "sort" |
| "strings" |
| + "time" |
| "amadan.net/rastrillo/fiesta" |
| "amadan.net/rastrillo/fiesta/internal/build" |
| @@ -161,9 +164,9 @@ func siteCmd(args []string, stderr io.Writer) error { |
| if ok, err := parse(flags, args, stderr); !ok { |
| return err |
| } |
| - for name, value := range map[string]string{"-releases": *releases, "-pages": *pages, "-out": *out} { |
| - if value == "" { |
| - return errors.New(name + " is required\n" + usage) |
| + for _, f := range []struct{ name, value string }{{"-releases", *releases}, {"-pages", *pages}, {"-out", *out}} { |
| + if f.value == "" { |
| + return errors.New(f.name + " is required\n" + usage) |
| } |
| } |
| return writeSite(*releases, *pages, *out) |
| @@ -190,14 +193,25 @@ func writeGallery(out, mount string) error { |
| return writeTree(out, "fiesta gallery", files) |
| } |
| +// shipDir is the directory under fiesta site's -out that holds the |
| +// tree to ship. The stamp claim writes sits in -out itself, above it, |
| +// so the shipped tree holds nothing hidden: site.Build refuses hidden |
| +// pages and skips the one hidden file a release carries, and this |
| +// keeps the command's own mark out too. |
| +const shipDir = "public" |
| + |
| // writeSite assembles the site from releases and pages and puts it in |
| -// out, a directory it claims first. |
| +// out/public/, after claiming out. |
| func writeSite(releases, pages, out string) error { |
| files, err := site.Build(os.DirFS(releases), os.DirFS(pages)) |
| if err != nil { |
| return err |
| } |
| - return writeTree(out, "fiesta site", files) |
| + shipped := make(map[string][]byte, len(files)) |
| + for name, data := range files { |
| + shipped[shipDir+"/"+name] = data |
| + } |
| + return writeTree(out, "fiesta site", shipped) |
| } |
| // release builds the dist at version and writes it to releases/<version>/. |
| @@ -211,22 +225,24 @@ func writeSite(releases, pages, out string) error { |
| // is refused: a release that has gone out never changes. The one |
| // exception is -candidate, for a release that has been built and |
| // committed but not yet tagged, which may be rebuilt after a fix. It |
| -// is allowed only while every one of these holds, and refused by name |
| +// is allowed only while both of these hold, and refused by name |
| // otherwise: |
| // |
| // - no tag v<version> exists locally (git rev-parse) or on origin |
| // (git ls-remote), so nothing has been published under the name; |
| // a git that cannot answer is a refusal, not an absence; |
| -// - the directory is a release Fiesta wrote: every file in it is a |
| -// regular file listed in its SHA256SUMS with matching bytes, and |
| -// every listed file is present (site.Release). That is the |
| -// ownership check, in place of the stamp build and gallery use, |
| -// because a release is published as is and carries no stamp. |
| +// - the directory is fiesta release's own, by claim's rule: it holds |
| +// this command's stamp at the top, no other stamp anywhere below, |
| +// and no symbolic link. The stamp is committed with the release |
| +// and is the one file in the directory site.Release does not |
| +// publish. A directory of files that merely agree with a |
| +// SHA256SUMS is not enough: that is the sha256sum format, and any |
| +// checksummed folder has one. |
| // |
| // An empty directory is taken either way, since there is nothing in it |
| -// to lose. A candidate that passes is removed whole and rewritten, so |
| -// a file the new dist no longer has is gone too, and the directory is |
| -// the dist, every time. |
| +// to lose. A candidate that passes is emptied and rewritten, so a file |
| +// the new dist no longer has is gone too, and the directory is the |
| +// dist, every time. |
| func release(releases, version string, candidate bool) error { |
| if version != fiesta.Version { |
| return fmt.Errorf("the tree is version %s, so that is the only release it can build; -version %s was asked for", fiesta.Version, version) |
| @@ -242,6 +258,13 @@ func release(releases, version string, candidate bool) error { |
| if err != nil { |
| return fmt.Errorf("resolving -releases: %w", err) |
| } |
| + // A link is resolved here as claim resolves it, so the refusal |
| + // below and the claim see the same directory. |
| + if real, err := filepath.EvalSymlinks(root); err == nil { |
| + root = real |
| + } else if !os.IsNotExist(err) { |
| + return fmt.Errorf("resolving %s: %w", root, err) |
| + } |
| entries, err := os.ReadDir(root) |
| switch { |
| case os.IsNotExist(err): |
| @@ -259,42 +282,50 @@ func release(releases, version string, candidate bool) error { |
| return fmt.Errorf("refusing to replace %s: the tag %s exists, so it has been released. "+ |
| "A fix is the next version", root, tag) |
| } |
| - if _, err := site.Release(os.DirFS(root)); err != nil { |
| - return fmt.Errorf("refusing to replace %s: it is not a release fiesta wrote, so it is not this command's to empty: %s/%v. "+ |
| - "Empty it yourself, or point -releases somewhere else", root, root, err) |
| - } |
| - if err := os.RemoveAll(root); err != nil { |
| - return fmt.Errorf("emptying %s: %w", root, err) |
| - } |
| } |
| - if err := os.MkdirAll(root, 0o755); err != nil { |
| - return fmt.Errorf("creating %s: %w", root, err) |
| + if err := claim(root, "fiesta release"); err != nil { |
| + return err |
| } |
| return writeFiles(root, dist) |
| } |
| +// gitTimeout bounds each git call tagExists makes. ls-remote talks to |
| +// origin, and a remote that hangs would otherwise hold the command |
| +// forever; a run that outlives this is a refusal. |
| +const gitTimeout = 30 * time.Second |
| + |
| // tagExists reports whether tag is a tag of the repository holding |
| // dir, locally or on its origin. Either git failing to answer is an |
| // error: the caller is about to empty a directory on the strength of |
| -// the answer, and "I could not check" is not "no". |
| +// the answer, and "I could not check" is not "no". Neither call may |
| +// ask for credentials: GIT_TERMINAL_PROMPT=0 makes a remote that |
| +// wants them fail at once, which is a refusal too. |
| func tagExists(dir, tag string) (bool, error) { |
| - local := exec.Command("git", "-C", dir, "rev-parse", "-q", "--verify", "refs/tags/"+tag) |
| + ctx, cancel := context.WithTimeout(context.Background(), gitTimeout) |
| + defer cancel() |
| + env := append(os.Environ(), "GIT_TERMINAL_PROMPT=0") |
| + local := exec.CommandContext(ctx, "git", "-C", dir, "rev-parse", "-q", "--verify", "refs/tags/"+tag) |
| + local.Env = env |
| local.Stdout, local.Stderr = io.Discard, io.Discard |
| err := local.Run() |
| var exit *exec.ExitError |
| switch { |
| case err == nil: |
| return true, nil |
| - case errors.As(err, &exit) && exit.ExitCode() == 1: |
| + case errors.As(err, &exit) && exit.ExitCode() == 1 && ctx.Err() == nil: |
| // rev-parse -q --verify exits 1 for a ref that does not exist. |
| default: |
| return false, fmt.Errorf("git rev-parse: %w", err) |
| } |
| - remote := exec.Command("git", "-C", dir, "ls-remote", "--tags", "origin", tag) |
| + remote := exec.CommandContext(ctx, "git", "-C", dir, "ls-remote", "--tags", "origin", tag) |
| + remote.Env = env |
| var stderr strings.Builder |
| remote.Stderr = &stderr |
| out, err := remote.Output() |
| if err != nil { |
| + if ctx.Err() != nil { |
| + return false, fmt.Errorf("git ls-remote --tags origin: no answer within %s", gitTimeout) |
| + } |
| return false, fmt.Errorf("git ls-remote --tags origin: %w: %s", err, strings.TrimSpace(stderr.String())) |
| } |
| return strings.TrimSpace(string(out)) != "", nil |
diff --git a/cmd/fiesta/release_foreign_test.go b/cmd/fiesta/release_foreign_test.go| new file mode 100644 |
| index 0000000..dc73252 |
| --- /dev/null |
| +++ b/cmd/fiesta/release_foreign_test.go |
| @@ -0,0 +1,67 @@ |
| +package main |
| + |
| +import ( |
| + "crypto/sha256" |
| + "encoding/hex" |
| + "os" |
| + "path/filepath" |
| + "sort" |
| + "strings" |
| + "testing" |
| + |
| + "amadan.net/rastrillo/fiesta" |
| +) |
| + |
| +// TestACandidateIsNotAnyChecksummedDirectory: a directory whose files |
| +// agree with a sha256sum-format SHA256SUMS is not thereby fiesta's. A |
| +// foreign folder at releases/<version>/ with its own valid sums, in an |
| +// untagged repository with a reachable origin, is refused by |
| +// -candidate, naming the stamp it lacks, and nothing in it is touched. |
| +func TestACandidateIsNotAnyChecksummedDirectory(t *testing.T) { |
| + repo := scratchRepo(t) |
| + releases := filepath.Join(repo, "releases") |
| + dir := filepath.Join(releases, fiesta.Version) |
| + if err := os.MkdirAll(dir, 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + files := map[string][]byte{ |
| + "data.bin": {0, 1, 2, 3, 'n', 'o', 't', ' ', 'f', 'i', 'e', 's', 't', 'a'}, |
| + "readme.txt": []byte("someone else's notes\n"), |
| + } |
| + names := make([]string, 0, len(files)) |
| + for name := range files { |
| + names = append(names, name) |
| + } |
| + sort.Strings(names) |
| + var sums strings.Builder |
| + for _, name := range names { |
| + if err := os.WriteFile(filepath.Join(dir, name), files[name], 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + sum := sha256.Sum256(files[name]) |
| + sums.WriteString(hex.EncodeToString(sum[:]) + " " + name + "\n") |
| + } |
| + if err := os.WriteFile(filepath.Join(dir, "SHA256SUMS"), []byte(sums.String()), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + err := fiestaRelease(t, releases, fiesta.Version, true) |
| + if err == nil { |
| + t.Fatalf("fiesta release -candidate emptied %s, a checksummed folder that is not fiesta's", dir) |
| + } |
| + if !strings.Contains(err.Error(), stampName) { |
| + t.Errorf("the refusal does not name the missing %s stamp: %v", stampName, err) |
| + } |
| + for name, want := range files { |
| + got, err := os.ReadFile(filepath.Join(dir, name)) |
| + if err != nil || string(got) != string(want) { |
| + t.Errorf("the refused directory was touched: %s is %q, %v", name, got, err) |
| + } |
| + } |
| + if _, err := os.Stat(filepath.Join(dir, "fiesta.css")); !os.IsNotExist(err) { |
| + t.Errorf("the dist was written into the refused directory: %v", err) |
| + } |
| + if entries, _ := os.ReadDir(dir); len(entries) != len(files)+1 { |
| + t.Errorf("the refused directory holds %d entries, want %d", len(entries), len(files)+1) |
| + } |
| +} |
diff --git a/cmd/fiesta/release_test.go b/cmd/fiesta/release_test.go| index 773895f..da9fa3a 100644 |
| --- a/cmd/fiesta/release_test.go |
| +++ b/cmd/fiesta/release_test.go |
| @@ -2,6 +2,7 @@ package main |
| import ( |
| "bytes" |
| + "fmt" |
| "io" |
| "os" |
| "os/exec" |
| @@ -14,6 +15,29 @@ import ( |
| "amadan.net/rastrillo/fiesta/internal/site" |
| ) |
| +// requireGitEnv names the environment variable that turns a missing |
| +// git from a skip into a failure, as FIESTA_TEST_REQUIRE_NODE does for |
| +// node. make ci sets it: the tag guard of fiesta release -candidate is |
| +// tested in a scratch repository, and a runner without git would |
| +// otherwise go green with the guard untested. |
| +const requireGitEnv = "FIESTA_TEST_REQUIRE_GIT" |
| + |
| +// requireGit skips the test when git is not on PATH, saying so on |
| +// stderr as well, since go test prints a skip's reason only under -v, |
| +// or fails it when requireGitEnv is set. |
| +func requireGit(t *testing.T) { |
| + t.Helper() |
| + if _, err := exec.LookPath("git"); err == nil { |
| + return |
| + } |
| + msg := fmt.Sprintf("%s: git is not on PATH, so the tag guard of fiesta release -candidate goes unverified", t.Name()) |
| + if os.Getenv(requireGitEnv) != "" { |
| + t.Fatalf("%s (%s is set, so this is a failure, not a skip)", msg, requireGitEnv) |
| + } |
| + fmt.Fprintln(os.Stderr, "SKIP "+msg) |
| + t.Skip(msg) |
| +} |
| + |
| // fiestaRelease runs `fiesta release -version <version> -releases |
| // <releases> [-candidate]` as main would, and returns its error. |
| func fiestaRelease(t *testing.T, releases, version string, candidate bool) error { |
| @@ -35,8 +59,8 @@ func dist(t *testing.T) map[string][]byte { |
| return d |
| } |
| -// assertIsTheDist checks that dir holds the dist file for file and |
| -// nothing else: no stamp, no leftovers. |
| +// assertIsTheDist checks that dir holds the dist file for file, plus |
| +// fiesta release's stamp at the top, and nothing else: no leftovers. |
| func assertIsTheDist(t *testing.T, dir string, d map[string][]byte) { |
| t.Helper() |
| for name, want := range d { |
| @@ -49,6 +73,9 @@ func assertIsTheDist(t *testing.T, dir string, d map[string][]byte) { |
| t.Errorf("%s on disk differs from the dist", name) |
| } |
| } |
| + if b, err := os.ReadFile(filepath.Join(dir, stampName)); err != nil || string(b) != stampBody("fiesta release") { |
| + t.Errorf("the stamp is not fiesta release's: %q, %v", b, err) |
| + } |
| var extra []string |
| err := filepath.WalkDir(dir, func(p string, e os.DirEntry, err error) error { |
| if err != nil { |
| @@ -58,6 +85,9 @@ func assertIsTheDist(t *testing.T, dir string, d map[string][]byte) { |
| return nil |
| } |
| rel, _ := filepath.Rel(dir, p) |
| + if rel == stampName { |
| + return nil |
| + } |
| if _, ok := d[filepath.ToSlash(rel)]; !ok { |
| extra = append(extra, rel) |
| } |
| @@ -71,10 +101,39 @@ func assertIsTheDist(t *testing.T, dir string, d map[string][]byte) { |
| } |
| } |
| +// age turns the release in dir into an older candidate: one built from |
| +// a tree without NOTICE, with the sums to match. It is still |
| +// self-consistent and still stamped, and it is not what the tree |
| +// builds now, so NOTICE coming back is the sign of a rebuild and its |
| +// absence the sign of a refusal that touched nothing. |
| +func age(t *testing.T, dir string, d map[string][]byte) { |
| + t.Helper() |
| + if err := os.Remove(filepath.Join(dir, "NOTICE")); err != nil { |
| + t.Fatal(err) |
| + } |
| + var kept []string |
| + for _, line := range strings.SplitAfter(string(d["SHA256SUMS"]), "\n") { |
| + if line != "" && !strings.HasSuffix(line, " NOTICE\n") { |
| + kept = append(kept, line) |
| + } |
| + } |
| + if err := os.WriteFile(filepath.Join(dir, "SHA256SUMS"), []byte(strings.Join(kept, "")), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| +} |
| + |
| +// untouched asserts an aged candidate was not rebuilt. |
| +func untouched(t *testing.T, dir, what string) { |
| + t.Helper() |
| + if _, err := os.Stat(filepath.Join(dir, "NOTICE")); !os.IsNotExist(err) { |
| + t.Errorf("after %s the candidate was rebuilt: NOTICE is back", what) |
| + } |
| +} |
| + |
| // TestReleaseRefusesToOverwrite: a version other than fiesta.Version |
| // is refused and nothing is created; the first run writes the dist, |
| -// file for file, with no stamp; a second run finds the directory and |
| -// refuses it, leaving it exactly as it was. |
| +// file for file, with the command's stamp; a second run finds the |
| +// directory and refuses it, leaving it exactly as it was. |
| func TestReleaseRefusesToOverwrite(t *testing.T) { |
| releases := filepath.Join(t.TempDir(), "releases") |
| err := fiestaRelease(t, releases, "9.9.9", false) |
| @@ -133,9 +192,7 @@ func git(t *testing.T, dir string, args ...string) string { |
| // machine. |
| func scratchRepo(t *testing.T) string { |
| t.Helper() |
| - if _, err := exec.LookPath("git"); err != nil { |
| - t.Skip("git is not installed") |
| - } |
| + requireGit(t) |
| root := t.TempDir() |
| repo := filepath.Join(root, "repo") |
| origin := filepath.Join(root, "origin.git") |
| @@ -155,72 +212,45 @@ func scratchRepo(t *testing.T) string { |
| } |
| // TestACandidateMayBeRebuiltUntilItIsTagged: -candidate replaces an |
| -// existing release directory whole, but only while it is a release |
| -// fiesta wrote and no tag vX.Y.Z names it, locally or on origin. A |
| -// stray file or a link inside, a local tag, or a tag on origin alone, |
| -// each refuses the rebuild and leaves the directory as it was. |
| +// existing release directory whole, but only while it carries fiesta |
| +// release's own stamp and no tag vX.Y.Z names it, locally or on |
| +// origin. A stray file in a directory the command owns is replaced |
| +// with the rest; a link inside, another command's stamp, a missing |
| +// stamp, a local tag, or a tag on origin alone, each refuses the |
| +// rebuild and leaves the directory as it was. |
| func TestACandidateMayBeRebuiltUntilItIsTagged(t *testing.T) { |
| repo := scratchRepo(t) |
| releases := filepath.Join(repo, "releases") |
| dir := filepath.Join(releases, fiesta.Version) |
| d := dist(t) |
| tag := "v" + fiesta.Version |
| + stamp := filepath.Join(dir, stampName) |
| if err := fiestaRelease(t, releases, fiesta.Version, true); err != nil { |
| t.Fatalf("fiesta release -candidate into an absent directory: %v", err) |
| } |
| assertIsTheDist(t, dir, d) |
| - // An older candidate, built from a tree without NOTICE: still a |
| - // self-consistent release, and not what the tree builds now. |
| - older := func() { |
| - t.Helper() |
| - if err := os.Remove(filepath.Join(dir, "NOTICE")); err != nil { |
| - t.Fatal(err) |
| - } |
| - var kept []string |
| - for _, line := range strings.SplitAfter(string(d["SHA256SUMS"]), "\n") { |
| - if line != "" && !strings.HasSuffix(line, " NOTICE\n") { |
| - kept = append(kept, line) |
| - } |
| - } |
| - if err := os.WriteFile(filepath.Join(dir, "SHA256SUMS"), []byte(strings.Join(kept, "")), 0o644); err != nil { |
| - t.Fatal(err) |
| - } |
| - } |
| - untouched := func(what string) { |
| - t.Helper() |
| - if _, err := os.Stat(filepath.Join(dir, "NOTICE")); !os.IsNotExist(err) { |
| - t.Errorf("after %s the candidate was rebuilt: NOTICE is back", what) |
| - } |
| - } |
| - |
| - older() |
| + age(t, dir, d) |
| if err := fiestaRelease(t, releases, fiesta.Version, true); err != nil { |
| t.Fatalf("fiesta release -candidate refused an untagged candidate: %v", err) |
| } |
| assertIsTheDist(t, dir, d) |
| - older() |
| + age(t, dir, d) |
| stray := filepath.Join(dir, "notes.txt") |
| if err := os.WriteFile(stray, []byte("mine"), 0o644); err != nil { |
| t.Fatal(err) |
| } |
| - err := fiestaRelease(t, releases, fiesta.Version, true) |
| - if err == nil { |
| - t.Fatal("fiesta release -candidate replaced a directory holding a file its SHA256SUMS does not list") |
| - } |
| - if !strings.Contains(err.Error(), "notes.txt") { |
| - t.Errorf("the refusal does not name the stray file: %v", err) |
| - } |
| - untouched("a stray file") |
| - if b, err := os.ReadFile(stray); err != nil || string(b) != "mine" { |
| - t.Errorf("the refused directory was touched: notes.txt is %q, %v", b, err) |
| + if err := fiestaRelease(t, releases, fiesta.Version, true); err != nil { |
| + t.Fatalf("fiesta release -candidate refused its own directory over a stray file: %v", err) |
| } |
| - if err := os.Remove(stray); err != nil { |
| - t.Fatal(err) |
| + if _, err := os.Stat(stray); !os.IsNotExist(err) { |
| + t.Errorf("the stray file survived a rebuild of a directory the command owns: %v", err) |
| } |
| + assertIsTheDist(t, dir, d) |
| + age(t, dir, d) |
| link := filepath.Join(dir, "LICENSE") |
| if err := os.Remove(link); err != nil { |
| t.Fatal(err) |
| @@ -228,14 +258,14 @@ func TestACandidateMayBeRebuiltUntilItIsTagged(t *testing.T) { |
| if err := os.Symlink(filepath.Join(repo, "README"), link); err != nil { |
| t.Fatal(err) |
| } |
| - err = fiestaRelease(t, releases, fiesta.Version, true) |
| + err := fiestaRelease(t, releases, fiesta.Version, true) |
| if err == nil { |
| t.Fatal("fiesta release -candidate replaced a directory holding a symbolic link") |
| } |
| if !strings.Contains(err.Error(), "LICENSE") { |
| t.Errorf("the refusal does not name the link: %v", err) |
| } |
| - untouched("a link") |
| + untouched(t, dir, "a link") |
| if err := os.Remove(link); err != nil { |
| t.Fatal(err) |
| } |
| @@ -243,6 +273,33 @@ func TestACandidateMayBeRebuiltUntilItIsTagged(t *testing.T) { |
| t.Fatal(err) |
| } |
| + if err := os.WriteFile(stamp, []byte(stampBody("fiesta build")), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + err = fiestaRelease(t, releases, fiesta.Version, true) |
| + if err == nil { |
| + t.Fatal("fiesta release -candidate replaced a directory stamped by fiesta build") |
| + } |
| + if !strings.Contains(err.Error(), "fiesta build") { |
| + t.Errorf("the refusal does not name the command that owns the directory: %v", err) |
| + } |
| + untouched(t, dir, "another command's stamp") |
| + |
| + if err := os.Remove(stamp); err != nil { |
| + t.Fatal(err) |
| + } |
| + err = fiestaRelease(t, releases, fiesta.Version, true) |
| + if err == nil { |
| + t.Fatal("fiesta release -candidate replaced a directory with no stamp") |
| + } |
| + if !strings.Contains(err.Error(), stampName) { |
| + t.Errorf("the refusal does not name the missing stamp: %v", err) |
| + } |
| + untouched(t, dir, "a missing stamp") |
| + if err := os.WriteFile(stamp, []byte(stampBody("fiesta release")), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| git(t, repo, "tag", tag) |
| err = fiestaRelease(t, releases, fiesta.Version, true) |
| if err == nil { |
| @@ -251,7 +308,7 @@ func TestACandidateMayBeRebuiltUntilItIsTagged(t *testing.T) { |
| if !strings.Contains(err.Error(), tag) { |
| t.Errorf("the refusal does not name the tag: %v", err) |
| } |
| - untouched("a local tag") |
| + untouched(t, dir, "a local tag") |
| git(t, repo, "push", "-q", "origin", tag) |
| git(t, repo, "tag", "-d", tag) |
| @@ -265,18 +322,43 @@ func TestACandidateMayBeRebuiltUntilItIsTagged(t *testing.T) { |
| if !strings.Contains(err.Error(), tag) { |
| t.Errorf("the refusal does not name the tag: %v", err) |
| } |
| - untouched("a tag on origin") |
| + untouched(t, dir, "a tag on origin") |
| err = fiestaRelease(t, releases, fiesta.Version, false) |
| if err == nil { |
| t.Fatal("fiesta release without -candidate replaced an existing directory") |
| } |
| - untouched("a run without -candidate") |
| + untouched(t, dir, "a run without -candidate") |
| +} |
| + |
| +// TestAnUnreachableOriginRefusesACandidate: when git cannot say |
| +// whether origin has the tag, the answer is a refusal, not "no". The |
| +// origin is pointed at a path that does not exist, and the aged |
| +// candidate stays as it was. |
| +func TestAnUnreachableOriginRefusesACandidate(t *testing.T) { |
| + repo := scratchRepo(t) |
| + releases := filepath.Join(repo, "releases") |
| + dir := filepath.Join(releases, fiesta.Version) |
| + d := dist(t) |
| + if err := fiestaRelease(t, releases, fiesta.Version, true); err != nil { |
| + t.Fatal(err) |
| + } |
| + age(t, dir, d) |
| + git(t, repo, "remote", "set-url", "origin", filepath.Join(t.TempDir(), "gone.git")) |
| + err := fiestaRelease(t, releases, fiesta.Version, true) |
| + if err == nil { |
| + t.Fatalf("fiesta release -candidate replaced %s without being able to ask origin about its tag", dir) |
| + } |
| + if !strings.Contains(err.Error(), "could not tell") || !strings.Contains(err.Error(), "v"+fiesta.Version) { |
| + t.Errorf("the refusal does not say the tag could not be checked: %v", err) |
| + } |
| + untouched(t, dir, "an unreachable origin") |
| } |
| // TestSiteWritesWhatBuildReturns: `fiesta site -releases R -pages P |
| -// -out DIR` writes site.Build's tree into DIR, file for file, and |
| -// stamps DIR as its own. |
| +// -out DIR` writes site.Build's tree into DIR/public, file for file, |
| +// stamps DIR itself as its own, and leaves nothing hidden in the tree |
| +// to ship. |
| func TestSiteWritesWhatBuildReturns(t *testing.T) { |
| root := t.TempDir() |
| releases := filepath.Join(root, "releases") |
| @@ -303,8 +385,9 @@ func TestSiteWritesWhatBuildReturns(t *testing.T) { |
| if len(want) == 0 { |
| t.Fatal("the site is empty") |
| } |
| + shipped := filepath.Join(out, shipDir) |
| for name, data := range want { |
| - got, err := os.ReadFile(filepath.Join(out, filepath.FromSlash(name))) |
| + got, err := os.ReadFile(filepath.Join(shipped, filepath.FromSlash(name))) |
| if err != nil { |
| t.Errorf("the site did not write %s: %v", name, err) |
| continue |
| @@ -318,19 +401,28 @@ func TestSiteWritesWhatBuildReturns(t *testing.T) { |
| t.Errorf("_redirects is %q", got) |
| } |
| if b, err := os.ReadFile(filepath.Join(out, stampName)); err != nil || string(b) != stampBody("fiesta site") { |
| - t.Errorf("the stamp is not fiesta site's: %q, %v", b, err) |
| + t.Errorf("the stamp is not fiesta site's, or is not in -out itself: %q, %v", b, err) |
| } |
| var written int |
| - err = filepath.WalkDir(out, func(p string, e os.DirEntry, err error) error { |
| - if err == nil && !e.IsDir() && filepath.Base(p) != stampName { |
| + err = filepath.WalkDir(shipped, func(p string, e os.DirEntry, err error) error { |
| + if err != nil { |
| + return err |
| + } |
| + if p != shipped && strings.HasPrefix(e.Name(), ".") { |
| + t.Errorf("%s is hidden and sits in the tree to ship", p) |
| + } |
| + if !e.IsDir() { |
| written++ |
| } |
| - return err |
| + return nil |
| }) |
| if err != nil { |
| t.Fatal(err) |
| } |
| if written != len(want) { |
| - t.Errorf("%s holds %d files besides the stamp, the build has %d", out, written, len(want)) |
| + t.Errorf("%s holds %d files, the build has %d", shipped, written, len(want)) |
| + } |
| + if entries, err := os.ReadDir(out); err != nil || len(entries) != 2 { |
| + t.Errorf("-out holds %d entries, want the stamp and %s: %v", len(entries), shipDir, err) |
| } |
| } |
diff --git a/internal/site/site.go b/internal/site/site.go| index fc0ad5e..c579f84 100644 |
| --- a/internal/site/site.go |
| +++ b/internal/site/site.go |
| @@ -28,6 +28,25 @@ const sumsName = "SHA256SUMS" |
| // file Build writes that is neither a release nor a page. |
| const redirectsName = "_redirects" |
| +// StampName is the file cmd/fiesta leaves at the top of a directory it |
| +// owns, fiesta release's release directories included, so a later run |
| +// knows the directory is its own to empty. It is committed with the |
| +// release and is the one hidden file Release expects: it is skipped, |
| +// never published, and never listed in SHA256SUMS. cmd/fiesta's claim |
| +// uses this constant, so the two cannot disagree. |
| +const StampName = ".fiesta" |
| + |
| +// hidden reports whether any segment of a slash path starts with a |
| +// dot: an editor's swap file, a Finder's .DS_Store, a stamp. |
| +func hidden(p string) bool { |
| + for _, seg := range strings.Split(p, "/") { |
| + if strings.HasPrefix(seg, ".") { |
| + return true |
| + } |
| + } |
| + return false |
| +} |
| + |
| // strictVersion is X.Y.Z and nothing else: no v, no prerelease, no |
| // build metadata, no leading zero. A release directory is named this |
| // way or it is not a release. |
| @@ -134,7 +153,18 @@ func Build(releases fs.FS, pages fs.FS) (map[string][]byte, error) { |
| if err != nil { |
| return fmt.Errorf("reading pages: %w", err) |
| } |
| - if p == "." || d.IsDir() { |
| + if p == "." { |
| + return nil |
| + } |
| + // A hidden page is refused rather than skipped: pages/ is |
| + // committed, so a dotfile there is a mistake someone should |
| + // see, and skipping it quietly would also hide a stamp that |
| + // a wrong -pages pointed at. Directories too, so an empty |
| + // .git or .cache under pages/ is named, not passed over. |
| + if hidden(p) { |
| + return fmt.Errorf("page %s is hidden, and a hidden file is never published; remove it from pages", p) |
| + } |
| + if d.IsDir() { |
| return nil |
| } |
| // Checked per file rather than per directory, so the refusal |
| @@ -174,9 +204,19 @@ func Build(releases fs.FS, pages fs.FS) (map[string][]byte, error) { |
| // because it is not what Fiesta wrote. |
| // |
| // Only regular files are a release's. A link, or anything else, is |
| -// refused by name: fiesta release writes none, and reading through one |
| -// would check bytes that live somewhere else. |
| +// refused by name, the sums file included: fiesta release writes none, |
| +// and reading through one would check bytes that live somewhere else. |
| +// |
| +// The one file not held to the sums is StampName at the top of the |
| +// release, fiesta release's own ownership mark, which is skipped and |
| +// never returned. Any other hidden file, and a stamp anywhere deeper, |
| +// is unlisted and refused like any stray. |
| func Release(dir fs.FS) (map[string][]byte, error) { |
| + if fi, err := fs.Lstat(dir, sumsName); err != nil { |
| + return nil, fmt.Errorf("%s: %w", sumsName, err) |
| + } else if !fi.Mode().IsRegular() { |
| + return nil, fmt.Errorf("%s is not a regular file, so it is not a release file", sumsName) |
| + } |
| sums, err := fs.ReadFile(dir, sumsName) |
| if err != nil { |
| return nil, fmt.Errorf("%s: %w", sumsName, err) |
| @@ -197,12 +237,15 @@ func Release(dir fs.FS) (map[string][]byte, error) { |
| if p == "." || d.IsDir() { |
| return nil |
| } |
| - if p == sumsName { |
| + if p == StampName { |
| return nil |
| } |
| if !d.Type().IsRegular() { |
| return fmt.Errorf("%s is not a regular file, so it is not a release file", p) |
| } |
| + if p == sumsName { |
| + return nil |
| + } |
| want, ok := listed[p] |
| if !ok { |
| return fmt.Errorf("%s is not listed in %s", p, sumsName) |
diff --git a/internal/site/site_test.go b/internal/site/site_test.go| index af8ab0b..9f54493 100644 |
| --- a/internal/site/site_test.go |
| +++ b/internal/site/site_test.go |
| @@ -5,6 +5,8 @@ import ( |
| "crypto/sha256" |
| "encoding/hex" |
| "io/fs" |
| + "os" |
| + "path/filepath" |
| "sort" |
| "strings" |
| "testing" |
| @@ -279,3 +281,106 @@ func TestWhatBuildWritesPassesTheCheck(t *testing.T) { |
| t.Errorf("Release read %d files, the dist has %d", len(got), len(dist)) |
| } |
| } |
| + |
| +// TestTheStampIsSkippedAndNeverPublished: the one hidden file a |
| +// committed release carries, fiesta release's stamp at its top, is |
| +// skipped and never copied to the site. A stamp deeper down, or any |
| +// other hidden file, is a stray: unlisted, so refused by name. |
| +func TestTheStampIsSkippedAndNeverPublished(t *testing.T) { |
| + releases := releasesOf("1.0.0") |
| + stamp := "1.0.0/" + site.StampName |
| + releases[stamp] = &fstest.MapFile{Data: []byte("This directory is written by fiesta release, which EMPTIES it on every\nrun.\n")} |
| + out, err := site.Build(releases, pagesOf(map[string]string{"index.html": "<h1>Fiesta</h1>\n"})) |
| + if err != nil { |
| + t.Fatalf("Build refused a release carrying its stamp: %v", err) |
| + } |
| + for name := range out { |
| + if strings.Contains(name, site.StampName) { |
| + t.Errorf("the stamp was published as %s", name) |
| + } |
| + } |
| + if want := len(releases) - 1 + 1 + 1; len(out) != want { |
| + t.Errorf("the site holds %d files, want %d (the release without its stamp, the page and _redirects)", len(out), want) |
| + } |
| + for _, stray := range []string{"1.0.0/i18n/" + site.StampName, "1.0.0/.DS_Store"} { |
| + t.Run(stray, func(t *testing.T) { |
| + releases := releasesOf("1.0.0") |
| + releases[stray] = &fstest.MapFile{Data: []byte("x")} |
| + _, err := site.Build(releases, pagesOf(nil)) |
| + if err == nil { |
| + t.Fatalf("Build published a release holding %s", stray) |
| + } |
| + if !strings.Contains(err.Error(), stray) { |
| + t.Errorf("the error does not name %s: %v", stray, err) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// TestHiddenPagesAreRefused: a hidden file or directory anywhere under |
| +// pages is refused, never published and never skipped: an editor's |
| +// swap file, a Finder's .DS_Store, a stamp, a .well-known directory, |
| +// an empty .cache. The error names the hidden entry itself, so a |
| +// hidden directory is named rather than the first file inside it. |
| +func TestHiddenPagesAreRefused(t *testing.T) { |
| + for page, named := range map[string]string{ |
| + ".DS_Store": ".DS_Store", |
| + site.StampName: site.StampName, |
| + "docs/.index.html.swp": "docs/.index.html.swp", |
| + ".well-known/security.txt": ".well-known", |
| + } { |
| + t.Run(page, func(t *testing.T) { |
| + pages := pagesOf(map[string]string{"index.html": "<h1>Fiesta</h1>\n", page: "hidden"}) |
| + _, err := site.Build(releasesOf("1.0.0"), pages) |
| + if err == nil { |
| + t.Fatalf("Build published the hidden page %s", page) |
| + } |
| + if !strings.Contains(err.Error(), named) { |
| + t.Errorf("the error does not name %s: %v", named, err) |
| + } |
| + }) |
| + } |
| + t.Run("an empty hidden directory", func(t *testing.T) { |
| + pages := pagesOf(map[string]string{"index.html": "<h1>Fiesta</h1>\n"}) |
| + pages[".cache"] = &fstest.MapFile{Mode: fs.ModeDir | 0o755} |
| + _, err := site.Build(releasesOf("1.0.0"), pages) |
| + if err == nil || !strings.Contains(err.Error(), ".cache") { |
| + t.Errorf("Build did not refuse the hidden directory .cache by name: %v", err) |
| + } |
| + }) |
| +} |
| + |
| +// TestALinkedSumsFileIsRefused: SHA256SUMS that is a symbolic link is |
| +// not read through. Only a regular file is a release's, the sums file |
| +// included, so the link is refused by name before anything is checked |
| +// against what it points at. |
| +func TestALinkedSumsFileIsRefused(t *testing.T) { |
| + dir := t.TempDir() |
| + files := release("1.0.0") |
| + elsewhere := filepath.Join(t.TempDir(), "sums") |
| + if err := os.WriteFile(elsewhere, files["SHA256SUMS"], 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + for name, data := range files { |
| + if name == "SHA256SUMS" { |
| + continue |
| + } |
| + p := filepath.Join(dir, filepath.FromSlash(name)) |
| + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(p, data, 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if err := os.Symlink(elsewhere, filepath.Join(dir, "SHA256SUMS")); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, err := site.Release(os.DirFS(dir)) |
| + if err == nil { |
| + t.Fatal("Release read SHA256SUMS through a symbolic link") |
| + } |
| + if !strings.Contains(err.Error(), "SHA256SUMS") || !strings.Contains(err.Error(), "regular") { |
| + t.Errorf("the error does not refuse SHA256SUMS as not a regular file: %v", err) |
| + } |
| +} |