From 6bbc2c3a838d9223e29a09b07777ccf0bc244b0a Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Tue, 28 Jul 2026 10:15:14 +0200 Subject: [PATCH] feat(depot): select a depot tree on disk with --out DIR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `depot status` and `depot get` chose a local depot tree with `--target local`, resolving the root from `DEPOT_DIR`. That split one decision over two flags and left `--target local` valid with no directory anywhere. Adopt the nwsync convention instead: `--out DIR` names the tree on disk in the same breath as choosing it, and `--target` now names remote backends only. The two are mutually exclusive. `--target local` stays as a deprecated alias for `--out $DEPOT_DIR` so existing callers keep working, and `status --source` is now ignored with a warning instead of silently accepted. Also split positionals off by hand in `depot get`, so the documented `depot get --out DIR` form parses its flags at all — Go's flag package stops at the first positional. Closes #66 --- docs/command-surface.md | 11 +++- internal/depot/run.go | 74 +++++++++++++++++---------- internal/depot/run_test.go | 96 +++++++++++++++++++++++++++++++++++ internal/dispatch/dispatch.go | 6 +-- 4 files changed, 154 insertions(+), 33 deletions(-) diff --git a/docs/command-surface.md b/docs/command-surface.md index b51c99e..641cb93 100644 --- a/docs/command-surface.md +++ b/docs/command-surface.md @@ -29,9 +29,15 @@ aliases. | `assets` | `fix-mdl` | Lowercase `.mdl` names and rewrite model identity to match. | | `assets` | `check-dupes` | Report runtime-name (basename) collisions across dirs. | | `assets` | `clean-dupes` | Delete clean-tree files whose basename collides with primary. | +| `depot` | `status` | Report referenced-vs-present drift against a backend. | +| `depot` | `push` | Upload referenced-but-absent blobs from a local depot. | +| `depot` | `verify` | Existence sweep plus sampled download re-hash. | +| `depot` | `get` | Fetch one blob with sha re-verify. | +| `depot` | `pull` | Incremental verified pull of every referenced blob. | -`crucible-depot` remains registered but unwired. It fails closed with exit `70` -and never emits placeholder artifacts. +`depot status` and `depot get` pick their backend either with `--out DIR`, a +depot tree on disk, or with `--target bunny|cdn`, a remote backend. The two +flags are mutually exclusive. ## Hidden compatibility aliases @@ -49,6 +55,7 @@ but omitted from routine help and the interactive menu: | `topdata` | `build-top-package` | `build-top-package` | | `topdata` | `compare-topdata` | `compare-topdata` | | `topdata` | `convert-topdata` | `convert-topdata` | +| `depot` | `--target local` | `--out $DEPOT_DIR` on `status`/`get` | | `wiki` | `build-wiki` | `build-wiki` | | `wiki` | `deploy-wiki` | `deploy-wiki` | diff --git a/internal/depot/run.go b/internal/depot/run.go index a46b1be..d2ff33d 100644 --- a/internal/depot/run.go +++ b/internal/depot/run.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "sort" + "strings" "sync" "time" ) @@ -51,13 +52,15 @@ func Run(args []string, stdout, stderr io.Writer, getenv func(string) string) in func printRunUsage(w io.Writer) { fmt.Fprint(w, `usage: - depot status [--manifests DIR] [--source LOCAL_DEPOT] --target bunny|cdn|local - depot push [--manifests DIR] --source LOCAL_DEPOT --target bunny - depot verify [--manifests DIR] --target bunny|cdn [--sample N] - depot get --target cdn|bunny|local - depot pull [--manifests DIR] --dest DIR --target cdn|bunny + depot status [--manifests DIR] --target bunny|cdn | --out DIR + depot push [--manifests DIR] --source LOCAL_DEPOT --target bunny + depot verify [--manifests DIR] --target bunny|cdn [--sample N] + depot get --target cdn|bunny | --out DIR + depot pull [--manifests DIR] --dest DIR --target cdn|bunny ---target local uses the DEPOT_DIR environment variable as the local root. +--out DIR reads and writes a depot tree on disk at DIR instead of a remote +backend. --target names a remote backend only; the two are mutually exclusive. +--target local is a deprecated alias for --out $DEPOT_DIR. `) } @@ -65,16 +68,27 @@ func printRunUsage(w io.Writer) { // than internal/backend failures (exit 70). var errUsage = errors.New("usage error") -// resolveBackend builds the named backend, resolving "local" against DEPOT_DIR. -func resolveBackend(target string, getenv func(string) string, cfg Config) (Backend, error) { - root := "" - if target == "local" { - root = getenv("DEPOT_DIR") +// resolveBackend picks the backend for a command that can work either against a +// depot tree on disk (--out DIR) or a remote backend (--target bunny|cdn). +// "--target local" stays as a deprecated alias for --out $DEPOT_DIR so older +// callers keep working. +func resolveBackend(out, target string, getenv func(string) string, cfg Config) (Backend, error) { + switch { + case out != "" && target != "": + return nil, fmt.Errorf("--out and --target are mutually exclusive: %w", errUsage) + case out != "": + return NewBackend("local", out, cfg) + case target == "local": + root := getenv("DEPOT_DIR") if root == "" { - return nil, fmt.Errorf("--target local requires DEPOT_DIR to be set: %w", errUsage) + return nil, fmt.Errorf("--target local requires DEPOT_DIR to be set (prefer --out DIR): %w", errUsage) } + return NewBackend("local", root, cfg) + case target == "": + return nil, fmt.Errorf("--out DIR or --target bunny|cdn is required: %w", errUsage) + default: + return NewBackend(target, "", cfg) } - return NewBackend(target, root, cfg) } // backendErrExit maps a resolveBackend error onto the exit contract. @@ -119,18 +133,18 @@ func runStatus(args []string, stdout, stderr io.Writer, getenv func(string) stri fs := flag.NewFlagSet("status", flag.ContinueOnError) fs.SetOutput(stderr) manifests := fs.String("manifests", "assets", "directory of *.yml manifests") - _ = fs.String("source", "", "local depot root (unused for status target=local; see DEPOT_DIR)") - target := fs.String("target", "", "bunny|cdn|local") + source := fs.String("source", "", "deprecated and ignored (use --out DIR for a depot tree on disk)") + out := fs.String("out", "", "depot tree on disk to read instead of a remote backend") + target := fs.String("target", "", "bunny|cdn") if err := fs.Parse(args); err != nil { return exitUsage } - if *target == "" { - fmt.Fprintln(stderr, "depot status: --target is required") - return exitUsage + if *source != "" { + fmt.Fprintln(stderr, "depot status: --source is ignored; use --out DIR") } cfg := LoadConfig(getenv) - backend, err := resolveBackend(*target, getenv, cfg) + backend, err := resolveBackend(*out, *target, getenv, cfg) if err != nil { fmt.Fprintln(stderr, "depot status:", err) return backendErrExit(err) @@ -309,13 +323,21 @@ func runVerify(args []string, stdout, stderr io.Writer, getenv func(string) stri func runGet(args []string, stdout, stderr io.Writer, getenv func(string) string) int { fs := flag.NewFlagSet("get", flag.ContinueOnError) fs.SetOutput(stderr) - target := fs.String("target", "", "cdn|bunny|local") - if err := fs.Parse(args); err != nil { + out := fs.String("out", "", "depot tree on disk to read instead of a remote backend") + target := fs.String("target", "", "cdn|bunny") + // Positionals come first in the documented usage, and Go's flag package + // stops parsing at the first one, so split them off by hand. + split := 0 + for split < len(args) && !strings.HasPrefix(args[split], "-") { + split++ + } + positional := args[:split] + if err := fs.Parse(args[split:]); err != nil { return exitUsage } - positional := fs.Args() + positional = append(positional, fs.Args()...) if len(positional) != 2 { - fmt.Fprintln(stderr, "depot get: usage: depot get --target cdn|bunny|local") + fmt.Fprintln(stderr, "depot get: usage: depot get --target cdn|bunny | --out DIR") return exitUsage } sha, dest := positional[0], positional[1] @@ -323,13 +345,9 @@ func runGet(args []string, stdout, stderr io.Writer, getenv func(string) string) fmt.Fprintf(stderr, "depot get: invalid sha %q\n", sha) return exitUsage } - if *target == "" { - fmt.Fprintln(stderr, "depot get: --target is required") - return exitUsage - } cfg := LoadConfig(getenv) - backend, err := resolveBackend(*target, getenv, cfg) + backend, err := resolveBackend(*out, *target, getenv, cfg) if err != nil { fmt.Fprintln(stderr, "depot get:", err) return backendErrExit(err) diff --git a/internal/depot/run_test.go b/internal/depot/run_test.go index a2bcbda..a4b2152 100644 --- a/internal/depot/run_test.go +++ b/internal/depot/run_test.go @@ -54,6 +54,91 @@ func TestRunUsage(t *testing.T) { }) } +func TestOutFlag(t *testing.T) { + t.Run("status --out reads the given tree", func(t *testing.T) { + sha := shaOf("out-blob") + manifests := t.TempDir() + writeManifest(t, manifests, sha, 8) + depotDir := t.TempDir() + writeBlob(t, depotDir, sha, "out-blob") + + var out, errb bytes.Buffer + code := Run([]string{"status", "--manifests", manifests, "--out", depotDir}, &out, &errb, testGetenv(nil)) + if code != 0 { + t.Fatalf("expected 0, got %d (stdout=%s stderr=%s)", code, out.String(), errb.String()) + } + if !bytesContains(out.String(), "present=1") { + t.Fatalf("expected present=1, got %s", out.String()) + } + }) + + t.Run("get --out fetches from the given tree", func(t *testing.T) { + sha := shaOf("get-blob") + depotDir := t.TempDir() + writeBlob(t, depotDir, sha, "get-blob") + dest := filepath.Join(t.TempDir(), "fetched") + + var out, errb bytes.Buffer + code := Run([]string{"get", sha, dest, "--out", depotDir}, &out, &errb, testGetenv(nil)) + if code != 0 { + t.Fatalf("expected 0, got %d (stderr=%s)", code, errb.String()) + } + got, err := os.ReadFile(dest) + if err != nil { + t.Fatal(err) + } + if string(got) != "get-blob" { + t.Fatalf("got %q", got) + } + }) + + t.Run("--out with any --target is rejected", func(t *testing.T) { + for _, target := range []string{"bunny", "cdn", "local"} { + var out, errb bytes.Buffer + code := Run([]string{"status", "--manifests", t.TempDir(), "--out", t.TempDir(), "--target", target}, &out, &errb, testGetenv(nil)) + if code != 64 { + t.Fatalf("--target %s: expected 64, got %d (stderr=%s)", target, code, errb.String()) + } + } + }) + + t.Run("get accepts flags before the positionals", func(t *testing.T) { + sha := shaOf("flags-first-blob") + depotDir := t.TempDir() + writeBlob(t, depotDir, sha, "flags-first-blob") + dest := filepath.Join(t.TempDir(), "fetched") + + var out, errb bytes.Buffer + code := Run([]string{"get", "--out", depotDir, sha, dest}, &out, &errb, testGetenv(nil)) + if code != 0 { + t.Fatalf("expected 0, got %d (stderr=%s)", code, errb.String()) + } + }) + + t.Run("neither --out nor --target is rejected", func(t *testing.T) { + var out, errb bytes.Buffer + code := Run([]string{"status", "--manifests", t.TempDir()}, &out, &errb, testGetenv(nil)) + if code != 64 { + t.Fatalf("expected 64, got %d (stderr=%s)", code, errb.String()) + } + }) + + t.Run("--target local still works as a hidden alias", func(t *testing.T) { + sha := shaOf("alias-blob") + manifests := t.TempDir() + writeManifest(t, manifests, sha, 11) + depotDir := t.TempDir() + writeBlob(t, depotDir, sha, "alias-blob") + + var out, errb bytes.Buffer + code := Run([]string{"status", "--manifests", manifests, "--target", "local"}, &out, &errb, + testGetenv(map[string]string{"DEPOT_DIR": depotDir})) + if code != 0 { + t.Fatalf("expected 0, got %d (stdout=%s stderr=%s)", code, out.String(), errb.String()) + } + }) +} + func TestGetInvalidSHA(t *testing.T) { var out, errb bytes.Buffer dir := t.TempDir() @@ -172,6 +257,17 @@ func bytesContains(s, substr string) bool { return bytes.Contains([]byte(s), []byte(substr)) } +func writeBlob(t *testing.T, root, sha, content string) { + t.Helper() + path := filepath.Join(root, BlobKey(sha)) + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatal(err) + } +} + func writeManifest(t *testing.T, dir, sha string, size int64) { t.Helper() content := fmt.Sprintf("assets:\n - path: foo\n sha256: %s\n size: %d\n", sha, size) diff --git a/internal/dispatch/dispatch.go b/internal/dispatch/dispatch.go index 75bfeb0..2c963c2 100644 --- a/internal/dispatch/dispatch.go +++ b/internal/dispatch/dispatch.go @@ -94,12 +94,12 @@ var Registry = []Builder{ { Name: "depot", Bin: "crucible-depot", - Summary: "content-addressed asset depot (local/cdn/bunny)", + Summary: "content-addressed asset depot (disk/cdn/bunny)", Commands: []Command{ - {Name: "status", Summary: "report referenced-vs-present drift against a backend", Usage: "crucible depot status [--manifests DIR] [--source DIR] --target bunny|cdn|local"}, + {Name: "status", Summary: "report referenced-vs-present drift against a backend", Usage: "crucible depot status [--manifests DIR] --target bunny|cdn | --out DIR"}, {Name: "push", Summary: "upload referenced-but-absent blobs from a local depot", Usage: "crucible depot push [--manifests DIR] --source DIR --target bunny"}, {Name: "verify", Summary: "existence sweep plus sampled download re-hash", Usage: "crucible depot verify [--manifests DIR] --target bunny|cdn [--sample N]"}, - {Name: "get", Summary: "fetch one blob with sha re-verify", Usage: "crucible depot get --target cdn|bunny|local"}, + {Name: "get", Summary: "fetch one blob with sha re-verify", Usage: "crucible depot get --target cdn|bunny | --out DIR"}, {Name: "pull", Summary: "incremental verified pull of every referenced blob", Usage: "crucible depot pull [--manifests DIR] --dest DIR --target cdn|bunny"}, }, Wired: true,