feat(depot): select a depot tree on disk with --out DIR

`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 <sha> <dest> --out DIR` form parses its flags at all — Go's flag package stops at the first positional. Closes #66
2026-07-28 10:15:14 +02:00
parent ed6945d308
commit 6bbc2c3a83
4 changed files with 154 additions and 33 deletions
@@ -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    <sha> <dest>       --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    <sha> <dest>      --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 <sha> <dest> --target cdn|bunny|local")
+		fmt.Fprintln(stderr, "depot get: usage: depot get <sha> <dest> --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)
@@ -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)
@@ -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 <sha> <dest> --target cdn|bunny|local"},
+			{Name: "get", Summary: "fetch one blob with sha re-verify", Usage: "crucible depot get <sha> <dest> --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,