diff --git a/internal/appsettings/appsettings.go b/internal/appsettings/appsettings.go index 7567023e..16c7400b 100644 --- a/internal/appsettings/appsettings.go +++ b/internal/appsettings/appsettings.go @@ -793,6 +793,18 @@ func DetectCloudCandidate(home string) string { return DetectDriveCandidate(home) } +// DefaultMirrorPath is the folder `dot sync` mirrors into when a workspace +// store sets no target: "/work" on the same cloud as the detected +// backup root, or ~/gdrive-workspace/work when no cloud root qualifies. +func DefaultMirrorPath(home string) string { + if cloud := DetectCloudCandidate(home); cloud != "" { + // cloud is "/secrets/dotfiles-backup"; the mirror lives at + // "/work", two levels up from the secrets marker. + return filepath.Join(filepath.Dir(filepath.Dir(cloud)), "work") + } + return filepath.Join(home, "gdrive-workspace", "work") +} + // detectDropboxCandidate scans for a Dropbox cloud root (under // ~/Library/CloudStorage, then the ~/Dropbox symlink in home) that has a // "secrets" subdirectory marker. CloudStorage is scanned first so the diff --git a/internal/cli/sync_cli_test.go b/internal/cli/sync_cli_test.go index 3bbaf679..aa777dad 100644 --- a/internal/cli/sync_cli_test.go +++ b/internal/cli/sync_cli_test.go @@ -39,7 +39,7 @@ func newSyncCLIFixture(t *testing.T) *syncCLIFixture { if err := os.MkdirAll(filepath.Dir(stateFile), 0o755); err != nil { t.Fatal(err) } - state := "modules:\n gsync:\n local_path: " + local + "\n mirror_path: " + mirror + "\n" + state := "modules:\n gsync:\n local_path: " + local + "\n" if err := os.WriteFile(stateFile, []byte(state), 0o644); err != nil { t.Fatal(err) } @@ -414,33 +414,34 @@ func TestSyncMirrorCLI_PrintAndDryRunAreReadOnly(t *testing.T) { } } -func TestSyncMirrorCLI_HonorsHomeOverride(t *testing.T) { +func TestSyncTargetCLI_HonorsHomeOverride(t *testing.T) { f := newSyncCLIFixture(t) other := t.TempDir() // a different user's home if err := os.MkdirAll(filepath.Join(other, ".config", "dotfiles"), 0o755); err != nil { t.Fatal(err) } - // ~ in the path must expand against --home, and global state must be - // written for that home — not the current HOME. + // ~ in the path must expand against --home, and the store that records it + // must be that home's store — not the current user's. out, errOut, err := runDotForTest("sync", "target", "~/Dropbox/work", "--home", other) if err != nil { - t.Fatalf("gsync mirror --home: %v\nstdout=%s\nstderr=%s", err, out, errOut) + t.Fatalf("sync target --home: %v\nstdout=%s\nstderr=%s", err, out, errOut) } wantMirror := filepath.Join(other, "Dropbox", "work") if !strings.Contains(out, wantMirror) { t.Errorf("~ should expand against --home %q:\n%s", wantMirror, out) } - otherCfg, err := os.ReadFile(filepath.Join(other, ".config", "dotfiles", "config.yaml")) + otherStore := filepath.Join(other, "workspace", "work", ".dotfiles", "sync", "config.yaml") + otherCfg, err := os.ReadFile(otherStore) if err != nil { - t.Fatalf("read --home state: %v", err) + t.Fatalf("read --home store: %v", err) } - if !strings.Contains(string(otherCfg), "mirror_path: "+wantMirror) { - t.Errorf("--home global state missing mirror_path:\n%s", otherCfg) + if !strings.Contains(string(otherCfg), "target: local:"+wantMirror) { + t.Errorf("--home store missing the target:\n%s", otherCfg) } - // The current user's state must NOT have been written with this mirror. - if cur, _ := os.ReadFile(filepath.Join(f.home, ".config", "dotfiles", "config.yaml")); strings.Contains(string(cur), wantMirror) { - t.Errorf("current-user state wrongly written under --home:\n%s", cur) + // The current user's store must NOT have been written with this target. + if cur, _ := os.ReadFile(filepath.Join(f.local, ".dotfiles", "sync", "config.yaml")); strings.Contains(string(cur), wantMirror) { + t.Errorf("current-user store wrongly written under --home:\n%s", cur) } } @@ -580,7 +581,7 @@ func TestSyncPushRefusesLegacyOnlyStore(t *testing.T) { f := newSyncCLIFixture(t) legacy := filepath.Join(f.local, ".dotfiles", "gdrive-sync") writeCLITestFile(t, filepath.Join(legacy, "baseline.manifest"), "docs/a.md\t1\n") - writeCLITestFile(t, filepath.Join(legacy, "config.yaml"), "mirror_path: "+f.mirror+"\n") + writeCLITestFile(t, filepath.Join(legacy, "config.yaml"), "target: local:"+f.mirror+"\n") writeCLITestFile(t, filepath.Join(f.local, "docs", "a.md"), "local edit\n") writeCLITestFile(t, filepath.Join(f.mirror, "docs", "a.md"), "mirror copy\n") before := snapshotTree(t, f.home) diff --git a/internal/cli/sync_configure.go b/internal/cli/sync_configure.go index f55a1c62..1c0a9a9e 100644 --- a/internal/cli/sync_configure.go +++ b/internal/cli/sync_configure.go @@ -59,7 +59,6 @@ func runSyncConfigure(cmd *cobra.Command, _ []string) error { } if target.Kind == syncer.TargetLocal { target.Path = appsettings.ExpandHome(target.Path, homeFor(cmd)) - local.MirrorPath = target.Path } local.Target = target.String() } diff --git a/internal/cli/sync_target_cmd.go b/internal/cli/sync_target_cmd.go index 5c6666d8..e178a4ff 100644 --- a/internal/cli/sync_target_cmd.go +++ b/internal/cli/sync_target_cmd.go @@ -68,39 +68,19 @@ func runSyncTarget(cmd *cobra.Command, args []string) error { p.KV("Target", bs.Config.Target.String()) return nil } - p.Line("[dry-run] would set target to %s (local config%s)", target.String(), - map[bool]string{true: " + global state", false: ""}[target.Kind == syncer.TargetLocal]) + p.Line("[dry-run] would set target to %s (local config)", target.String()) return nil } - homeOverride, _ := cmd.Flags().GetString("home") - - // Local config governs the current workspace (global state is ignored - // once it exists), so write it for immediate effect — but only for the - // current user. Under --home the admin isn't in the target user's - // workspace, so the local config (always current-workspace) doesn't - // apply; only the home-aware global state below is meaningful there. - if homeOverride == "" { - bs, err := syncer.Bootstrap(syncBootstrapOptions(cmd, false)) - if err != nil { - return err - } - if err := syncer.SetLocalTarget(bs.Config, target); err != nil { - return err - } + // The store belongs to the workspace and the workspace belongs to the home: + // Bootstrap resolved both against --home, so an admin running this for + // another user writes that user's store, not the invoking user's. + bs, err := syncer.Bootstrap(syncBootstrapOptions(cmd, false)) + if err != nil { + return err } - - // Global state, home-aware: local targets are inherited by future - // workspaces. SSH targets stay workspace-local. - if target.Kind == syncer.TargetLocal { - state, err := loadStateForCmd(cmd) - if err != nil { - return fmt.Errorf("load global state: %w", err) - } - state.Modules.Gsync.MirrorPath = target.Path - if err := persistUserState(cmd, state); err != nil { - p.Warn("could not update global state: %v", err) - } + if err := syncer.SetLocalTarget(bs.Config, target); err != nil { + return err } p.Line("%s", ui.StyleSuccess.Render("✓ sync target set")) diff --git a/internal/config/state.go b/internal/config/state.go index d2262fa3..88f6485c 100644 --- a/internal/config/state.go +++ b/internal/config/state.go @@ -10,6 +10,8 @@ import ( "strings" "time" + "github.com/STAIxBWLB/dotfiles/internal/appsettings" + "github.com/STAIxBWLB/dotfiles/internal/fileutil" "github.com/STAIxBWLB/dotfiles/internal/sliceutil" "github.com/STAIxBWLB/dotfiles/internal/tunnel" "gopkg.in/yaml.v3" @@ -177,6 +179,72 @@ func sliceHasRetiredCask(tokens []string) bool { return false } +// retiredStateKeys are the state keys no release reads any more, with what +// replaces each one. `modules.gdrive_sync.mirror_path` lost its last reader +// when ResolveConfig stopped falling back to it (#394 row 10): the destination +// is the workspace store's `target` now, so the value cannot be carried over as +// a setting. It is carried as evidence while it would still move a +// destination (retiredMirrorPathStillMoves), and dropped once it does not. +var retiredStateKeys = map[string]string{ + retiredMirrorPathStateKey: "`dot sync` uses the workspace store's `target` (set it with `dot sync target`)", +} + +const retiredMirrorPathStateKey = "modules.gdrive_sync.mirror_path" + +// detectRetiredKeys records which retired keys the raw state still sets, so +// the next save can say what it dropped. It reads the raw bytes the way +// applyLegacyKeys does -- the field these keys named is gone, so a decode +// cannot see them -- but it runs for every schema version: #373's stamped-file +// rule governs keys whose values are carried, and these have no value left to +// carry. +func detectRetiredKeys(state *UserState, raw []byte) { + var legacy struct { + Modules struct { + Gsync struct { + MirrorPath string `yaml:"mirror_path"` + } `yaml:"gdrive_sync"` + } `yaml:"modules"` + } + if err := yaml.Unmarshal(raw, &legacy); err != nil { + return + } + if legacy.Modules.Gsync.MirrorPath != "" { + state.retiredKeys = append(state.retiredKeys, retiredMirrorPathStateKey) + state.retiredMirrorPath = legacy.Modules.Gsync.MirrorPath + } +} + +// RetiredMirrorPath returns the value this state file still sets for the +// retired `modules.gdrive_sync.mirror_path` key, or "". The sync engine reads +// it only to refuse a silent destination change on a store that never set +// `target`: the key does not route anything any more (#394 row 10). +func (s *UserState) RetiredMirrorPath() string { + if s == nil { + return "" + } + return s.retiredMirrorPath +} + +// retiredMirrorPathStillMoves reports whether this state's retired value has to +// survive a save. A workspace store that sets no target resolves the detected +// cloud default, and `dot sync` refuses exactly that while the retired value +// names a different folder (syncer.refuseRetiredDestinationChange reads it from +// here when the store has none of its own). Dropping it on an unrelated save +// (`dot apply`, `dot reconfigure`) would take that evidence away, and the next +// push would mirror, and delete, in a tree nobody picked. Once the value names +// the folder resolution would use, it decides nothing and goes. +func retiredMirrorPathStillMoves(state *UserState, home string) bool { + value := strings.TrimSpace(state.retiredMirrorPath) + if value == "" { + return false + } + if home == "" { + // No home, so no default to compare with: keep the evidence. + return true + } + return !fileutil.SameDirectory(appsettings.ExpandHome(value, home), appsettings.DefaultMirrorPath(home)) +} + // warnForwardSchema reports that a state file was written by a newer dot. It // does not fail the load: an additively-forward file still decodes, and an // incompatibly-forward one gets this warning before its decode error so the @@ -210,6 +278,18 @@ type UserState struct { // so yaml.v3 cannot see it: the flag can never be written to disk and // needs no tag. saveStateAt reads it to print one note per converted write. carriedLegacy []string + + // retiredKeys names the keys the file this state was read from still + // carried after they stopped being read, in detection order. Unexported for + // the same reason as carriedLegacy: the save that drops them is the only + // place that may speak of them, and it prints one note, then clears this. + retiredKeys []string + + // retiredMirrorPath is the value `modules.gdrive_sync.mirror_path` held in + // that same file, kept for the one reader that refuses a destination change + // (RetiredMirrorPath), and written back by a save until it names the folder + // resolution would use. + retiredMirrorPath string } // UserModulesState holds module opt-in/config from user state. @@ -329,21 +409,21 @@ type UserRsyncState struct { // UserGsyncState holds local↔local rsync mirror config from user state. // // It holds the machine-wide defaults `dot sync` seeds a new workspace store -// with (mirror path, max delete, shared excludes) and LocalPath, which locates -// the workspace. Per-workspace settings live in /.dotfiles/sync/. +// with (max delete, shared excludes) and LocalPath, which locates the +// workspace. The destination is not one of them: it lives in the workspace +// store's `target`. Per-workspace settings live in /.dotfiles/sync/. // // Paused defaults to true on a fresh state so the user explicitly opts in by // flipping it to false via `resume`. type UserGsyncState struct { - LocalPath string `yaml:"local_path,omitempty"` // primary tree, default ~/workspace/work - MirrorPath string `yaml:"mirror_path,omitempty"` // mirror tree, default ~/gdrive-workspace/work + LocalPath string `yaml:"local_path,omitempty"` // primary tree, default ~/workspace/work LastPull time.Time `yaml:"last_pull,omitempty"` LastPush time.Time `yaml:"last_push,omitempty"` ConflictDir string `yaml:"conflict_dir,omitempty"` // default /.sync-conflicts Paused bool `yaml:"paused,omitempty"` // gates pull/push/sync; cleared by `resume` MaxDelete int `yaml:"max_delete,omitempty"` // safety cap for push --delete-after, default 1000 Interval int `yaml:"interval,omitempty"` // auto-sync interval in seconds (launchd/systemd), default 300 - SharedExcludes []string `yaml:"shared_excludes,omitempty"` // operator-curated owned-but-shared-out folders, relative to mirror_path + SharedExcludes []string `yaml:"shared_excludes,omitempty"` // operator-curated owned-but-shared-out folders, relative to the sync target } // UserTunnelState holds the Cloudflare Tunnel values managed by `dot tunnel`. @@ -492,12 +572,12 @@ func (s *UserState) Validate() error { } } for _, p := range s.Modules.Gsync.SharedExcludes { - // Paths must be relative to mirror_path. Absolute paths and parent + // Paths must be relative to the sync target. Absolute paths and parent // escapes would let the manual list reach outside the mirror tree // and exclude unrelated content (or be portable across machines // in misleading ways). if strings.HasPrefix(p, "/") { - return fmt.Errorf("gdrive_sync.shared_excludes entry %q must be relative to mirror_path (no leading /)", p) + return fmt.Errorf("gdrive_sync.shared_excludes entry %q must be relative to the sync target (no leading /)", p) } for _, seg := range strings.Split(p, "/") { if seg == ".." { @@ -626,6 +706,12 @@ func readStateFile(path, noun string) (*UserState, int, error) { if version == 0 { applyLegacyKeys(&state, data) } + // Retired keys are the exception to that gate: #373 is about keys whose + // values are migrated to a canonical form, and there is no canonical form + // for these. The value is kept only as evidence for the destination refusal + // in internal/syncer, and the save decides whether to keep or drop it, + // stamped or not. + detectRetiredKeys(&state, data) rewriteRetiredValues(&state) return &state, version, nil } @@ -694,6 +780,14 @@ func MarshalState(state *UserState) ([]byte, error) { // On POSIX filesystems, rename is atomic, so partial writes cannot corrupt // the existing config file. func saveStateAt(path string, state *UserState) error { + home, _ := os.UserHomeDir() + return saveStateAtForHome(path, state, home) +} + +// saveStateAtForHome is saveStateAt for an explicit home: `dot ... --home` +// writes another user's state, and the retired destination is judged against +// that home's detected default, never the invoking user's. +func saveStateAtForHome(path string, state *UserState, home string) error { if err := state.Validate(); err != nil { return fmt.Errorf("refusing to save invalid state: %w", err) } @@ -710,6 +804,15 @@ func saveStateAt(path string, state *UserState) error { if err != nil { return err } + // A retired value that would still move a sync destination is evidence, not + // a setting: it stays in the file until it names the folder resolution would + // use, so an unrelated save cannot quietly hand the next push a new one. + carryRetired := state.retiredMirrorPath != "" && retiredMirrorPathStillMoves(state, home) + if carryRetired { + if data, err = carryRetiredMirrorPath(data, strings.TrimSpace(state.retiredMirrorPath)); err != nil { + return err + } + } // Write to temp file in the same directory (same filesystem → rename is atomic) tmpFile, err := os.CreateTemp(dir, ".config.yaml.*.tmp") @@ -750,9 +853,73 @@ func saveStateAt(path string, state *UserState) error { fmt.Fprintf(os.Stderr, "note: migrated legacy state keys to their canonical form: %s\n", strings.Join(state.carriedLegacy, ", ")) } + // A retired key is the other half of the same notice: nothing replaces its + // value, so the note has to name the command that sets the replacement. + // Printed once and cleared, so a run that saves twice says it once. A key + // this save carried was not dropped, and says nothing. + if len(state.retiredKeys) > 0 { + for _, key := range state.retiredKeys { + if carryRetired && key == retiredMirrorPathStateKey { + continue + } + fmt.Fprintf(os.Stderr, "note: dropped the retired `%s` key: %s\n", key, retiredStateKeys[key]) + } + state.retiredKeys = nil + } + if !carryRetired { + // The value is gone from the file, so the memory of it goes too: a + // later save in the same run would otherwise write it back. + state.retiredMirrorPath = "" + } return nil } +// carryRetiredMirrorPath writes the retired key back into the marshaled state. +// The struct has no field for it any more, so this is a document edit: find +// (or add) modules.gdrive_sync and append the key. The value is carried as +// evidence for the destination refusal; nothing reads it as a setting. +func carryRetiredMirrorPath(data []byte, value string) ([]byte, error) { + var doc yaml.Node + if err := yaml.Unmarshal(data, &doc); err != nil { + return nil, fmt.Errorf("carrying the retired mirror_path: %w", err) + } + if len(doc.Content) == 0 || doc.Content[0].Kind != yaml.MappingNode { + return nil, fmt.Errorf("carrying the retired mirror_path: the state is not a mapping") + } + node := doc.Content[0] + for _, section := range []string{"modules", "gdrive_sync"} { + node = yamlMappingChild(node, section) + } + node.Content = append(node.Content, + &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: "mirror_path"}, + &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: value}, + ) + out, err := yaml.Marshal(&doc) + if err != nil { + return nil, fmt.Errorf("carrying the retired mirror_path: %w", err) + } + return out, nil +} + +// yamlMappingChild returns the mapping's value node for key, adding an empty +// mapping when the key is missing or holds something else. The state struct +// always writes both sections, so this is the defensive path. +func yamlMappingChild(mapping *yaml.Node, key string) *yaml.Node { + for i := 0; i+1 < len(mapping.Content); i += 2 { + if mapping.Content[i].Value != key { + continue + } + if mapping.Content[i+1].Kind != yaml.MappingNode { + mapping.Content[i+1] = &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"} + } + return mapping.Content[i+1] + } + child := &yaml.Node{Kind: yaml.MappingNode, Tag: "!!map"} + mapping.Content = append(mapping.Content, + &yaml.Node{Kind: yaml.ScalarNode, Tag: "!!str", Value: key}, child) + return child +} + // schemaForceEnv overrides the downgrade refusal below. It ships with the name // DEBT-02 gives it. Three naming conventions are in play in this tree and none // of them is this one -- the single existing DOT_* variable is an internal @@ -834,7 +1001,7 @@ func LoadStateForHome(homeDir string) (*UserState, error) { // SaveStateForHome writes user state to a specific home directory atomically. func SaveStateForHome(homeDir string, state *UserState) error { - return saveStateAt(StatePathForHome(homeDir), state) + return saveStateAtForHome(StatePathForHome(homeDir), state, homeDir) } // ApplyStateToConfig merges user state into a Config loaded from a profile. diff --git a/internal/config/state_retired_test.go b/internal/config/state_retired_test.go index 42fed8f9..179d62e2 100644 --- a/internal/config/state_retired_test.go +++ b/internal/config/state_retired_test.go @@ -5,6 +5,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/STAIxBWLB/dotfiles/internal/appsettings" ) // #373 AC1: each retired value loads, is rewritten on the next save and @@ -65,3 +67,119 @@ func TestLoadState_StampedFileIgnoresStrayLegacyKey(t *testing.T) { t.Fatalf("stray legacy keys were overlaid: ai=%v warp=%v carried=%v", loaded.Modules.AI.Enabled, loaded.Modules.Warp, loaded.carriedLegacy) } } + +// #394 row 10: modules.gdrive_sync.mirror_path lost its last reader, so a file +// that still sets it has nothing left to resolve. It does not lose the value +// yet: while it names a folder other than the detected default, a workspace +// store without `target` would silently redirect, and internal/syncer refuses +// exactly that on the strength of this value. Any unrelated save (`dot apply`, +// `dot reconfigure`) must carry it through, silently. +func TestLoadState_CarriesTheRetiredMirrorPathWhileItStillMoves(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + path := filepath.Join(t.TempDir(), "config.yaml") + body := "schema_version: 4\nname: Test\nprofile: full\nmodules:\n gdrive_sync:\n mirror_path: /legacy/mirror\n" + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + loaded, err := loadStateAt(path) + if err != nil { + t.Fatalf("load: %v", err) + } + if len(loaded.retiredKeys) != 1 || loaded.retiredKeys[0] != retiredMirrorPathStateKey { + t.Fatalf("retiredKeys = %v, want [%s]", loaded.retiredKeys, retiredMirrorPathStateKey) + } + if got := loaded.RetiredMirrorPath(); got != "/legacy/mirror" { + t.Errorf("RetiredMirrorPath = %q, want /legacy/mirror", got) + } + + out := captureStderr(t, func() { + if err := saveStateAt(path, loaded); err != nil { + t.Fatalf("save: %v", err) + } + }) + if strings.Contains(out, "retired") { + t.Errorf("a carried key was announced as dropped: %q", out) + } + + data, _ := os.ReadFile(path) + if !strings.Contains(string(data), "mirror_path: /legacy/mirror") { + t.Fatalf("the save dropped a value that still moves a destination:\n%s", data) + } + + // Carrying is idempotent: the flag is spent, the value stays. + if again := captureStderr(t, func() { + if err := saveStateAt(path, loaded); err != nil { + t.Fatalf("second save: %v", err) + } + }); strings.Contains(again, "retired") { + t.Errorf("second save repeated the note: %q", again) + } + reloaded, err := loadStateAt(path) + if err != nil { + t.Fatal(err) + } + if got := reloaded.RetiredMirrorPath(); got != "/legacy/mirror" { + t.Errorf("reloaded value = %q, want /legacy/mirror", got) + } +} + +// The other half of the same rule: once the retired value names the folder +// resolution would use, it decides nothing, so the save drops it and says so +// once. The stamped version is the point: #373's gate governs keys whose values +// are migrated, and this one has no canonical form, so the note has to fire for +// a canonical file too. +func TestLoadState_DropsTheRetiredMirrorPathKeyOnceItIsMoot(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + moot := filepath.Join(home, "gdrive-workspace", "work") + if want := appsettings.DefaultMirrorPath(home); moot != want { + t.Fatalf("test default drifted: %q vs %q", moot, want) + } + path := filepath.Join(t.TempDir(), "config.yaml") + body := "schema_version: 4\nname: Test\nprofile: full\nmodules:\n gdrive_sync:\n mirror_path: " + moot + "\n" + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + loaded, err := loadStateAt(path) + if err != nil { + t.Fatalf("load: %v", err) + } + + out := captureStderr(t, func() { + if err := saveStateAt(path, loaded); err != nil { + t.Fatalf("save: %v", err) + } + }) + if got := strings.Count(out, "note: dropped the retired `"+retiredMirrorPathStateKey+"` key"); got != 1 { + t.Fatalf("want exactly one retired-key note, got %d in %q", got, out) + } + if !strings.Contains(out, "`dot sync target`") { + t.Errorf("the note does not name the command that sets the replacement: %q", out) + } + + data, _ := os.ReadFile(path) + if strings.Contains(string(data), "mirror_path") { + t.Fatalf("saved file still carries the retired key:\n%s", data) + } + + // One note per run: the save that announced the key clears the flag. + if again := captureStderr(t, func() { + if err := saveStateAt(path, loaded); err != nil { + t.Fatalf("second save: %v", err) + } + }); strings.Contains(again, "retired") { + t.Errorf("second save repeated the note: %q", again) + } + + reloaded, err := loadStateAt(path) + if err != nil { + t.Fatal(err) + } + if len(reloaded.retiredKeys) != 0 { + t.Errorf("rewritten file still reports retired keys: %v", reloaded.retiredKeys) + } + if got := reloaded.RetiredMirrorPath(); got != "" { + t.Errorf("rewritten file still reports a retired value: %q", got) + } +} diff --git a/internal/fileutil/samedir.go b/internal/fileutil/samedir.go new file mode 100644 index 00000000..15dcb484 --- /dev/null +++ b/internal/fileutil/samedir.go @@ -0,0 +1,26 @@ +package fileutil + +import ( + "path/filepath" + "strings" +) + +// SameDirectory reports whether two paths name the same directory, following +// symlinks on both sides: a cloud folder is routinely reachable under a +// symlinked spelling (~/Dropbox -> ~/Library/CloudStorage/Dropbox), and a +// textual compare would read one tree as two. A path that does not resolve (it +// may not exist yet) makes the comparison textual, which leans towards +// reporting a difference rather than guessing that two paths are the same. +func SameDirectory(a, b string) bool { + a, b = strings.TrimSpace(a), strings.TrimSpace(b) + if a == "" || b == "" { + return false + } + ca, cb := filepath.Clean(a), filepath.Clean(b) + ra, errA := filepath.EvalSymlinks(ca) + rb, errB := filepath.EvalSymlinks(cb) + if errA == nil && errB == nil { + return ra == rb + } + return ca == cb +} diff --git a/internal/fileutil/samedir_test.go b/internal/fileutil/samedir_test.go new file mode 100644 index 00000000..f40bc49f --- /dev/null +++ b/internal/fileutil/samedir_test.go @@ -0,0 +1,41 @@ +package fileutil + +import ( + "os" + "path/filepath" + "testing" +) + +func TestSameDirectory(t *testing.T) { + tmp := t.TempDir() + real := filepath.Join(tmp, "Dropbox") + if err := os.MkdirAll(real, 0o755); err != nil { + t.Fatal(err) + } + link := filepath.Join(tmp, "Box") + if err := os.Symlink(real, link); err != nil { + t.Fatal(err) + } + + for _, tc := range []struct { + name string + a, b string + want bool + }{ + {"identical", real, real, true}, + {"trailing separator", real, real + string(filepath.Separator), true}, + {"unclean spelling", real, real + string(filepath.Separator) + ".", true}, + {"through a symlink", link, real, true}, + {"different folders", real, filepath.Join(tmp, "other"), false}, + {"one side missing", real, "", false}, + {"both missing", "", "", false}, + {"missing but spelled the same", "/no/such/tree", "/no/such/tree", true}, + {"missing and spelled apart", "/no/such/tree", "/no/such/../such/sibling", false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := SameDirectory(tc.a, tc.b); got != tc.want { + t.Errorf("SameDirectory(%q, %q) = %v, want %v", tc.a, tc.b, got, tc.want) + } + }) + } +} diff --git a/internal/syncer/fetch_test.go b/internal/syncer/fetch_test.go index 5fcd0bfd..efcd416a 100644 --- a/internal/syncer/fetch_test.go +++ b/internal/syncer/fetch_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/STAIxBWLB/dotfiles/internal/config" "github.com/STAIxBWLB/dotfiles/internal/exec" ) @@ -295,3 +296,35 @@ func TestPullDirect_WithoutBackupIgnoresTheConflictsDir(t *testing.T) { t.Errorf("PullDirect refused although it passes no --backup: %v", err) } } + +// Fetch copies mirror → workspace, so a destination that moved unasked would +// refill the workspace from a tree nobody picked and overwrite what is there. +// It refuses on the same terms as push and pull (#394 row 10) and writes +// nothing. +func TestFetch_RefusesARetiredDestination(t *testing.T) { + f := newIntakeFixture(t) + t.Setenv("HOME", t.TempDir()) + if err := os.WriteFile(f.cfg.LocalPaths.ConfigFile, []byte("mirror_path: /legacy/mirror\n"), 0o644); err != nil { + t.Fatal(err) + } + f.writeMirror("notes/a.md", "mirror copy") + f.writeLocal("notes/a.md", "workspace copy") + + state := &config.UserState{} + state.Modules.Gsync.LocalPath = f.local + cfg, err := ResolveConfigReadOnly(state) + if err != nil { + t.Fatalf("ResolveConfigReadOnly: %v", err) + } + if !cfg.MirrorIsDefault { + t.Fatalf("the store sets no target, so the destination must be the default: %q", cfg.MirrorPath) + } + + if _, err := Fetch(context.Background(), f.runner, cfg, []string{"notes/a.md"}, false); err == nil || + !strings.Contains(err.Error(), "dot sync target /legacy/mirror") { + t.Fatalf("Fetch = %v, want a refusal naming the fix", err) + } + if body, _ := os.ReadFile(filepath.Join(f.local, "notes/a.md")); string(body) != "workspace copy" { + t.Errorf("the workspace copy was overwritten: %q", body) + } +} diff --git a/internal/syncer/intake.go b/internal/syncer/intake.go index cc1bb49d..da5ba584 100644 --- a/internal/syncer/intake.go +++ b/internal/syncer/intake.go @@ -70,6 +70,9 @@ func Intake(ctx context.Context, runner *exec.Runner, cfg *Config, opts IntakeOp if err := refuseSharedDriveMirror(cfg); err != nil { return nil, err } + if err := refuseRetiredDestinationChange(cfg); err != nil { + return nil, err + } paths := cfg.LocalPaths mirror := strings.TrimRight(cfg.MirrorPath, "/") diff --git a/internal/syncer/local_store.go b/internal/syncer/local_store.go index 2282dfe9..e5ee0a07 100644 --- a/internal/syncer/local_store.go +++ b/internal/syncer/local_store.go @@ -11,6 +11,7 @@ import ( "gopkg.in/yaml.v3" "github.com/STAIxBWLB/dotfiles/internal/config" + "github.com/STAIxBWLB/dotfiles/internal/fileutil" "github.com/STAIxBWLB/dotfiles/internal/template" ) @@ -135,7 +136,8 @@ func (p PropagationPolicy) String() string { // settings. Persists to /config.yaml. type LocalConfig struct { // Target is the canonical destination spec ("local:~/Dropbox/work" or - // "ssh:user@host:path"). When empty, MirrorPath (legacy) applies. + // "ssh:user@host:path"). When empty, ResolveConfig uses the detected cloud + // default; it never consults a mirror path from anywhere else. Target string `yaml:"target,omitempty"` // Owner is the hostname allowed to push this profile. A mirror with two // pushers corrupts: both carry different baselines and delete propagation @@ -164,7 +166,6 @@ type LocalConfig struct { // the opposite: "either machine can continue the same work" includes the // uncommitted state inside submodules, which Git by definition has not seen. IncludeSubmodules bool `yaml:"include_submodules,omitempty"` - MirrorPath string `yaml:"mirror_path,omitempty"` FilterMode FilterMode `yaml:"filter_mode,omitempty"` Propagation PropagationPolicy `yaml:"propagation"` MaxDelete int `yaml:"max_delete,omitempty"` @@ -184,6 +185,14 @@ type LocalConfig struct { // ("~/" is the peer's home). Unset, the newest release found there wins; // set it to use a dev build (#176). RemoteDot string `yaml:"remote_dot,omitempty"` + // retired holds the keys the file this config was read from still set after + // they stopped being read, with the values they held. Unexported, so it can + // never be written to disk and needs no tag; SaveLocalConfig prints one note + // per retired key and then clears it, which keeps a store that is written + // twice in one run quiet the second time. The values are not dead weight: + // a store with no `target` is guarded by the `mirror_path` it still names + // (refuseRetiredDestinationChange). + retired map[string]string } // LocalState holds non-config runtime telemetry — the sticky timestamps @@ -234,6 +243,11 @@ type LocalPaths struct { LogFile string WorkspaceRoot string // the local sync tree itself (parent of .dotfiles) WorkspaceIgnore string // /.gitignore (workspace-level) + // Home is the home this layout was resolved for (resolveConfig sets it). + // A save needs it to tell a retired destination from the detected default; + // empty means the caller built the layout without one, so a save cannot + // prove a retired value is moot and carries it. + Home string } // ResolveLocalPaths returns the canonical layout for a given local @@ -471,6 +485,7 @@ func LoadLocalConfig(paths *LocalPaths) (*LocalConfig, bool, error) { if err := yaml.Unmarshal(data, &cfg); err != nil { return nil, false, fmt.Errorf("parsing %s: %w", paths.ConfigFile, err) } + cfg.retired = retiredLocalConfigValues(data) if err := cfg.Propagation.Validate(); err != nil { // Fall back to defaults when on-disk policy is unusable rather than blocking. fmt.Fprintf(os.Stderr, "warning: %s has invalid propagation (%v); using defaults\n", paths.ConfigFile, err) @@ -515,10 +530,103 @@ func SaveLocalConfig(paths *LocalPaths, cfg *LocalConfig) error { if err != nil { return fmt.Errorf("marshaling local config: %w", err) } - if data, err = keepUnknownConfigKeys(paths.ConfigFile, data); err != nil { + carried := retiredKeysStillInPlay(cfg, paths.Home) + if data, err = keepUnknownConfigKeys(paths.ConfigFile, data, carried); err != nil { return err } - return atomicWrite(paths.ConfigFile, data) + if err := atomicWrite(paths.ConfigFile, data); err != nil { + return err + } + announceRetiredLocalConfigKeys(paths.ConfigFile, cfg, carried) + return nil +} + +// retiredKeysStillInPlay names the retired keys a save has to carry instead of +// dropping. The only evidence `dot sync` has for refusing a destination that +// would move unasked is the value this store still sets (see +// refuseRetiredDestinationChange), so a save for an unrelated reason +// (`dot sync pause`, `dot sync owner`, `dot sync setup`, `dot sync shared`) +// must not take it away: after one of those, the store would resolve the +// detected default and the next push would mirror, and delete, there. +// +// A store that sets `target` chose its destination itself, and one whose value +// names the folder resolution would land on decides nothing either, so both +// drop. Without a home the save cannot prove the match, and carrying is the +// safe side of that doubt. +func retiredKeysStillInPlay(cfg *LocalConfig, home string) map[string]bool { + stored := strings.TrimSpace(cfg.retired[retiredMirrorPathKey]) + if stored == "" || strings.TrimSpace(cfg.Target) != "" { + return nil + } + if home != "" && fileutil.SameDirectory(expandHome(stored, home), defaultMirrorPath(home)) { + return nil + } + return map[string]bool{retiredMirrorPathKey: true} +} + +// announceRetiredLocalConfigKeys reports the keys this save dropped, once per +// loaded config: a key that no release reads any more disappears here, and +// saying so is the difference between a migration and a silent edit of the +// user's file. Kept at the write choke point so no read path can print it. A +// carried key stays in cfg.retired, so a later save in the same run carries it +// again rather than announcing a drop that did not happen. +func announceRetiredLocalConfigKeys(path string, cfg *LocalConfig, carried map[string]bool) { + if len(cfg.retired) == 0 { + return + } + keys := make([]string, 0, len(cfg.retired)) + for _, key := range retiredLocalConfigKeys { + if _, ok := cfg.retired[key]; !ok || carried[key] { + continue + } + keys = append(keys, key) + delete(cfg.retired, key) + } + if len(keys) == 0 { + return + } + fmt.Fprintf(os.Stderr, "note: dropped the retired `%s` key from %s; `dot sync` uses this store's `target` (set it with `dot sync target`)\n", + strings.Join(keys, "`, `"), path) +} + +// retiredMirrorPathKey is the one retired key whose value the sync engine still +// looks at, and only to refuse: a store that sets it and no `target` would +// otherwise move its destination to the detected default without being asked. +const retiredMirrorPathKey = "mirror_path" + +// retiredLocalConfigKeys are the config keys this release no longer reads +// (#394 row 10 removed mirror_path in favor of target). A save drops them +// rather than carrying them the way keepUnknownConfigKeys carries a key +// LocalConfig does not own, and LoadLocalConfig records them so the drop is +// announced. +var retiredLocalConfigKeys = []string{retiredMirrorPathKey} + +func isRetiredLocalConfigKey(key string) bool { + for _, retired := range retiredLocalConfigKeys { + if key == retired { + return true + } + } + return false +} + +// retiredLocalConfigValues returns the retired keys the given config bytes +// still set, with the values they hold. +func retiredLocalConfigValues(data []byte) map[string]string { + var probe map[string]any + if err := yaml.Unmarshal(data, &probe); err != nil { + return nil + } + var found map[string]string + for _, key := range retiredLocalConfigKeys { + if value, ok := probe[key]; ok && value != nil && value != "" { + if found == nil { + found = map[string]string{} + } + found[key] = fmt.Sprint(value) + } + } + return found } // keepUnknownConfigKeys carries over the top-level keys of the config on @@ -530,8 +638,12 @@ func SaveLocalConfig(paths *LocalPaths, cfg *LocalConfig) error { // aliases resolved (an anchor on a key the struct rewrote is gone), and // the result must load as a config, or the save refuses. // +// retiredCarried names the retired keys this save must carry rather than drop +// (retiredKeysStillInPlay): they are the store's only evidence for refusing a +// destination change, so they follow the same path as any unknown key. +// // ponytail: known ceiling. See docs/CEILINGS.md (unknown config keys, top level only). -func keepUnknownConfigKeys(path string, data []byte) ([]byte, error) { +func keepUnknownConfigKeys(path string, data []byte, retiredCarried map[string]bool) ([]byte, error) { old, err := os.ReadFile(path) if err != nil { return data, nil @@ -554,7 +666,7 @@ func keepUnknownConfigKeys(path string, data []byte) ([]byte, error) { if err != nil { return nil, fmt.Errorf("refusing to save %s: %w", path, err) } - if known[key.Value] || key.ShortTag() == "!!merge" { + if known[key.Value] || (isRetiredLocalConfigKey(key.Value) && !retiredCarried[key.Value]) || key.ShortTag() == "!!merge" { continue } value, err := resolveYAMLAliases(was.Content[0].Content[i+1]) @@ -671,7 +783,6 @@ func UpdateLocalState(paths *LocalPaths, mutate func(*LocalState)) error { func localConfigFromGlobal(globalState *config.UserState) *LocalConfig { gs := globalState.Modules.Gsync return &LocalConfig{ - MirrorPath: gs.MirrorPath, FilterMode: DefaultFilterMode(), Propagation: DefaultPropagationPolicy(), MaxDelete: gs.MaxDelete, @@ -686,8 +797,9 @@ func localConfigFromGlobal(globalState *config.UserState) *LocalConfig { // SeedFromGlobal writes /config.yaml for a workspace that has // none, from the machine-wide defaults in `state.modules.gdrive_sync` -// (mirror path, max delete, shared excludes), which `dot sync target` -// keeps for future workspaces. Returns the new LocalConfig. +// (max delete, shared excludes). The destination is not among them: a store +// starts with an empty `target`, so ResolveConfig uses the detected cloud +// default until `dot sync target` sets one. Returns the new LocalConfig. func SeedFromGlobal(globalState *config.UserState, paths *LocalPaths) (*LocalConfig, error) { cfg := localConfigFromGlobal(globalState) if err := EnsureLocalLayout(paths); err != nil { diff --git a/internal/syncer/local_store_test.go b/internal/syncer/local_store_test.go index eea212e5..b2463be3 100644 --- a/internal/syncer/local_store_test.go +++ b/internal/syncer/local_store_test.go @@ -1,6 +1,7 @@ package syncer import ( + "io" "os" "path/filepath" "strings" @@ -204,7 +205,7 @@ func TestLocalConfig_RoundTrip(t *testing.T) { paths := ResolveLocalPaths(tmp) original := &LocalConfig{ - MirrorPath: "/x/mirror", + Target: "local:/x/mirror", FilterMode: FilterModeExclude, Propagation: PropagationPolicy{Create: true, Update: true, Delete: true}, MaxDelete: 500, @@ -226,7 +227,7 @@ func TestLocalConfig_RoundTrip(t *testing.T) { if !ok { t.Fatal("LoadLocalConfig reported missing after save") } - if got.MirrorPath != original.MirrorPath || + if got.Target != original.Target || got.FilterMode != original.FilterMode || got.Propagation != original.Propagation || got.MaxDelete != original.MaxDelete || @@ -410,7 +411,6 @@ func TestSeedFromGlobal_PopulatesLocalLayout(t *testing.T) { paths := ResolveLocalPaths(tmp) state := &config.UserState{} - state.Modules.Gsync.MirrorPath = "/legacy/mirror" state.Modules.Gsync.MaxDelete = 250 state.Modules.Gsync.Interval = 600 state.Modules.Gsync.Paused = true @@ -421,9 +421,13 @@ func TestSeedFromGlobal_PopulatesLocalLayout(t *testing.T) { t.Fatalf("SeedFromGlobal: %v", err) } // Migrated fields: - if cfg.MirrorPath != "/legacy/mirror" || cfg.MaxDelete != 250 || cfg.Interval != 0 || !cfg.Paused { + if cfg.MaxDelete != 250 || cfg.Interval != 0 || !cfg.Paused { t.Errorf("migrated fields wrong: %+v", cfg) } + // The destination is not inherited: only the store's own target sets it. + if cfg.Target != "" { + t.Errorf("seeded Target = %q, want empty (destination is per-store)", cfg.Target) + } if cfg.FilterMode != FilterModeInclude { t.Errorf("migrated FilterMode = %q, want include", cfg.FilterMode) } @@ -451,14 +455,14 @@ func TestLoadOrSeedLocalConfig_MigrationRunsOnce(t *testing.T) { paths := ResolveLocalPaths(tmp) state := &config.UserState{} - state.Modules.Gsync.MirrorPath = "/legacy/mirror" + state.Modules.Gsync.MaxDelete = 777 first, err := LoadOrSeedLocalConfig(state, paths) if err != nil { t.Fatalf("first LoadOrSeedLocalConfig: %v", err) } // Mutate the disk-resident config so we can detect re-migration. - first.MirrorPath = "/edited/by/operator" + first.MaxDelete = 250 if err := SaveLocalConfig(paths, first); err != nil { t.Fatalf("SaveLocalConfig: %v", err) } @@ -468,11 +472,157 @@ func TestLoadOrSeedLocalConfig_MigrationRunsOnce(t *testing.T) { if err != nil { t.Fatalf("second LoadOrSeedLocalConfig: %v", err) } - if second.MirrorPath != "/edited/by/operator" { - t.Errorf("re-migration overwrote local edits: MirrorPath = %q", second.MirrorPath) + if second.MaxDelete != 250 { + t.Errorf("re-migration overwrote local edits: MaxDelete = %d, want 250", second.MaxDelete) + } +} + +// #394 row 10: a store that still carries the retired mirror_path key loads, +// drops it on the next save (keepUnknownConfigKeys must not carry it back a +// third time), and says so once. A second save in the same run stays quiet. +func TestSaveLocalConfig_DropsTheRetiredMirrorPathKeyOnce(t *testing.T) { + tmp := t.TempDir() + paths := ResolveLocalPaths(tmp) + if err := os.MkdirAll(paths.StoreDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(paths.ConfigFile, []byte("target: local:/dropbox/work\nmirror_path: /dropbox/work\n"), 0o644); err != nil { + t.Fatal(err) + } + + cfg, ok, err := LoadLocalConfig(paths) + if err != nil || !ok { + t.Fatalf("LoadLocalConfig: ok=%v err=%v", ok, err) + } + if cfg.Target != "local:/dropbox/work" { + t.Fatalf("Target = %q, want the canonical value", cfg.Target) + } + + out := captureSyncerStderr(t, func() { + if err := SaveLocalConfig(paths, cfg); err != nil { + t.Fatalf("SaveLocalConfig: %v", err) + } + }) + if !strings.Contains(out, "note: dropped the retired `mirror_path` key") || !strings.Contains(out, "`dot sync target`") { + t.Errorf("save did not announce the drop once:\n%s", out) + } + saved, err := os.ReadFile(paths.ConfigFile) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(saved), "mirror_path") { + t.Errorf("saved config still carries the retired key:\n%s", saved) + } + if !strings.Contains(string(saved), "target: local:/dropbox/work") { + t.Errorf("saved config lost the target:\n%s", saved) + } + + again, ok, err := LoadLocalConfig(paths) + if err != nil || !ok { + t.Fatalf("second LoadLocalConfig: ok=%v err=%v", ok, err) + } + if quiet := captureSyncerStderr(t, func() { + if err := SaveLocalConfig(paths, again); err != nil { + t.Fatalf("second SaveLocalConfig: %v", err) + } + }); quiet != "" { + t.Errorf("second save spoke again: %q", quiet) + } +} + +// The mirror of that rule: a store with no target whose retired value names a +// different folder than the detected default is the only thing that stops +// `dot sync` from mirroring into a tree nobody picked. Every routine save +// rewrites the file, so such a save must carry the value and stay quiet; only +// one whose value names the default may drop it, and then it says so. +func TestSaveLocalConfig_CarriesTheRetiredMirrorPathWhileItStillMoves(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + tmp := t.TempDir() + paths := ResolveLocalPaths(tmp) + paths.Home = home + if err := os.MkdirAll(paths.StoreDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(paths.ConfigFile, []byte("mirror_path: /legacy/mirror\n"), 0o644); err != nil { + t.Fatal(err) + } + + cfg, ok, err := LoadLocalConfig(paths) + if err != nil || !ok { + t.Fatalf("LoadLocalConfig: ok=%v err=%v", ok, err) + } + out := captureSyncerStderr(t, func() { + if err := SaveLocalConfig(paths, cfg); err != nil { + t.Fatalf("SaveLocalConfig: %v", err) + } + }) + if strings.Contains(out, "retired") { + t.Errorf("a carried key was announced as dropped: %q", out) + } + saved, err := os.ReadFile(paths.ConfigFile) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(saved), "mirror_path: /legacy/mirror") { + t.Fatalf("the save dropped a value that still moves the destination:\n%s", saved) + } + + // The same store, once its value names the folder resolution would use: the + // key decides nothing, so it goes, out loud. + moot := filepath.Join(home, "gdrive-workspace", "work") + if want := defaultMirrorPath(home); moot != want { + t.Fatalf("test default drifted: %q vs %q", moot, want) + } + if err := os.WriteFile(paths.ConfigFile, []byte("mirror_path: "+moot+"\n"), 0o644); err != nil { + t.Fatal(err) + } + mootCfg, ok, err := LoadLocalConfig(paths) + if err != nil || !ok { + t.Fatalf("second LoadLocalConfig: ok=%v err=%v", ok, err) + } + out = captureSyncerStderr(t, func() { + if err := SaveLocalConfig(paths, mootCfg); err != nil { + t.Fatalf("second SaveLocalConfig: %v", err) + } + }) + if !strings.Contains(out, "note: dropped the retired `mirror_path` key") { + t.Errorf("a key that decides nothing was dropped without a note: %q", out) + } + if saved, err = os.ReadFile(paths.ConfigFile); err != nil { + t.Fatal(err) + } + if strings.Contains(string(saved), "mirror_path") { + t.Errorf("saved config still carries the retired key:\n%s", saved) } } +// captureSyncerStderr runs fn with os.Stderr redirected to a pipe and returns +// what was written. The syncer's notes go straight to os.Stderr and take no +// injectable writer, so the pipe is the only seam. +func captureSyncerStderr(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("os.Pipe: %v", err) + } + orig := os.Stderr + os.Stderr = w + done := make(chan string, 1) + go func() { + var buf strings.Builder + _, _ = io.Copy(&buf, r) + done <- buf.String() + }() + fn() + os.Stderr = orig + _ = w.Close() + out := <-done + _ = r.Close() + return out +} + func TestLocalState_RoundTrip(t *testing.T) { tmp := t.TempDir() paths := ResolveLocalPaths(tmp) @@ -635,7 +785,7 @@ func TestSaveLocalConfigRefusesRunawayAliases(t *testing.T) { // are exactly what it marshals (an untagged field would be carried twice). func TestLocalConfigKeysMatchWhatItMarshals(t *testing.T) { full := LocalConfig{Target: "t", Owner: "o", OwnerAliases: []string{"a"}, OwnerEpoch: 1, FencePending: true, IncludeSubmodules: true, - MirrorPath: "m", FilterMode: DefaultFilterMode(), MaxDelete: 1, Interval: 1, PullInterval: 1, PushMode: "clean", PullMode: "clean", + FilterMode: DefaultFilterMode(), MaxDelete: 1, Interval: 1, PullInterval: 1, PushMode: "clean", PullMode: "clean", Paused: true, SharedExcludes: []string{"x"}, Hooks: PeerHooks{OnActivate: []string{"a"}}, RemoteDot: "d", HostMerge: map[string][]string{"f": {"k"}}} data, err := yaml.Marshal(&full) if err != nil { diff --git a/internal/syncer/pull.go b/internal/syncer/pull.go index d8e55f77..285567c9 100644 --- a/internal/syncer/pull.go +++ b/internal/syncer/pull.go @@ -62,6 +62,9 @@ func PullTracked(cfg *Config, opts PullOptions) (*PullResult, error) { if err := refuseSharedDriveMirror(cfg); err != nil { return nil, err } + if err := refuseRetiredDestinationChange(cfg); err != nil { + return nil, err + } paths := cfg.LocalPaths local := strings.TrimRight(cfg.LocalPath, "/") diff --git a/internal/syncer/push_plan.go b/internal/syncer/push_plan.go index 7be75c4b..dd996ee4 100644 --- a/internal/syncer/push_plan.go +++ b/internal/syncer/push_plan.go @@ -100,6 +100,9 @@ func PlanPush(cfg *Config) (*PushPlan, error) { if err := refuseSharedDriveMirror(cfg); err != nil { return nil, err } + if err := refuseRetiredDestinationChange(cfg); err != nil { + return nil, err + } local := strings.TrimRight(cfg.LocalPath, "/") mirror := strings.TrimRight(cfg.MirrorPath, "/") diff --git a/internal/syncer/sync.go b/internal/syncer/sync.go index 857e6995..e49cafac 100644 --- a/internal/syncer/sync.go +++ b/internal/syncer/sync.go @@ -16,25 +16,20 @@ import ( "github.com/STAIxBWLB/dotfiles/internal/appsettings" "github.com/STAIxBWLB/dotfiles/internal/config" "github.com/STAIxBWLB/dotfiles/internal/exec" + "github.com/STAIxBWLB/dotfiles/internal/fileutil" ) -// defaultMirrorPath is the mirror used when neither the local config nor the -// global state set one. It prefers a detected cloud root (Dropbox, then -// Google Drive) so the mirror follows the same cloud the backup root uses; -// absent any cloud root it falls back to ~/gdrive-workspace/work. +// defaultMirrorPath is the mirror used when the store sets no target. It +// prefers a detected cloud root (Dropbox, then Google Drive) so the mirror +// follows the same cloud the backup root uses; absent any cloud root it falls +// back to ~/gdrive-workspace/work. func defaultMirrorPath(home string) string { - if cloud := appsettings.DetectCloudCandidate(home); cloud != "" { - // cloud is "/secrets/dotfiles-backup"; the mirror lives at - // "/work", i.e. two levels up from the secrets marker. - return filepath.Join(filepath.Dir(filepath.Dir(cloud)), "work") - } - return filepath.Join(home, defaultMirrorRel) + return appsettings.DefaultMirrorPath(home) } // Defaults applied when the user state has not specified a value. const ( defaultLocalRel = "workspace/work" - defaultMirrorRel = "gdrive-workspace/work" defaultMaxDelete = 1000 logRotateMaxLines = 2000 logRotateKeepLines = 1000 @@ -52,18 +47,26 @@ type Config struct { MirrorPath string // local mirror tree with trailing slash; empty for ssh targets Target Target // parsed destination (local dir or ssh host:path) MirrorIsDefault bool // target came from defaultMirrorPath, not explicit config - FilterMode FilterMode - IncludeFile string // editable include list (under .dotfiles/sync/) - IncludePatterns []string // parsed include list used by Go filters + rsync args - ExcludesFile string // materialized static exclude list (under .dotfiles/sync/) - IgnoreFile string // user-supplied ignore patterns (under .dotfiles/sync/) - AllowFile string // explicit secrets opt-in patterns (under .dotfiles/sync/) - AllowPatterns []string // parsed allow.txt — re-included ahead of the secrets layer - ConfigDir string // workspace-local store dir (.dotfiles/sync/) — dynamic files land here - SharedExcludes []string // operator-curated shared paths (relative to MirrorPath) - LogFile string - LockDir string // copied once from SystemPaths.LockDir at the single resolution point - RsyncPath string // resolved rsync binary; empty if not installed + // retiredMirrorPath is the folder a file this config was resolved from still + // names in the retired `mirror_path` key, expanded and absolute, when the + // store set no target and the destination fell back to the detected default. + // retiredMirrorSource says which file named it. The transfer entries read + // these only to refuse a destination that would move silently + // (refuseRetiredDestinationChange); they never route anything. + retiredMirrorPath string + retiredMirrorSource string + FilterMode FilterMode + IncludeFile string // editable include list (under .dotfiles/sync/) + IncludePatterns []string // parsed include list used by Go filters + rsync args + ExcludesFile string // materialized static exclude list (under .dotfiles/sync/) + IgnoreFile string // user-supplied ignore patterns (under .dotfiles/sync/) + AllowFile string // explicit secrets opt-in patterns (under .dotfiles/sync/) + AllowPatterns []string // parsed allow.txt — re-included ahead of the secrets layer + ConfigDir string // workspace-local store dir (.dotfiles/sync/) — dynamic files land here + SharedExcludes []string // operator-curated shared paths (relative to MirrorPath) + LogFile string + LockDir string // copied once from SystemPaths.LockDir at the single resolution point + RsyncPath string // resolved rsync binary; empty if not installed // RemoteRsyncPath is passed as --rsync-path for ssh targets whose default // rsync cannot serve a modern client (macOS 26 openrsync). Empty = default OK. RemoteRsyncPath string @@ -244,6 +247,7 @@ func resolveConfig(state *config.UserState, migrate bool, home, profile string) } localPaths := ResolveLocalPathsForProfile(localPath, profile) + localPaths.Home = home var localCfg *LocalConfig if migrate && profile == DefaultProfile { @@ -282,10 +286,16 @@ func resolveConfig(state *config.UserState, migrate bool, home, profile string) } } - // Resolve the destination: the canonical `target` spec wins, then the - // legacy mirror_path (local + global), then the detected cloud default. + // Resolve the destination: the store's `target` spec decides it, else the + // detected cloud default. The retired `mirror_path` key is never a route + // (#394 row 10); it is still read here when the store sets no target, + // because a machine that names another folder in it would otherwise change + // destination without being asked. The transfer entries refuse on that + // (refuseRetiredDestinationChange); this resolve does not, so the + // `dot sync target` that fixes it can still run. var target Target mirrorIsDefault := false + retiredMirrorPath, retiredMirrorSource := "", "" if spec := strings.TrimSpace(localCfg.Target); spec != "" { t, err := ParseTarget(spec) if err != nil { @@ -293,15 +303,13 @@ func resolveConfig(state *config.UserState, migrate bool, home, profile string) } target = t } else { - mirrorPath := localCfg.MirrorPath - if mirrorPath == "" { - mirrorPath = gs.MirrorPath - } - mirrorIsDefault = mirrorPath == "" - if mirrorIsDefault { - mirrorPath = defaultMirrorPath(home) + mirrorIsDefault = true + target = Target{Kind: TargetLocal, Path: defaultMirrorPath(home)} + if legacy := strings.TrimSpace(localCfg.retired[retiredMirrorPathKey]); legacy != "" { + retiredMirrorPath, retiredMirrorSource = expandHome(legacy, home), "this store's config" + } else if legacy := strings.TrimSpace(state.RetiredMirrorPath()); legacy != "" { + retiredMirrorPath, retiredMirrorSource = expandHome(legacy, home), "the global state" } - target = Target{Kind: TargetLocal, Path: mirrorPath} } mirrorPath := "" if target.Kind == TargetLocal { @@ -341,41 +349,43 @@ func resolveConfig(state *config.UserState, migrate bool, home, profile string) } return &Config{ - Profile: profile, - Home: override, - Owner: localCfg.Owner, - HostMerge: localCfg.HostMerge, - OwnerAliases: localCfg.OwnerAliases, - Hooks: localCfg.Hooks, - RemoteDot: localCfg.RemoteDot, - OwnerEpoch: localCfg.OwnerEpoch, - FencePending: localCfg.FencePending, - IncludeSubmodules: localCfg.IncludeSubmodules, - LocalPath: localPath, - MirrorPath: mirrorPath, - Target: target, - MirrorIsDefault: mirrorIsDefault, - FilterMode: filterMode, - IncludeFile: localPaths.IncludeFile, - IncludePatterns: includePatterns, - ExcludesFile: localPaths.ExcludeFile, - IgnoreFile: localPaths.IgnoreFile, - AllowFile: localPaths.AllowFile, - AllowPatterns: allowPatterns, - ConfigDir: localPaths.StoreDir, - SharedExcludes: append([]string(nil), localCfg.SharedExcludes...), - LogFile: localPaths.LogFile, - LockDir: systemPaths.LockDir, - RsyncPath: rsyncPath, - MaxDelete: maxDelete, - Interval: schedule.Interval, - PullInterval: schedule.PullInterval, - PushMode: schedule.PushMode, - PullMode: schedule.PullMode, - Propagation: policy, - Paused: localCfg.Paused, - LocalPaths: localPaths, - SystemPaths: systemPaths, + Profile: profile, + Home: override, + Owner: localCfg.Owner, + HostMerge: localCfg.HostMerge, + OwnerAliases: localCfg.OwnerAliases, + Hooks: localCfg.Hooks, + RemoteDot: localCfg.RemoteDot, + OwnerEpoch: localCfg.OwnerEpoch, + FencePending: localCfg.FencePending, + IncludeSubmodules: localCfg.IncludeSubmodules, + LocalPath: localPath, + MirrorPath: mirrorPath, + Target: target, + MirrorIsDefault: mirrorIsDefault, + retiredMirrorPath: retiredMirrorPath, + retiredMirrorSource: retiredMirrorSource, + FilterMode: filterMode, + IncludeFile: localPaths.IncludeFile, + IncludePatterns: includePatterns, + ExcludesFile: localPaths.ExcludeFile, + IgnoreFile: localPaths.IgnoreFile, + AllowFile: localPaths.AllowFile, + AllowPatterns: allowPatterns, + ConfigDir: localPaths.StoreDir, + SharedExcludes: append([]string(nil), localCfg.SharedExcludes...), + LogFile: localPaths.LogFile, + LockDir: systemPaths.LockDir, + RsyncPath: rsyncPath, + MaxDelete: maxDelete, + Interval: schedule.Interval, + PullInterval: schedule.PullInterval, + PushMode: schedule.PushMode, + PullMode: schedule.PullMode, + Propagation: policy, + Paused: localCfg.Paused, + LocalPaths: localPaths, + SystemPaths: systemPaths, }, nil } @@ -639,13 +649,42 @@ func refuseSharedDriveMirror(cfg *Config) error { return fmt.Errorf( "refusing to sync: mirror %q resolves under a Drive 'Shared drives' root.\n"+ "Workspace-authoritative semantics would propagate deletions into a team drive.\n"+ - "Point gsync.mirror_path at a folder under My Drive instead", + "Set the sync target to a folder under My Drive instead (`dot sync target`)", cfg.MirrorPath, ) } return nil } +// refuseRetiredDestinationChange stops a run whose destination would move +// without being asked. A store with no `target` resolves the detected cloud +// default, but the same machine may still name another folder in the retired +// `mirror_path` key, which chose the destination until #394 row 10. Push and +// pull both propagate, so going ahead could delete or overwrite in a tree the +// user never picked, and the save-time note arrives too late to say so. +// +// Reading the key here is a refusal, not a route: the same directory (through +// symlinks too) proceeds with the default this config already resolved, and +// anything else stops before a transfer with the command that settles it. It +// is deliberately not part of resolution: `dot sync target`, the fix the error +// names, resolves a config of its own and would fail with everything else. +func refuseRetiredDestinationChange(cfg *Config) error { + if cfg == nil || cfg.retiredMirrorPath == "" || !cfg.MirrorIsDefault { + return nil + } + if fileutil.SameDirectory(cfg.retiredMirrorPath, cfg.MirrorPath) { + return nil + } + stored := strings.TrimRight(cfg.retiredMirrorPath, "/") + detected := strings.TrimRight(cfg.MirrorPath, "/") + return fmt.Errorf( + "refusing to sync: %s still sets the retired `mirror_path` to %q and this store has no `target`.\n"+ + "Sync would use %q instead, and push and pull both propagate changes.\n"+ + "Keep the stored folder with `dot sync target %s`, or accept the new one with `dot sync target %s`", + cfg.retiredMirrorSource, stored, detected, stored, detected, + ) +} + // ── pull / push / sync ────────────────────────────────────────────────── // Push local → mirror under cfg.Propagation. The policy maps to rsync flags @@ -685,6 +724,9 @@ func Push(ctx context.Context, runner *exec.Runner, cfg *Config, dryRun bool) er if err := refuseSharedDriveMirror(cfg); err != nil { return err } + if err := refuseRetiredDestinationChange(cfg); err != nil { + return err + } rf, err := prepareRuntimeFilters(cfg, dryRun) defer rf.Cleanup() if err != nil { @@ -783,6 +825,12 @@ type FetchResult struct { // (#231). An absolute path is read relative to the workspace when it lies // under it. func Fetch(ctx context.Context, runner *exec.Runner, cfg *Config, rels []string, dryRun bool) (*FetchResult, error) { + // A fetch copies mirror → workspace, so a destination that moved without + // being asked would refill the workspace from a tree nobody picked and + // overwrite what is there. Same refusal as the other transfer entries. + if err := refuseRetiredDestinationChange(cfg); err != nil { + return nil, err + } if !dryRun { if err := ensureLogDir(cfg.LogFile); err != nil { return nil, err @@ -964,6 +1012,9 @@ func fetchScopeArgs(entries []fetchEntry) ([]string, error) { // files are never deleted; remote-newer files overwrite local with a backup // under .sync-conflicts/. func PullDirect(ctx context.Context, runner *exec.Runner, cfg *Config, dryRun bool) error { + if err := refuseRetiredDestinationChange(cfg); err != nil { + return err + } if !dryRun { if err := ensureLogDir(cfg.LogFile); err != nil { return err diff --git a/internal/syncer/sync_test.go b/internal/syncer/sync_test.go index a1f4d086..153203b9 100644 --- a/internal/syncer/sync_test.go +++ b/internal/syncer/sync_test.go @@ -57,6 +57,20 @@ func newIsolatedState(t *testing.T) *config.UserState { return state } +// writeStoreConfig puts body at /.dotfiles/sync/config.yaml, the +// store ResolveConfig reads. A read-only resolve never creates one, so a test +// that wants a store must write it itself. +func writeStoreConfig(t *testing.T, localPath, body string) { + t.Helper() + dir := filepath.Join(localPath, ".dotfiles", "sync") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + func TestPullArgs_WorkspaceAuthoritative(t *testing.T) { cfg := newTestConfig(t) conflict := &ConflictDir{Timestamp: "2026-05-01T12-00-00Z"} @@ -353,7 +367,6 @@ func TestResolveConfig_StateOverrides(t *testing.T) { tmp := t.TempDir() state := &config.UserState{} state.Modules.Gsync.LocalPath = tmp - state.Modules.Gsync.MirrorPath = "/custom/mirror" state.Modules.Gsync.MaxDelete = 50 cfg, err := ResolveConfig(state) @@ -363,9 +376,6 @@ func TestResolveConfig_StateOverrides(t *testing.T) { if cfg.LocalPath != tmp+"/" { t.Errorf("LocalPath = %q, want %q", cfg.LocalPath, tmp+"/") } - if cfg.MirrorPath != "/custom/mirror/" { - t.Errorf("MirrorPath = %q, want trailing-slashed /custom/mirror/", cfg.MirrorPath) - } if cfg.MaxDelete != 50 { t.Errorf("MaxDelete = %d, want 50", cfg.MaxDelete) } @@ -413,7 +423,7 @@ func TestResolveConfig_PrefersCloudMirror(t *testing.T) { } } -func TestResolveConfig_LocalMirrorWinsOverCloud(t *testing.T) { +func TestResolveConfig_LocalTargetWinsOverCloud(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) if err := os.MkdirAll(filepath.Join(home, "Library", "CloudStorage", "Dropbox", "secrets"), 0o755); err != nil { @@ -421,7 +431,7 @@ func TestResolveConfig_LocalMirrorWinsOverCloud(t *testing.T) { } state := &config.UserState{} state.Modules.Gsync.LocalPath = t.TempDir() - state.Modules.Gsync.MirrorPath = "/explicit/mirror" + writeStoreConfig(t, state.Modules.Gsync.LocalPath, "target: local:/explicit/mirror\n") cfg, err := ResolveConfig(state) if err != nil { @@ -432,6 +442,200 @@ func TestResolveConfig_LocalMirrorWinsOverCloud(t *testing.T) { } } +// #394 row 10: the retired mirror_path key decides nothing any more. A store +// that still carries one and no target resolves the detected default, the same +// as a store with an empty target. +func TestResolveConfig_IgnoresTheRetiredMirrorPathKey(t *testing.T) { + state := newIsolatedState(t) + writeStoreConfig(t, state.Modules.Gsync.LocalPath, "mirror_path: /legacy/mirror\n") + + cfg, err := ResolveConfigReadOnly(state) + if err != nil { + t.Fatalf("ResolveConfigReadOnly: %v", err) + } + want := filepath.Join(os.Getenv("HOME"), "gdrive-workspace", "work") + "/" + if cfg.MirrorPath != want { + t.Errorf("MirrorPath = %q, want the detected default %q (mirror_path must not be read)", cfg.MirrorPath, want) + } +} + +// #394 row 10: a store with no `target` resolves the detected default, but the +// folder it still names in `mirror_path` was the destination until this release. +// A different folder has to stop a transfer instead of moving it silently: push +// and pull propagate, so a run could delete in a tree nobody picked. +func TestRefuseRetiredDestinationChange_RefusesAnotherFolder(t *testing.T) { + state := newIsolatedState(t) + writeStoreConfig(t, state.Modules.Gsync.LocalPath, "mirror_path: /legacy/mirror\n") + + // Resolution itself must not refuse: `dot sync target`, the fix the error + // names, resolves a config of its own before it can write the store. + cfg, err := ResolveConfigReadOnly(state) + if err != nil { + t.Fatalf("ResolveConfigReadOnly: %v", err) + } + if cfg.retiredMirrorSource != "this store's config" { + t.Errorf("retiredMirrorSource = %q, want the store", cfg.retiredMirrorSource) + } + + err = refuseRetiredDestinationChange(cfg) + if err == nil { + t.Fatal("a store naming another folder in mirror_path must refuse a transfer") + } + for _, want := range []string{ + "/legacy/mirror", + "dot sync target /legacy/mirror", + strings.TrimRight(cfg.MirrorPath, "/"), + } { + if !strings.Contains(err.Error(), want) { + t.Errorf("refusal does not name %q:\n%v", want, err) + } + } +} + +// The same folder spelled through a symlink is not a destination change: the +// peer Mac's ~/Dropbox is a symlink into ~/Library/CloudStorage/Dropbox. +func TestRefuseRetiredDestinationChange_AllowsALinkedSameFolder(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + cloud := filepath.Join(home, "Library", "CloudStorage", "Dropbox") + for _, dir := range []string{"work", "secrets"} { + if err := os.MkdirAll(filepath.Join(cloud, dir), 0o755); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink(cloud, filepath.Join(home, "Dropbox")); err != nil { + t.Fatal(err) + } + state := &config.UserState{} + state.Modules.Gsync.LocalPath = t.TempDir() + writeStoreConfig(t, state.Modules.Gsync.LocalPath, + "mirror_path: "+filepath.Join(home, "Dropbox", "work")+"\n") + + cfg, err := ResolveConfigReadOnly(state) + if err != nil { + t.Fatalf("ResolveConfigReadOnly: %v", err) + } + want := filepath.Join(cloud, "work") + "/" + if cfg.MirrorPath != want { + t.Fatalf("MirrorPath = %q, want the cloud default %q", cfg.MirrorPath, want) + } + if err := refuseRetiredDestinationChange(cfg); err != nil { + t.Errorf("the same tree through a symlink must sync, got: %v", err) + } +} + +// The global state is the other place the key can still live, and a new +// workspace store is seeded from it. +func TestRefuseRetiredDestinationChange_ReadsTheGlobalState(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + localPath := t.TempDir() + statePath := filepath.Join(t.TempDir(), "config.yaml") + // `gdrive_sync` is the tag the struct and every real file use; `gsync` was + // never a key, so a fixture spelled that way proves nothing. + body := "schema_version: 4\nname: Test\nprofile: full\nmodules:\n gdrive_sync:\n local_path: " + + localPath + "\n mirror_path: /legacy/mirror\n" + if err := os.WriteFile(statePath, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + state, err := config.LoadStateFrom(statePath) + if err != nil { + t.Fatalf("LoadStateFrom: %v", err) + } + + cfg, err := ResolveConfigReadOnly(state) + if err != nil { + t.Fatalf("ResolveConfigReadOnly: %v", err) + } + if cfg.retiredMirrorSource != "the global state" { + t.Errorf("retiredMirrorSource = %q, want the global state", cfg.retiredMirrorSource) + } + err = refuseRetiredDestinationChange(cfg) + if err == nil { + t.Fatal("an empty target with a global mirror_path must refuse a transfer") + } + if !strings.Contains(err.Error(), "the global state") || !strings.Contains(err.Error(), "/legacy/mirror") { + t.Errorf("refusal does not name the source and the stored value:\n%v", err) + } +} + +// The refusals hang off the transfer entries, not off resolution: a push plan +// built from such a store stops before it walks either tree. +func TestPlanPush_RefusesARetiredDestination(t *testing.T) { + state := newIsolatedState(t) + writeStoreConfig(t, state.Modules.Gsync.LocalPath, "mirror_path: /legacy/mirror\n") + cfg, err := ResolveConfigReadOnly(state) + if err != nil { + t.Fatalf("ResolveConfigReadOnly: %v", err) + } + + _, err = PlanPush(cfg) + if err == nil { + t.Fatal("PlanPush planned a push into a destination nobody chose") + } + if !strings.Contains(err.Error(), "dot sync target /legacy/mirror") { + t.Errorf("PlanPush refusal does not name the fix:\n%v", err) + } +} + +// The refusal must not be one-shot. Its only evidence is the retired value, and +// every routine store save rewrites the file: `dot sync pause`/`resume` (the +// reload the other half of this change tells users to run), `dot sync owner`, +// `dot sync setup`, `dot sync shared` all go through SaveLocalConfig while +// `target` is still empty. A save that dropped the key there would hand the +// next push the detected default, which is exactly the silent redirect this +// guard exists to stop. +func TestPlanPush_SurvivesAnUnrelatedStoreSave(t *testing.T) { + state := newIsolatedState(t) + writeStoreConfig(t, state.Modules.Gsync.LocalPath, "mirror_path: /legacy/mirror\n") + + cfg, err := ResolveConfigReadOnly(state) + if err != nil { + t.Fatalf("ResolveConfigReadOnly: %v", err) + } + out := captureSyncerStderr(t, func() { + if err := SetLocalPaused(cfg, true); err != nil { + t.Fatalf("SetLocalPaused: %v", err) + } + }) + if strings.Contains(out, "retired") { + t.Errorf("an unrelated save announced a drop: %q", out) + } + saved, err := os.ReadFile(cfg.LocalPaths.ConfigFile) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(saved), "mirror_path: /legacy/mirror") { + t.Fatalf("the save dropped the only evidence of the destination change:\n%s", saved) + } + + // The next run resolves afresh, as the next `dot sync push` would. + next, err := ResolveConfigReadOnly(state) + if err != nil { + t.Fatalf("ResolveConfigReadOnly after the save: %v", err) + } + if _, err := PlanPush(next); err == nil { + t.Fatal("after an unrelated save the push no longer refuses a destination nobody chose") + } else if !strings.Contains(err.Error(), "dot sync target /legacy/mirror") { + t.Errorf("the refusal after the save does not name the fix:\n%v", err) + } +} + +// A store that sets `target` moved its destination deliberately: the retired +// key is irrelevant, whatever it says. +func TestRefuseRetiredDestinationChange_IgnoresAStoreWithTarget(t *testing.T) { + state := newIsolatedState(t) + writeStoreConfig(t, state.Modules.Gsync.LocalPath, "target: local:/chosen/mirror\nmirror_path: /legacy/mirror\n") + + cfg, err := ResolveConfigReadOnly(state) + if err != nil { + t.Fatalf("ResolveConfigReadOnly: %v", err) + } + if err := refuseRetiredDestinationChange(cfg); err != nil { + t.Errorf("an explicit target must not be guarded: %v", err) + } +} + func TestResolveConfigReadOnlyForHome_UsesGivenHome(t *testing.T) { // os.UserHomeDir() would resolve here — the --home override must not use it. currentHome := t.TempDir() @@ -440,7 +644,7 @@ func TestResolveConfigReadOnlyForHome_UsesGivenHome(t *testing.T) { overrideHome := t.TempDir() state := &config.UserState{} state.Modules.Gsync.LocalPath = t.TempDir() - // MirrorPath empty → default is /gdrive-workspace/work. + // No target in the store → default is /gdrive-workspace/work. cfg, err := ResolveConfigReadOnlyForHomeProfile(state, overrideHome, DefaultProfile) if err != nil { @@ -462,7 +666,7 @@ func TestResolveConfigReadOnlyForHome_ExpandsTildeAgainstHome(t *testing.T) { state := &config.UserState{} state.Modules.Gsync.LocalPath = t.TempDir() - state.Modules.Gsync.MirrorPath = "~/cloud/work" + writeStoreConfig(t, state.Modules.Gsync.LocalPath, "target: local:~/cloud/work\n") cfg, err := ResolveConfigReadOnlyForHomeProfile(state, overrideHome, DefaultProfile) if err != nil { diff --git a/internal/syncer/target.go b/internal/syncer/target.go index 9825e645..34bc4b1c 100644 --- a/internal/syncer/target.go +++ b/internal/syncer/target.go @@ -57,7 +57,8 @@ func ParseTarget(spec string) (Target, error) { } return Target{Kind: TargetSSH, Host: host, Path: path}, nil default: - // Bare paths keep legacy mirror_path values working. + // A bare path is the shorthand `dot sync target ~/Dropbox/work` takes; + // it is not a read of any retired config key. if err := validateTargetField("local path", spec, true); err != nil { return Target{}, err }