From 885b8d0323bc00e3b88ccea5568166d6502648e9 Mon Sep 17 00:00:00 2001 From: Peter Donald Date: Tue, 14 Jul 2026 23:37:29 +1000 Subject: [PATCH] feat(sync): require sources to opt in to pushes Add optional sync_push configuration and an add --sync-push flag. Sync now limits automatic push planning to opted-in branch sources while continuing to pull every selected source. --- README.md | 46 ++++++--- docs/migration-from-ruby-braid.md | 23 +++-- integration/support.go | 6 +- integration/sync_test.go | 6 +- internal/cli/cli.go | 2 + internal/cli/cli_test.go | 10 +- internal/cli/schema.go | 7 +- internal/clitest/completion_contract.go | 22 +++-- internal/command/add.go | 2 +- internal/command/add_test.go | 13 +++ internal/command/completion_test.go | 2 + internal/command/sync.go | 17 ++-- internal/command/sync_test.go | 120 +++++++++--------------- internal/config/config.go | 9 +- internal/config/config_test.go | 24 ++++- internal/source/source.go | 1 + 16 files changed, 178 insertions(+), 132 deletions(-) diff --git a/README.md b/README.md index dcbcd2f..300884c 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,7 @@ Start anywhere inside an existing Git repository. Add a named upstream source and one local mirror: ```bash -braid add lib/grit +braid add lib/grit --sync-push ``` Braid copies the upstream content into `lib/grit`, records the source and mirror @@ -174,7 +174,7 @@ Use `--no-commit` when the mirror add belongs in the same commit as other changes: ```bash -braid add lib/grit --no-commit +braid add lib/grit --sync-push --no-commit git commit ``` @@ -200,17 +200,18 @@ braid push lib/grit braid push lib/grit --branch myproject_customizations ``` -For the common branch-mirror workflow, `sync` combines the tracked-branch push -and follow-up pull: +Because the source was added with `--sync-push`, `sync` combines the +tracked-branch push and follow-up pull: ```bash braid sync lib/grit ``` -It pushes committed local mirror changes when the branch is still up to date, -then pulls the selected mirror so `.braids.json` records the new upstream -revision. Use the explicit `push` and `pull` commands when you need to push to -a different branch or handle each step separately. +For sources with `"sync_push": true`, it pushes committed local mirror changes +when the branch is still up to date. It then pulls every selected source, +including sources that are not opted into sync pushes, so `.braids.json` +records the new upstream revision. Use the explicit `push` and `pull` commands +when you need to push to a different branch or handle each step separately. ```bash braid pull lib/grit @@ -240,6 +241,17 @@ braid add https://github.com/rails/rails.git vendor/rails --branch 5-0-stable braid add https://github.com/rails/rails.git vendor/rails-7 --tag v7.0.0 ``` +Opt a branch-tracking source into the push phase of `braid sync`: + +```bash +braid add https://github.com/rails/rails.git vendor/rails --sync-push +``` + +Without `--sync-push`, `braid sync` pulls the source but does not push it. +`--sync-push` cannot be combined with `--tag` or `--revision`, and cannot be +used when adding mirrors to an existing `:source`. Explicit `braid push` is +unaffected by this setting. + Lock a source to an explicit revision when you do not want ordinary `braid pull` runs to move it: @@ -368,7 +380,8 @@ mirror path and `.braids.json` from `HEAD`, then remove `.git/MERGE_MSG`. ### Syncing Mirrors -`braid sync` runs the safe push-then-pull workflow for branch-tracking sources: +`braid sync` pulls selected sources and first pushes committed changes for +branch-tracking sources that have opted in with `sync_push`: ```bash braid sync vendor/rails @@ -426,12 +439,12 @@ index state. If automatic restoration succeeds but the saved stash cannot be dropped safely, Braid leaves your restored work in place, keeps the stash recoverable, and tells you to inspect `git stash list` before manual cleanup. -The default push phase only auto-pushes branch-tracking sources with committed -local mirror changes. Branch sources without committed local changes are skipped -quietly and still update normally, even if upstream has moved. Selected tag or -revision-locked sources with committed local changes stop the sync because `sync` has -no `--branch`; run `braid push --branch ` for that explicit push -intent, or rerun with `--pull-only` if you only intended to pull. +The push phase only considers branch-tracking sources with `"sync_push": true`. +Sources with omitted or false `sync_push` are skipped quietly during that phase +and still update normally, even when explicitly selected. Opted-in branch +sources without committed local changes are also skipped quietly. Tag and +revision-locked sources cannot enable `sync_push`; use +`braid push --branch ` for explicit push intent. If a changed branch source's upstream branch moved since the recorded revision, `sync` fails before any mirror is pushed. Pull first, resolve conflicts if @@ -623,6 +636,7 @@ Config version 2 records named sources and their mirrors: "url": "https://github.com/replicant4j/replicant.git", "branch": "master", "revision": "18480c9dc34f948218a0c15370712d27b2626fa0", + "sync_push": true, "mirrors": { "licenses/replicant-LICENSE.txt": "LICENSE.txt", "vendor/libs/replicant": "" @@ -635,6 +649,8 @@ Config version 2 records named sources and their mirrors: Braid validates configured mirror paths for cross-platform safety. It does not preflight every file inside the selected upstream tree; if an upstream filename cannot be materialized on the current OS, Git reports the checkout failure. +The optional `sync_push` field defaults to false and is only valid for +branch-tracking sources. When you vendor only a subdirectory or single file, remember that license files outside the mirrored path are not copied automatically. If the upstream license diff --git a/docs/migration-from-ruby-braid.md b/docs/migration-from-ruby-braid.md index 934160a..caedde6 100644 --- a/docs/migration-from-ruby-braid.md +++ b/docs/migration-from-ruby-braid.md @@ -102,9 +102,12 @@ Ruby Braid has no `sync` command. Current Go Braid adds: braid sync [selector...] [--pull-only] [--autostash] [--keep] ``` -The default `sync` workflow pushes committed local changes for branch-tracking -sources and then pulls every mirror in each source to the new upstream revision. -`--pull-only` skips the push phase and only pulls. With no selectors, `sync` +The default `sync` workflow pushes committed local changes only for +branch-tracking sources configured with `"sync_push": true`, then pulls every +mirror in each selected source to the new upstream revision. Add a new opted-in +source with `braid add --sync-push`. Sources with omitted or false +`sync_push` are pull-only during sync, even when explicitly selected. +`--pull-only` skips the push phase for every source. With no selectors, `sync` selects all branch and tag sources in lexicographic source-name order and skips revision-locked sources, matching no-selector `pull` selection. A selector may be `:source` or one of its mirror paths; aliases are deduplicated. @@ -117,12 +120,14 @@ changes; the push phase still uses the mirror content recorded in downstream Migration impact: -- Use `sync` for the common "push local mirror commits upstream, then pull the - downstream recorded revision" workflow. +- Set `"sync_push": true` on branch sources that should participate in sync's + push phase; omission is pull-only and requires no config version upgrade. +- Use `sync` for the common "push opted-in local mirror commits upstream, then + pull the downstream recorded revision" workflow. - Use `sync --pull-only` as a stricter, scoped pull workflow when you do not want any push attempt. -- Do not expect `sync` to push tag-tracking or revision-locked sources with local changes unless - you use `braid push --branch ` explicitly. +- Tag-tracking and revision-locked sources cannot enable `sync_push`; use + `braid push --branch ` explicitly. ### `push` is more explicit about committed changes and commit messages @@ -334,6 +339,6 @@ Known output differences include: shared full-cache path. 8. For push workflows, make sure local mirror edits are committed downstream before `braid push` or `braid sync`. -9. Prefer `braid sync` for the push-then-pull workflow once the team has - tested it on the repository. +9. Enable `sync_push` only for branch sources that should be pushed, then use + `braid sync` for the push-then-pull workflow. 10. Optionally install Bash completion with `braid completion bash`. diff --git a/integration/support.go b/integration/support.go index bd5eaff..3875c9c 100644 --- a/integration/support.go +++ b/integration/support.go @@ -288,6 +288,7 @@ type configSource struct { Tag string `json:"tag,omitempty"` Revision string `json:"revision"` PartialClone bool `json:"partial_clone,omitempty"` + SyncPush bool `json:"sync_push,omitempty"` Mirrors map[string]string `json:"mirrors"` } @@ -298,6 +299,7 @@ type configMirror struct { Tag string `json:"tag,omitempty"` Revision string `json:"revision"` PartialClone bool `json:"partial_clone,omitempty"` + SyncPush bool `json:"sync_push,omitempty"` } func expectedConfigRaw(t *testing.T, mirrors map[string]configMirror) string { @@ -312,7 +314,7 @@ func expectedConfigRaw(t *testing.T, mirrors map[string]configMirror) string { groups := map[string]string{} for _, local := range keys { m := mirrors[local] - key := strings.Join([]string{strings.TrimRight(m.URL, `/\`), m.Branch, m.Tag, m.Revision, strconv.FormatBool(m.PartialClone)}, "\x00") + key := strings.Join([]string{strings.TrimRight(m.URL, `/\`), m.Branch, m.Tag, m.Revision, strconv.FormatBool(m.PartialClone), strconv.FormatBool(m.SyncPush)}, "\x00") name := groups[key] if name == "" { name = strings.TrimSuffix(path.Base(strings.ReplaceAll(strings.TrimRight(m.URL, `/\`), `\`, "/")), ".git") @@ -321,7 +323,7 @@ func expectedConfigRaw(t *testing.T, mirrors map[string]configMirror) string { name = fmt.Sprintf("%s-%d", name, used[name]) } groups[key] = name - sources[name] = configSource{URL: strings.TrimRight(m.URL, `/\`), Branch: m.Branch, Tag: m.Tag, Revision: m.Revision, PartialClone: m.PartialClone, Mirrors: map[string]string{}} + sources[name] = configSource{URL: strings.TrimRight(m.URL, `/\`), Branch: m.Branch, Tag: m.Tag, Revision: m.Revision, PartialClone: m.PartialClone, SyncPush: m.SyncPush, Mirrors: map[string]string{}} } s := sources[name] s.Mirrors[local] = m.Path diff --git a/integration/sync_test.go b/integration/sync_test.go index 84e0f53..d51ffee 100644 --- a/integration/sync_test.go +++ b/integration/sync_test.go @@ -21,10 +21,10 @@ func TestExecutableSyncPushesThenUpdates(t *testing.T) { initRepo(t, env, downstream) writeFile(t, downstream, "README.md", "downstream\n") commitAll(t, env, downstream, "seed downstream") - add := runBraid(t, env, downstream, braid, "--quiet", "add", upstream, "vendor/basic") + add := runBraid(t, env, downstream, braid, "--quiet", "add", upstream, "vendor/basic", "--sync-push") assertResult(t, add, 0, "", "") assertConfigRaw(t, downstream, map[string]configMirror{ - "vendor/basic": {URL: upstream, Branch: "main", Revision: baseRevision}, + "vendor/basic": {URL: upstream, Branch: "main", Revision: baseRevision, SyncPush: true}, }) writeFile(t, downstream, "vendor/basic/README.md", "local\n") @@ -46,7 +46,7 @@ func TestExecutableSyncPushesThenUpdates(t *testing.T) { pushedRevision := gitOutput(t, env, upstream, "rev-parse", "HEAD") assertLatestCommit(t, env, upstream, defaultName+" <"+defaultEmail+">", "Executable sync") assertConfigRaw(t, downstream, map[string]configMirror{ - "vendor/basic": {URL: upstream, Branch: "main", Revision: pushedRevision}, + "vendor/basic": {URL: upstream, Branch: "main", Revision: pushedRevision, SyncPush: true}, }) assertLatestCommit(t, env, downstream, defaultName+" <"+defaultEmail+">", "Braid: Update source 'upstream' to '"+shortRevision(pushedRevision)+"'") assertNoRemote(t, env, downstream, remoteName("main", "upstream")) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 33d9ff2..a7bcd09 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -42,6 +42,7 @@ type AddOptions struct { Revision string NoCommit bool PartialClone bool + SyncPush bool } type MirrorMapping struct { @@ -354,6 +355,7 @@ func parseAdd(args []string, options *AddOptions) error { options.Revision = parsed.value("--revision") options.NoCommit = parsed.has("--no-commit") options.PartialClone = parsed.has("--partial-clone") + options.SyncPush = parsed.has("--sync-push") if strings.HasPrefix(positionals[0], ":") { options.ExistingSource = strings.TrimPrefix(positionals[0], ":") if options.ExistingSource == "" { diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 45a860d..60e4e82 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -17,7 +17,7 @@ func TestParseCommands(t *testing.T) { }{ { name: "add branch mirror", - args: []string{"--verbose", "--global-cache-dir", ".cache", "add", "https://example.test/repo.git", "vendor/repo=lib", "--branch", "main", "--no-commit"}, + args: []string{"--verbose", "--global-cache-dir", ".cache", "add", "https://example.test/repo.git", "vendor/repo=lib", "--branch", "main", "--no-commit", "--sync-push"}, want: Invocation{ Global: GlobalOptions{GlobalCacheDir: ".cache", GlobalCacheDirSet: true, Verbose: true}, Command: CommandAdd, @@ -26,6 +26,7 @@ func TestParseCommands(t *testing.T) { Mirrors: []MirrorMapping{{LocalPath: "vendor/repo", UpstreamPath: "lib"}}, Branch: "main", NoCommit: true, + SyncPush: true, }, }, }, @@ -188,6 +189,9 @@ func TestParseUsageErrors(t *testing.T) { {name: "empty global cache dir", args: []string{"--global-cache-dir=", "version"}, want: "--global-cache-dir requires a non-empty value"}, {name: "existing source needs mirror", args: []string{"add", ":source"}, want: "add to an existing source requires at least one mirror"}, {name: "tag branch conflict", args: []string{"add", "url", "--tag", "v1", "--branch", "main"}, want: "add cannot combine --tag and --branch"}, + {name: "sync push tag conflict", args: []string{"add", "url", "--sync-push", "--tag", "v1"}, want: "add cannot combine --sync-push and --tag"}, + {name: "sync push revision conflict", args: []string{"add", "url", "--sync-push", "--revision", "abc"}, want: "add cannot combine --sync-push and --revision"}, + {name: "existing source sync push", args: []string{"add", ":source", "vendor/new", "--sync-push"}, want: "add to an existing source cannot use --name, --branch, --tag, --revision, --partial-clone, or --sync-push"}, {name: "pull all strategy flag", args: []string{"pull", "--branch", "main"}, want: "pull without local_path cannot use --branch, --tag, or --revision"}, {name: "diff args require separator", args: []string{"diff", "--stat"}, want: "unknown flag for diff: --stat"}, {name: "sync unknown flag", args: []string{"sync", "--branch", "main"}, want: "unknown flag for sync: --branch"}, @@ -314,7 +318,7 @@ func TestUsageDocumentsVerboseAsGlobalOnly(t *testing.T) { if strings.Contains(Usage(), " update") || strings.Contains(Usage(), "\n up ") { t.Fatalf("top-level usage exposes update aliases:\n%s", Usage()) } - if !strings.Contains(Usage(), " sync Push local mirror changes, then pull sources") { + if !strings.Contains(Usage(), " sync Push opted-in local changes, then pull sources") { t.Fatalf("top-level usage missing sync command:\n%s", Usage()) } if !strings.Contains(Usage(), " completion") { @@ -323,7 +327,7 @@ func TestUsageDocumentsVerboseAsGlobalOnly(t *testing.T) { if strings.Contains(Usage(), "__complete") { t.Fatalf("top-level usage exposes hidden completion callback:\n%s", Usage()) } - if got, want := CommandUsage(CommandAdd), "usage: braid add [local_path[=upstream_path]...] [--name ] [--branch|-b ] [--tag|-t ] [--revision|-r ] [--no-commit] [--partial-clone]\n"; got != want { + if got, want := CommandUsage(CommandAdd), "usage: braid add [local_path[=upstream_path]...] [--name ] [--branch|-b ] [--tag|-t ] [--revision|-r ] [--no-commit] [--partial-clone] [--sync-push]\n"; got != want { t.Fatalf("CommandUsage(add) = %q, want %q", got, want) } if got, want := CommandUsage(CommandPull), "usage: braid pull [local_path|:source] [--branch|-b ] [--tag|-t ] [--revision|-r ] [--keep] [--no-commit]\n"; got != want { diff --git a/internal/cli/schema.go b/internal/cli/schema.go index ef747d2..15822cf 100644 --- a/internal/cli/schema.go +++ b/internal/cli/schema.go @@ -143,6 +143,7 @@ var commandSpecs = []CommandSpec{ {Long: "--revision", Short: "-r", ValueName: "revision", UsageValue: "rev", Availability: Availability{ForbiddenPrefix: ":"}}, {Long: "--no-commit"}, {Long: "--partial-clone", Availability: Availability{ForbiddenPrefix: ":"}}, + {Long: "--sync-push", Availability: Availability{ForbiddenPrefix: ":"}}, }, Positionals: []PositionalSpec{ {Name: "url|:source", Usage: "", Required: true, Completion: CompletionFree}, @@ -151,9 +152,11 @@ var commandSpecs = []CommandSpec{ Conflicts: []ConflictSpec{ {Options: []string{"--tag", "--branch"}, Error: "add cannot combine --tag and --branch"}, {Options: []string{"--tag", "--revision"}, Error: "add cannot combine --tag and --revision"}, + {Options: []string{"--sync-push", "--tag"}, Error: "add cannot combine --sync-push and --tag"}, + {Options: []string{"--sync-push", "--revision"}, Error: "add cannot combine --sync-push and --revision"}, }, ConditionalPositionals: []ConditionalPositionalsSpec{{Index: 0, Prefix: ":", Minimum: 2, Error: "add to an existing source requires at least one mirror"}}, - AvailabilityError: "add to an existing source cannot use --name, --branch, --tag, --revision, or --partial-clone", + AvailabilityError: "add to an existing source cannot use --name, --branch, --tag, --revision, --partial-clone, or --sync-push", }, { Command: CommandPull, Name: "pull", Aliases: []string{"update", "up"}, Summary: "Pull one source or every eligible source", @@ -188,7 +191,7 @@ var commandSpecs = []CommandSpec{ Positionals: []PositionalSpec{{Name: "local_path|:source", Usage: "", Required: true, Completion: CompletionMirror}}, }, { - Command: CommandSync, Name: "sync", Summary: "Push local mirror changes, then pull sources", + Command: CommandSync, Name: "sync", Summary: "Push opted-in local changes, then pull sources", Options: []OptionSpec{{Long: "--pull-only"}, {Long: "--autostash"}, {Long: "--keep"}}, Positionals: []PositionalSpec{{Name: "local_path|:source", Usage: "[local_path|:source ...]", Repeatable: true, Completion: CompletionMirror}}, }, diff --git a/internal/clitest/completion_contract.go b/internal/clitest/completion_contract.go index db4afca..2b12fdd 100644 --- a/internal/clitest/completion_contract.go +++ b/internal/clitest/completion_contract.go @@ -152,14 +152,22 @@ func commandCases(spec cli.CommandSpec, commandName string) []CompletionCase { if len(conflict.Options) < 2 { continue } - first, second := conflict.Options[0], conflict.Options[1] - words := []string{commandName, first} - if option, ok := spec.Option(first); ok && option.TakesValue() { - words = append(words, "value") + for _, selected := range conflict.Options { + words := []string{commandName, selected} + if option, ok := spec.Option(selected); ok && option.TakesValue() { + words = append(words, "value") + } + words = append(words, "--") + var unwanted []string + for _, conflicting := range conflict.Options { + if conflicting == selected { + continue + } + option, _ := spec.Option(conflicting) + unwanted = append(unwanted, optionNames(option)...) + } + cases = append(cases, CompletionCase{Name: commandName + " conflict " + selected + " in " + strings.Join(conflict.Options, " "), Words: words, Unwanted: unwanted}) } - words = append(words, "--") - option, _ := spec.Option(second) - cases = append(cases, CompletionCase{Name: commandName + " conflict " + strings.Join(conflict.Options, " "), Words: words, Unwanted: optionNames(option)}) } return cases } diff --git a/internal/command/add.go b/internal/command/add.go index c959b98..7e7285d 100644 --- a/internal/command/add.go +++ b/internal/command/add.go @@ -76,7 +76,7 @@ func (h AddHandler) add(ctx context.Context, repo RepoContext, git AddGit, inv c } else if addOptions.Tag != "" { tracking = source.TagTracking{Tag: addOptions.Tag} } - s = source.Source{Name: name, URL: source.CleanURL(addOptions.URL), Tracking: tracking, Revision: addOptions.Revision, PartialClone: addOptions.PartialClone} + s = source.Source{Name: name, URL: source.CleanURL(addOptions.URL), Tracking: tracking, Revision: addOptions.Revision, PartialClone: addOptions.PartialClone, SyncPush: addOptions.SyncPush} } requested := addOptions.Mirrors if len(requested) == 0 { diff --git a/internal/command/add_test.go b/internal/command/add_test.go index 1cbd837..48f10ca 100644 --- a/internal/command/add_test.go +++ b/internal/command/add_test.go @@ -36,6 +36,19 @@ func TestAddCommandDefaultBranchCommitsAndRemovesRemote(t *testing.T) { assertClean(t, repo) } +func TestAddCommandEnablesSyncPush(t *testing.T) { + upstream := testutil.InitRepo(t) + testutil.WriteFile(t, upstream, "README.md", "upstream\n") + testutil.CommitAll(t, upstream, "upstream") + repo := initDownstream(t) + + runCommandOK(t, repo, []string{"add", upstream, "vendor/basic", "--sync-push"}) + + if m := loadMirror(t, repo, "vendor/basic"); !m.SyncPush { + t.Fatalf("mirror = %#v, want sync push enabled", m) + } +} + func TestAddCommandPreservesUnrelatedIndexAndWorktreeState(t *testing.T) { upstream := testutil.InitRepo(t) testutil.WriteFile(t, upstream, "README.md", "hello from upstream\n") diff --git a/internal/command/completion_test.go b/internal/command/completion_test.go index 00c0de1..c94440b 100644 --- a/internal/command/completion_test.go +++ b/internal/command/completion_test.go @@ -80,6 +80,7 @@ func TestCompleteCommandOptions(t *testing.T) { candidates = completeCandidates(t, dir, "add", "") assertCandidate(t, candidates, "--no-commit") + assertCandidate(t, candidates, "--sync-push") candidates = completeCandidates(t, dir, "add", ":replicant", "--") assertCandidate(t, candidates, "--no-commit") assertNoCandidate(t, candidates, "--name") @@ -87,6 +88,7 @@ func TestCompleteCommandOptions(t *testing.T) { assertNoCandidate(t, candidates, "--tag") assertNoCandidate(t, candidates, "--revision") assertNoCandidate(t, candidates, "--partial-clone") + assertNoCandidate(t, candidates, "--sync-push") candidates = completeCandidates(t, dir, "push", "") assertCandidate(t, candidates, "--message") diff --git a/internal/command/sync.go b/internal/command/sync.go index 6eae394..d94d8ae 100644 --- a/internal/command/sync.go +++ b/internal/command/sync.go @@ -84,11 +84,17 @@ func (h SyncHandler) Run(inv cli.Invocation, stdout, stderr io.Writer) error { var updateConflict bool var pushedSources []string if !inv.Sync.PullOnly { - if err := h.hydrateMissingRecordedRevisions(ctx, git, cache, targets, inv.Sync.Keep, inv.Global.Verbose, progress, stderr); err != nil { + pushTargets := make([]syncTarget, 0, len(targets)) + for _, target := range targets { + if target.Mirror.SyncPush { + pushTargets = append(pushTargets, target) + } + } + if err := h.hydrateMissingRecordedRevisions(ctx, git, cache, pushTargets, inv.Sync.Keep, inv.Global.Verbose, progress, stderr); err != nil { runErr = err } if runErr == nil { - plan, err := h.buildPushPlan(ctx, git, cache, targets, inv.Sync.Keep, inv.Global.Verbose, progress, stderr) + plan, err := h.buildPushPlan(ctx, git, cache, pushTargets, inv.Sync.Keep, inv.Global.Verbose, progress, stderr) if err != nil { runErr = err } else { @@ -307,9 +313,6 @@ func (h SyncHandler) buildPushPlan(ctx context.Context, git PushGit, cache Cache if !changed { continue } - if target.Mirror.Branch() == "" { - return syncPushPlan{}, syncNonBranchLocalChangeError(target.LocalPath) - } actions = append(actions, syncPushAction{Target: target, BaseRevision: baseRevision}) } @@ -442,10 +445,6 @@ func syncNotUpToDateError(localPath string) error { return fmt.Errorf("sync cannot push %s because the upstream branch is not up to date; run braid pull %s, resolve conflicts if needed, commit, then rerun braid sync", localPath, localPath) } -func syncNonBranchLocalChangeError(localPath string) error { - return fmt.Errorf("sync cannot push committed local changes for non-branch mirror %s; run braid push %s --branch or rerun braid sync --pull-only %s if you only intended to pull", localPath, localPath, localPath) -} - func syncMirrorPathDeletedError(localPath string) error { return fmt.Errorf("sync cannot push deletion of mirror path %s; restore the mirror path, commit, then rerun braid sync", localPath) } diff --git a/internal/command/sync_test.go b/internal/command/sync_test.go index 63920b9..ed0b51a 100644 --- a/internal/command/sync_test.go +++ b/internal/command/sync_test.go @@ -23,7 +23,7 @@ func TestSyncCommandPushesChangedBranchThenUpdates(t *testing.T) { repo := initDownstream(t) testutil.Git(t, repo, "config", "--local", "user.name", "Sync User") testutil.Git(t, repo, "config", "--local", "user.email", "sync@example.invalid") - runCommandOK(t, repo, []string{"add", upstream, "vendor/basic"}) + runCommandOK(t, repo, []string{"add", upstream, "vendor/basic", "--sync-push"}) testutil.WriteFile(t, repo, "vendor/basic/README.md", "local\n") testutil.CommitAll(t, repo, "local mirror change") t.Setenv("GIT_EDITOR", writeEditor(t, "Sync push")) @@ -47,7 +47,7 @@ func TestSyncCommandPushesChangedBranchThenUpdates(t *testing.T) { assertNoGitRemote(t, repo, "main_braid_001") } -func TestSyncCommandProvenanceGuidanceIsPerPushedMirror(t *testing.T) { +func TestSyncCommandPushesOnlyOptedInSources(t *testing.T) { upstreamA := testutil.InitRepo(t) testutil.WriteFile(t, upstreamA, "README.md", "a base\n") testutil.CommitAll(t, upstreamA, "a base") @@ -58,9 +58,34 @@ func TestSyncCommandProvenanceGuidanceIsPerPushedMirror(t *testing.T) { testutil.Git(t, upstreamB, "config", "receive.denyCurrentBranch", "updateInstead") repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a"}) + runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a", "--sync-push"}) runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b"}) testutil.WriteFile(t, repo, "vendor/a/README.md", "a local\n") + testutil.WriteFile(t, repo, "vendor/b/README.md", "b local\n") + testutil.CommitAll(t, repo, "local mirror changes") + t.Setenv("GIT_EDITOR", writeEditor(t, "Sync opted in source")) + + runCommandOK(t, repo, []string{"sync", "vendor/a", "vendor/b"}) + + assertFile(t, upstreamA, "README.md", "a local\n") + assertCommitSubject(t, upstreamA, "Sync opted in source") + assertFile(t, upstreamB, "README.md", "b base\n") +} + +func TestSyncCommandProvenanceGuidanceIsPerPushedMirror(t *testing.T) { + upstreamA := testutil.InitRepo(t) + testutil.WriteFile(t, upstreamA, "README.md", "a base\n") + testutil.CommitAll(t, upstreamA, "a base") + testutil.Git(t, upstreamA, "config", "receive.denyCurrentBranch", "updateInstead") + upstreamB := testutil.InitRepo(t) + testutil.WriteFile(t, upstreamB, "README.md", "b base\n") + testutil.CommitAll(t, upstreamB, "b base") + testutil.Git(t, upstreamB, "config", "receive.denyCurrentBranch", "updateInstead") + + repo := initDownstream(t) + runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a", "--sync-push"}) + runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b", "--sync-push"}) + testutil.WriteFile(t, repo, "vendor/a/README.md", "a local\n") aCommit := commitAllWithMessage(t, repo, "local a sync") testutil.WriteFile(t, repo, "vendor/b/README.md", "b local\n") bCommit := commitAllWithMessage(t, repo, "local b sync") @@ -100,8 +125,8 @@ func TestSyncCommandGeneratedMessagesArePerPushedMirror(t *testing.T) { testutil.Git(t, upstreamB, "config", "receive.denyCurrentBranch", "updateInstead") repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a"}) - runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b"}) + runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a", "--sync-push"}) + runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b", "--sync-push"}) testutil.WriteFile(t, repo, "vendor/a/README.md", "a local\n") testutil.CommitAll(t, repo, "local a generated") testutil.WriteFile(t, repo, "vendor/b/README.md", "b local\n") @@ -750,33 +775,6 @@ func TestSyncCommandExplicitPrecheckIgnoresDirtyNonSelectedMirror(t *testing.T) } func TestSyncCommandPushPlanValidationPreventsEarlierPush(t *testing.T) { - t.Run("later non branch local change", func(t *testing.T) { - upstreamA := testutil.InitRepo(t) - testutil.WriteFile(t, upstreamA, "README.md", "a base\n") - aBase := testutil.CommitAll(t, upstreamA, "a base") - testutil.Git(t, upstreamA, "config", "receive.denyCurrentBranch", "updateInstead") - upstreamB := testutil.InitRepo(t) - testutil.WriteFile(t, upstreamB, "README.md", "b base\n") - testutil.CommitAll(t, upstreamB, "b base") - testutil.Git(t, upstreamB, "tag", "v1") - - repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a"}) - runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b", "--tag", "v1"}) - testutil.WriteFile(t, repo, "vendor/a/README.md", "a local\n") - testutil.WriteFile(t, repo, "vendor/b/README.md", "b local\n") - testutil.CommitAll(t, repo, "local mirror changes") - t.Setenv("GIT_EDITOR", writeFailingEditor(t)) - - stderr := runCommandError(t, repo, []string{"sync", "vendor/a", "vendor/b"}) - - assertContains(t, stderr, "sync cannot push committed local changes for non-branch mirror vendor/b") - assertContains(t, stderr, "braid push vendor/b --branch ") - if got := testutil.CurrentRevision(t, upstreamA); got != aBase { - t.Fatalf("upstream a revision = %q, want unchanged %q", got, aBase) - } - }) - t.Run("later stale branch", func(t *testing.T) { upstreamA := testutil.InitRepo(t) testutil.WriteFile(t, upstreamA, "README.md", "a base\n") @@ -788,8 +786,8 @@ func TestSyncCommandPushPlanValidationPreventsEarlierPush(t *testing.T) { testutil.Git(t, upstreamB, "config", "receive.denyCurrentBranch", "updateInstead") repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a"}) - runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b"}) + runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a", "--sync-push"}) + runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b", "--sync-push"}) testutil.WriteFile(t, repo, "vendor/a/README.md", "a local\n") testutil.WriteFile(t, repo, "vendor/b/README.md", "b local\n") testutil.CommitAll(t, repo, "local mirror changes") @@ -817,7 +815,7 @@ func TestSyncCommandStopsBeforePullPhaseWhenPushFails(t *testing.T) { bBase := testutil.CommitAll(t, upstreamB, "b base") repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a"}) + runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a", "--sync-push"}) runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b"}) testutil.WriteFile(t, repo, "vendor/a/README.md", "a local\n") testutil.CommitAll(t, repo, "local a") @@ -851,8 +849,8 @@ func TestSyncCommandLaterEditorFailureLeavesEarlierGeneratedPushComplete(t *test testutil.Git(t, upstreamB, "config", "receive.denyCurrentBranch", "updateInstead") repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a"}) - runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b"}) + runCommandOK(t, repo, []string{"add", upstreamA, "vendor/a", "--sync-push"}) + runCommandOK(t, repo, []string{"add", upstreamB, "vendor/b", "--sync-push"}) testutil.WriteFile(t, repo, "vendor/a/README.md", "a local\n") testutil.CommitAll(t, repo, "local a partial") testutil.WriteFile(t, repo, "vendor/b/README.md", "b local\n") @@ -896,40 +894,6 @@ func TestSyncCommandUnchangedMovedBranchUpdatesNormally(t *testing.T) { } } -func TestSyncCommandRejectsSelectedNonBranchLocalChanges(t *testing.T) { - t.Run("no path tag mirror", func(t *testing.T) { - upstream := testutil.InitRepo(t) - testutil.WriteFile(t, upstream, "README.md", "base\n") - testutil.CommitAll(t, upstream, "base") - testutil.Git(t, upstream, "tag", "v1") - - repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstream, "vendor/tagged", "--tag", "v1"}) - testutil.WriteFile(t, repo, "vendor/tagged/README.md", "local\n") - testutil.CommitAll(t, repo, "local tag change") - - stderr := runCommandError(t, repo, []string{"sync"}) - - assertContains(t, stderr, "sync cannot push committed local changes for non-branch mirror vendor/tagged") - }) - - t.Run("explicit revision mirror", func(t *testing.T) { - upstream := testutil.InitRepo(t) - testutil.WriteFile(t, upstream, "README.md", "base\n") - revision := testutil.CommitAll(t, upstream, "base") - - repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstream, "vendor/revision", "--revision", revision}) - testutil.WriteFile(t, repo, "vendor/revision/README.md", "local\n") - testutil.CommitAll(t, repo, "local revision change") - - stderr := runCommandError(t, repo, []string{"sync", "vendor/revision"}) - - assertContains(t, stderr, "sync cannot push committed local changes for non-branch mirror vendor/revision") - assertContains(t, stderr, "braid sync --pull-only vendor/revision") - }) -} - func TestSyncCommandRejectsDeletedSelectedMirrorPath(t *testing.T) { tests := []struct { name string @@ -939,14 +903,14 @@ func TestSyncCommandRejectsDeletedSelectedMirrorPath(t *testing.T) { }{ { name: "directory", - addArgs: func(upstream string) []string { return []string{"add", upstream, "vendor/basic"} }, + addArgs: func(upstream string) []string { return []string{"add", upstream, "vendor/basic", "--sync-push"} }, localPath: "vendor/basic", remove: "vendor/basic", }, { name: "single file", addArgs: func(upstream string) []string { - return []string{"add", upstream, "licenses/THIRD_PARTY.txt=LICENSE.txt"} + return []string{"add", upstream, "licenses/THIRD_PARTY.txt=LICENSE.txt", "--sync-push"} }, localPath: "licenses/THIRD_PARTY.txt", remove: "licenses/THIRD_PARTY.txt", @@ -982,7 +946,7 @@ func TestSyncCommandRemotePathAwareClassification(t *testing.T) { testutil.CommitAll(t, upstream, "base") testutil.Git(t, upstream, "config", "receive.denyCurrentBranch", "updateInstead") repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstream, "vendor/lib=lib"}) + runCommandOK(t, repo, []string{"add", upstream, "vendor/lib=lib", "--sync-push"}) testutil.WriteFile(t, repo, "vendor/lib/component.txt", "local\n") testutil.CommitAll(t, repo, "local subdir") t.Setenv("GIT_EDITOR", writeEditor(t, "Sync subdir")) @@ -1019,7 +983,7 @@ func TestSyncCommandRemotePathAwareClassification(t *testing.T) { testutil.CommitAll(t, upstream, "base") testutil.Git(t, upstream, "config", "receive.denyCurrentBranch", "updateInstead") repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstream, "licenses/THIRD_PARTY.txt=LICENSE.txt"}) + runCommandOK(t, repo, []string{"add", upstream, "licenses/THIRD_PARTY.txt=LICENSE.txt", "--sync-push"}) testutil.WriteFile(t, repo, "licenses/THIRD_PARTY.txt", "local license\n") testutil.CommitAll(t, repo, "local license") t.Setenv("GIT_EDITOR", writeEditor(t, "Sync license")) @@ -1055,7 +1019,7 @@ func TestSyncCommandHydratesMissingRecordedRevision(t *testing.T) { testutil.WriteFile(t, upstream, "README.md", "base\n") revision := testutil.CommitAll(t, upstream, "base") repo := initDownstream(t) - runCommandOK(t, repo, []string{"add", upstream, "vendor/basic"}) + runCommandOK(t, repo, []string{"add", upstream, "vendor/basic", "--sync-push"}) parent := t.TempDir() clone := filepath.Join(parent, "clone") @@ -1079,7 +1043,9 @@ func TestSyncCommandReportsUnavailableRecordedRevisionAfterHydration(t *testing. repo := initDownstream(t) cfg := config.Empty() bogusRevision := strings.Repeat("0", 40) - if err := cfg.AddSource(testSourceMirror("vendor/basic", "", upstream, "main", "", bogusRevision, false).Source); err != nil { + s := testSourceMirror("vendor/basic", "", upstream, "main", "", bogusRevision, false).Source + s.SyncPush = true + if err := cfg.AddSource(s); err != nil { t.Fatalf("add mirror config: %v", err) } if err := cfg.WriteFile(filepath.Join(repo, config.FileName)); err != nil { diff --git a/internal/config/config.go b/internal/config/config.go index 08e67fa..d4e1fbb 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -60,6 +60,7 @@ type readSource struct { Tag string `json:"tag"` Revision string `json:"revision"` PartialClone bool `json:"partial_clone"` + SyncPush bool `json:"sync_push"` Mirrors map[string]json.RawMessage `json:"mirrors"` } type readMirrorV1 struct { @@ -79,6 +80,7 @@ type writeSource struct { Tag string `json:"tag,omitempty"` Revision string `json:"revision"` PartialClone bool `json:"partial_clone,omitempty"` + SyncPush bool `json:"sync_push,omitempty"` Mirrors map[string]string `json:"mirrors"` } @@ -146,7 +148,7 @@ func decodeSource(name string, rs readSource) (source.Source, error) { } mirrors = append(mirrors, source.Mirror{LocalPath: local, UpstreamPath: upstream}) } - return source.Source{Name: name, URL: source.CleanURL(rs.URL), Tracking: tracking, Revision: rs.Revision, PartialClone: rs.PartialClone, Mirrors: mirrors}, nil + return source.Source{Name: name, URL: source.CleanURL(rs.URL), Tracking: tracking, Revision: rs.Revision, PartialClone: rs.PartialClone, SyncPush: rs.SyncPush, Mirrors: mirrors}, nil } func (c Config) SourceNames() []string { @@ -305,6 +307,9 @@ func validateSource(s source.Source) error { if s.Tracking == nil { return errors.New("missing tracking") } + if s.SyncPush && s.Branch() == "" { + return errors.New("sync_push requires branch tracking") + } if len(s.Mirrors) == 0 { return errors.New("missing mirrors") } @@ -338,7 +343,7 @@ func (c Config) MarshalJSON() ([]byte, error) { for _, m := range s.SortedMirrors() { mirrors[m.LocalPath] = m.UpstreamPath } - ws := writeSource{URL: s.URL, Revision: s.Revision, PartialClone: s.PartialClone, Mirrors: mirrors} + ws := writeSource{URL: s.URL, Revision: s.Revision, PartialClone: s.PartialClone, SyncPush: s.SyncPush, Mirrors: mirrors} switch t := s.Tracking.(type) { case source.BranchTracking: ws.Branch = t.Branch diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 832505f..dcc2ba0 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -8,13 +8,13 @@ import ( ) func TestParseAndMarshalCanonicalV2(t *testing.T) { - input := []byte(`{"config_version":2,"sources":{"replicant":{"url":"https://example.test/replicant.git/","branch":"main","revision":"abc","mirrors":{"vendor/replicant":"","licenses/LICENSE":"LICENSE"}}}}`) + input := []byte(`{"config_version":2,"sources":{"replicant":{"url":"https://example.test/replicant.git/","branch":"main","revision":"abc","sync_push":true,"mirrors":{"vendor/replicant":"","licenses/LICENSE":"LICENSE"}}}}`) cfg, err := Parse(input) if err != nil { t.Fatal(err) } s, ok := cfg.SourceByName("replicant") - if !ok || s.URL != "https://example.test/replicant.git" || s.Branch() != "main" { + if !ok || s.URL != "https://example.test/replicant.git" || s.Branch() != "main" || !s.SyncPush { t.Fatalf("source=%#v", s) } data, err := cfg.MarshalJSON() @@ -28,6 +28,7 @@ func TestParseAndMarshalCanonicalV2(t *testing.T) { "url": "https://example.test/replicant.git", "branch": "main", "revision": "abc", + "sync_push": true, "mirrors": { "licenses/LICENSE": "LICENSE", "vendor/replicant": "" @@ -41,6 +42,23 @@ func TestParseAndMarshalCanonicalV2(t *testing.T) { } } +func TestSyncPushDefaultsFalseAndExplicitFalseIsOmitted(t *testing.T) { + cfg, err := Parse([]byte(`{"config_version":2,"sources":{"x":{"url":"u","branch":"main","revision":"r","sync_push":false,"mirrors":{"x":""}}}}`)) + if err != nil { + t.Fatal(err) + } + if cfg.Sources["x"].SyncPush { + t.Fatal("sync push enabled, want disabled") + } + data, err := cfg.MarshalJSON() + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(data), "sync_push") { + t.Fatalf("config contains omitted false sync_push:\n%s", data) + } +} + func TestParseRejectsInvalidV2(t *testing.T) { tests := []struct{ name, json, want string }{ {"obsolete", `{"config_version":2,"mirrors":{}}`, "obsolete unreleased"}, @@ -54,6 +72,8 @@ func TestParseRejectsInvalidV2(t *testing.T) { {"trailing local separator", `{"config_version":2,"sources":{"x":{"url":"u","revision":"r","mirrors":{"x/":""}}}}`, "empty path element"}, {"trailing upstream separator", `{"config_version":2,"sources":{"x":{"url":"u","revision":"r","mirrors":{"x":"upstream/"}}}}`, "empty path element"}, {"tracking conflict", `{"config_version":2,"sources":{"x":{"url":"u","branch":"a","tag":"b","revision":"r","mirrors":{"x":""}}}}`, "both branch and tag"}, + {"tag sync push", `{"config_version":2,"sources":{"x":{"url":"u","tag":"v1","revision":"r","sync_push":true,"mirrors":{"x":""}}}}`, "sync_push requires branch tracking"}, + {"revision sync push", `{"config_version":2,"sources":{"x":{"url":"u","revision":"r","sync_push":true,"mirrors":{"x":""}}}}`, "sync_push requires branch tracking"}, {"overlap", `{"config_version":2,"sources":{"x":{"url":"u","revision":"r","mirrors":{"a":"","a/b":"x"}}}}`, "overlap"}, {"case-fold overlap", `{"config_version":2,"sources":{"x":{"url":"u","revision":"r","mirrors":{"Vendor/Lib":"","vendor/lib/LICENSE":"LICENSE"}}}}`, "case-fold"}, {"trailing JSON", `{"config_version":2,"sources":{}} {}`, "unexpected data"}, diff --git a/internal/source/source.go b/internal/source/source.go index 986791a..5850f30 100644 --- a/internal/source/source.go +++ b/internal/source/source.go @@ -42,6 +42,7 @@ type Source struct { Tracking Tracking Revision string PartialClone bool + SyncPush bool Mirrors []Mirror }