diff --git a/.claude/skills/mendix/teamserver-git/SKILL.md b/.claude/skills/mendix/teamserver-git/SKILL.md index e064d43702..a547ad0084 100644 --- a/.claude/skills/mendix/teamserver-git/SKILL.md +++ b/.claude/skills/mendix/teamserver-git/SKILL.md @@ -14,9 +14,9 @@ description: "Commit, branch and push a Mendix project in git the way Studio Pro | 42-55 | Branches: create them on the server | | 56-101 | Commit from the agent | | 102-116 | Push the branch and the notes together, and soon | -| 117-129 | Never rewrite or merge model history with git | -| 130-140 | Reading what Studio Pro recorded | -| 141-149 | Checklist | +| 117-138 | Never rewrite or merge model history with git | +| 139-149 | Reading what Studio Pro recorded | +| 150-158 | Checklist | # Working with Git on a Mendix Team Server Project @@ -116,11 +116,20 @@ git push origin refs/notes/mx_metadata ## Never rewrite or merge model history with git -- **No `git merge`, `rebase` or `cherry-pick` of model changes.** The `.mpr` is - a SQLite file; git cannot merge it, and a textual merge of `mprcontents/` - leaves an index that does not match the files. Studio Pro has a model-aware - merge: **Version Control › Merge Changes Here** (*Merge feature branch* or - *Cherry Pick*). Studio Pro's history on Team Server is linear for that reason. +- **No plain `git merge`, `rebase` or `cherry-pick` of model changes.** The + `.mpr` is a SQLite file; git cannot merge it (every two-sided model change + is a binary conflict), and a textual merge of `mprcontents/` leaves an + index that does not match the files. Model-aware merging exists in two + forms: **`mxcli git merge `** from the command line (it hands the + conflict to `mx merge` over three project folders — Studio Pro's own + engine, Linux devcontainers included — and commits the resolved model as a + normal merge commit with the note), and **Version + Control › Merge Changes Here** in Studio Pro (*Merge feature branch* or + *Cherry Pick*) — which is also where the conflicts mx cannot auto-resolve + go. A conflict `mxcli git merge` cannot resolve — and an engine failure — + aborts the merge cleanly; nothing is left half-done. + Studio Pro's history on Team Server is linear for a reason: do not "fix" + a conflicted model merge by hand-editing files. - **No `--amend` or force-push of a pushed commit.** Before pushing, an amend is fine: the `notes.rewriteRef` Studio Pro puts in `.git/config` carries the note. Re-run `mxcli git note --write` anyway; it is cheap. diff --git a/CHANGELOG.md b/CHANGELOG.md index fc5801b326..ccf12d5a87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added - **`mxcli git commit -m … -p app.mpr` — commit the way Studio Pro does** (mendixlabs/mxcli#1669) — Studio Pro's *Commit* is more than `git commit`: it keeps the ContentsHash index in step, stages exactly the files a Mendix repository ships, and attaches the mx_metadata note. The new command is that sequence in one line: mismatched hashes are repaired from the files on disk first (`MISSING` and `ORPHAN` entries have no automatic repair and refuse the commit), everything is staged and the files Studio Pro never commits (`project-settings.user.json`, `*.mpr.lock`, `*.mpr.bak`, and the project's own `.mendix-cache/`, `deployment/`, `theme-cache/`, `.mxcli/` beside the `.mpr` — a folder of the same name further down, such as `resources/deployment/`, is committed) are unstaged again, `git commit` runs with the caller's message and identity, and the mx_metadata note is computed from the new commit and attached, so Studio Pro can version-check the revision and show what changed. `--push` pushes the branch and the notes ref together (`git push` alone loses the note the next time Studio Pro fetches); `--dry-run` reports what it would commit without committing — and without repairing anything, hash drift included, because a dry run writes nothing: it stages into a copy of the index, so what the user had staged is left as it was. The commit is refused while Studio Pro has the project open (`.mpr.lock`) — honouring the same `MXCLI_ALLOW_STUDIO_PRO_OPEN=1` override as every write — and during an unresolved merge, which `git add -A` would otherwise commit with its conflict markers. Docs: "Working Outside Studio Pro" › *One commit, all the steps*. +- **`mxcli git merge ` — model-aware merging from the command line** (mendixlabs/mxcli#1669) — `git merge` cannot merge Mendix models: the `.mpr` is SQLite and every `.mxunit` is BSON, so two sides changing the model always meet a binary `CONFLICT (content)` (measured in the git lab: both files, every time). Studio Pro resolves that with a model-aware merge, and the same engine ships as a command — `mx merge BASE MINE THEIRS` over three project folders, which runs on Linux devcontainers too (the older `mx git-merge` file form is Windows-bound). `mxcli git merge ` runs `git merge` first (a merge git resolves itself is annotated like `mxcli git commit`; a merge stopped on text conflicts resumes once those are resolved and staged), and on a model-file conflict checks the base and theirs revisions out into temporary worktrees and hands the three project folders to the engine, mx resolved for the project's Mendix version (PATH, the mxcli cache, the Studio Pro installation): everything the engine auto-resolves is staged, hash-checked and committed as the normal two-parent merge commit with the mx_metadata note — and the engine owns the `.mpr`, so a unit one side added while the other changed existing ones is kept, not dropped; a conflict the engine reports as `ManualResolutionRequired` (exit 2) is a review step, never a silent choice: the merge aborts leaving the working tree exactly as before, and both sides' changes are printed unit by unit at property level (from `mx diff`), with a plan template (`.merge-plan.json`, never staged by `mxcli git commit`) beside the project — the human re-runs with `--plan ` to resolve **per unit** (each conflicted unit takes `ours` or `theirs`, the `.mpr` index is repaired from the mixed files; modified-on-both units only, added/deleted/moved goes to Studio Pro), or with `--ours`/`--theirs` to take one side of every conflicted unit — hash-gated: when the sides' document sets differ (one side added or deleted a unit) the commit is refused and the merge aborted, because a whole side would orphan the other side's documents — or resolves in Studio Pro (*Version Control › Merge Changes Here*), which remains the tool for mixed resolutions; any other engine exit (its 129, an mx that will not start) is reported as the engine failing and the merge is aborted cleanly, never steered into a decision. Non-model conflicts (text sources) are left in place like plain git: resolve them, `git add` them, re-run the command. Refused while Studio Pro has the project open. Docs: "Working Outside Studio Pro" › *Merging branches*. - **`mxcli git doctor [-p app.mpr] [--fix]` — audit a repository for everything a plain-git history misses, and repair it** (mendixlabs/mxcli#1669) — a repository committed only with git drifts from what Studio Pro expects in four ways, each invisible on its own: the local git config Studio Pro writes is absent (`core.autocrlf=False` keeps a checkout from rewriting line endings inside `.mxunit` files on Windows; `mendix.commits-since-gc` and `mendix.lineEndingResetDone` are its own bookkeeping), the remote is SSH which Studio Pro cannot use (HTTPS only), commits have no note under `refs/notes/mx_metadata` (history shows `(unknown)`, the version check fails), and a malformed note Studio Pro cannot read at all. `doctor` reports all four in one read-only pass and exits 1 while an issue remains, so it can gate CI. `--fix` sets the config keys, rewrites SSH remote URLs (`git@host:org/app.git`, `ssh://…`) to HTTPS — judged as configured, so a `url..insteadOf` rule in the user's git config (https → `git@` is a common one) is reported rather than "repaired" on every run — and backfills notes: computed from the model with `-p` (the same engine `git note` uses), or a template note with a `--version` ModelerVersion where the model at a commit cannot be read (MPR v1 history, a revision without the `.mpr`); with neither, the newest valid note in history supplies the version. Notes Studio Pro wrote are never touched, with or without `--fix`; `--fix` does not push. A clone whose notes ref lacks the remote's notes — `git clone` never fetches `refs/notes/*`, so every fresh clone and CI checkout — is reported as exactly that, with the fetch command, rather than as a history of unannotated commits; `--fix` fetches the notes first, as a fast-forward only. Docs: "Working Outside Studio Pro" › *Auditing a repository*. ### Changed diff --git a/cmd/mxcli/cmd_git_commit.go b/cmd/mxcli/cmd_git_commit.go index 3942c276ce..27e575613f 100644 --- a/cmd/mxcli/cmd_git_commit.go +++ b/cmd/mxcli/cmd_git_commit.go @@ -152,16 +152,22 @@ func commitRun(mprPath, message string, push, dryRun bool, out io.Writer) error fmt.Fprintln(out, "nothing to commit, working tree clean") return nil } - fmt.Fprintf(out, "Staged %d file(s):\n", len(staged)-len(junk)) - kept := staged - if len(kept) > 10 { - kept = kept[:10] + kept := make([]string, 0, len(staged)-len(junk)) + for _, f := range staged { + if !isJunkPath(f, projDir) { + kept = append(kept, f) + } } - for _, f := range kept { + fmt.Fprintf(out, "Staged %d file(s):\n", len(kept)) + shown := kept + if len(shown) > 10 { + shown = shown[:10] + } + for _, f := range shown { fmt.Fprintf(out, " %s\n", f) } - if len(staged)-len(junk) > 10 { - fmt.Fprintf(out, " … and %d more\n", len(staged)-len(junk)-10) + if len(kept) > 10 { + fmt.Fprintf(out, " … and %d more\n", len(kept)-10) } if dryRun { @@ -184,16 +190,7 @@ func commitRun(mprPath, message string, push, dryRun bool, out io.Writer) error return fmt.Errorf("committed %s, but attaching the note failed (%v) — run 'mxcli git note -p %s --write' to attach it", shortCommit(head), err, mprPath) } - if push { - if branch == "" { - return errors.New("detached HEAD; cannot push — check out a branch and push manually") - } - if _, err := gitOutIn(root, nil, "push", "origin", branch, mxMetadataRef); err != nil { - return fmt.Errorf("push %s and %s: %w", branch, mxMetadataRef, err) - } - fmt.Fprintf(out, "Pushed %s and %s to origin.\n", branch, mxMetadataRef) - } - return nil + return pushBranchAndNotes(out, root, branch, push) } // indexCopy copies the repository's index to a temporary file, for a @@ -264,6 +261,10 @@ func isJunkPath(rel, projDir string) bool { return true case strings.HasSuffix(strings.ToLower(base), ".mpr.lock"), strings.HasSuffix(strings.ToLower(base), ".mpr.bak"): return true + // `mxcli git merge` writes the plan template beside the .mpr for the + // human to edit; it is working material, never project content. + case strings.HasSuffix(strings.ToLower(base), ".merge-plan.json"): + return true } inProject := f if projDir != "." && projDir != "" { diff --git a/cmd/mxcli/cmd_git_doctor.go b/cmd/mxcli/cmd_git_doctor.go index 06fb78f185..2413ff8700 100644 --- a/cmd/mxcli/cmd_git_doctor.go +++ b/cmd/mxcli/cmd_git_doctor.go @@ -23,8 +23,9 @@ var gitDoctorCmd = &cobra.Command{ Checked: 1. Local git config: core.autocrlf=False (a checkout must not rewrite line - endings inside .mxunit files), mendix.commits-since-gc and - mendix.lineEndingResetDone (Studio Pro's own bookkeeping). --fix sets them. + endings inside .mxunit files; --fix rewrites any other value), and the + mendix.* keys (Studio Pro's own bookkeeping) backfilled when missing — + a value Studio Pro wrote stays. 2. The remote URL, as configured. Studio Pro can only use HTTPS; an SSH remote (git@host:org/app.git or ssh://…) is rewritten to HTTPS by --fix. Other non-HTTPS URLs are reported without being changed. A diff --git a/cmd/mxcli/cmd_git_merge.go b/cmd/mxcli/cmd_git_merge.go new file mode 100644 index 0000000000..3aeecbe6b2 --- /dev/null +++ b/cmd/mxcli/cmd_git_merge.go @@ -0,0 +1,741 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "os/exec" + "path" + "path/filepath" + "strings" + + "github.com/mendixlabs/mxcli/cmd/mxcli/docker" + "github.com/mendixlabs/mxcli/modelsdk/mpr" + "github.com/spf13/cobra" +) + +// cmd_git_merge.go: `mxcli git merge ` — merge a branch the way Studio +// Pro does. `git merge` runs first; when it stops on the model files (the +// .mpr is SQLite and every .mxunit is BSON, so any two-sided model change is +// a binary conflict), the three revisions are handed to `mx merge` — the same +// model-aware engine Studio Pro's own merge uses, over the base and theirs +// checkouts and the working tree — with the project's Mendix version's mx. +// What the engine resolves is staged, hash-checked and committed as a normal +// two-parent merge commit with the mx_metadata note; what it reports as +// ManualResolutionRequired (exit 2) aborts the merge with the conflict list +// and a pointer to Studio Pro, leaving nothing behind; any other non-zero +// exit is the engine failing, reported as such, also with a clean abort. + +var gitMergeCmd = &cobra.Command{ + Use: "merge ", + Short: "Merge a branch with model-aware conflict resolution (mx merge)", + Long: `Merge into the current branch, resolving model conflicts the way +Studio Pro does. + +'git merge ' runs first. A merge git resolves itself (disjoint files, a +fast-forward) is committed and annotated like 'mxcli git commit' does. When +git stops on the model files — the .mpr and the .mxunit units are binary, so +two sides changing the model always conflict — the base and theirs revisions +are checked out into temporary worktrees and the three project folders go to +'mx merge BASE MINE THEIRS' (Studio Pro's own model-aware merge, resolved for +the project's Mendix version): + + - everything the engine auto-resolves is staged, hash-checked and committed + as the merge commit, with the mx_metadata note attached (this keeps both + sides — the usual best outcome; the engine owns the .mpr, so a unit one + side added while the other changed existing ones is kept, not dropped); + - a conflict the engine reports as ManualResolutionRequired (exit 2) is + never merged silently: the merge is aborted, and both sides' changes are + printed for review — at property level, from 'mx diff' (e.g. + "New type > Length: 100 → 150" on ours, "→ 200" on theirs). Then decide: + --plan resolve per unit from the plan template written beside + the .mpr (.merge-plan.json): set "take" to + "ours"/"theirs" per unit; the .mpr index is repaired + from the mixed files. Modified-on-both units only — + added/deleted/moved conflicts go to Studio Pro. + --ours / --theirs take one side of every conflicted unit + (deterministic; mx is not consulted again). The .mpr is + taken from that side too and the result is hash-checked: + when the sides' document sets differ (one added or + deleted a unit), the commit is refused — a whole side + would leave the other side's units orphaned on disk or + listed with no file — and the merge is aborted. Resolve + those in Studio Pro instead. + Studio Pro (Version Control › Merge Changes Here) for mixed + within-unit resolutions — still the tool when a decision + must mix both sides inside one unit; + - any other engine exit is mx failing, not a decision: it is reported and + the merge is aborted cleanly; + - a conflict in non-model files (text sources) is left in place for manual + resolution, like plain git: resolve them, 'git add' them, and re-run + this command — it finishes the merge (model conflicts included) and + commits. + +--ours/--theirs take one side of every conflicted unit — never a side of the +.mpr without the hash check, because the .mpr lists every document: a whole +side would drop the other side's added units and resurrect its deleted ones. + +The merge is refused while Studio Pro has the project open (.mpr.lock). +MPR v2 projects only. The mx binary is resolved for the project's Mendix +version (PATH, the mxcli cache, or the Studio Pro installation).`, + Example: ` mxcli git merge feature/orders -p app.mpr + mxcli git merge origin/main -p app.mpr --push + mxcli git merge feature/orders -p app.mpr --theirs # after reviewing the report + mxcli git merge feature/orders -p app.mpr --plan app.mpr.merge-plan.json`, + Args: cobra.ExactArgs(1), + RunE: runGitMerge, + SilenceUsage: true, + SilenceErrors: true, +} + +func init() { + gitMergeCmd.Flags().StringP("project", "p", "", "path to the Mendix project (.mpr)") + _ = gitMergeCmd.MarkFlagRequired("project") + gitMergeCmd.Flags().StringP("message", "m", "", "merge commit message (default: git's)") + gitMergeCmd.Flags().Bool("ours", false, "on a model conflict mx cannot resolve, take our side of every conflicted model file") + gitMergeCmd.Flags().Bool("theirs", false, "on a model conflict mx cannot resolve, take their side of every conflicted model file") + gitMergeCmd.Flags().String("plan", "", "resolve per unit from a plan file written by a previous run (.merge-plan.json)") + gitMergeCmd.Flags().Bool("push", false, "push the branch and the notes ref to their remote afterwards") + gitCmd.AddCommand(gitMergeCmd) +} + +func runGitMerge(cmd *cobra.Command, args []string) error { + ref := args[0] + message, _ := cmd.Flags().GetString("message") + ours, _ := cmd.Flags().GetBool("ours") + theirs, _ := cmd.Flags().GetBool("theirs") + plan, _ := cmd.Flags().GetString("plan") + push, _ := cmd.Flags().GetBool("push") + project, _ := cmd.Flags().GetString("project") + strategy := "" + switch { + case ours && theirs: + return errors.New("--ours and --theirs cannot be combined") + case ours && plan != "", theirs && plan != "": + return errors.New("--plan cannot be combined with --ours/--theirs: the plan is the finer decision") + case ours: + strategy = "ours" + case theirs: + strategy = "theirs" + } + return mergeRun(project, ref, message, strategy, plan, push, mxRunners{merge: realMxMerge, diff: realMxDiff}, cmd.OutOrStdout()) +} + +// mxRunners are the mx invocations, injected for tests. +// +// merge runs the engine over the three PROJECT FOLDERS — the base checkout, +// the working tree, the theirs checkout — and writes its result into the +// working tree, App.mpr included. Exit 0 is a resolved merge, 2 is a decision +// the engine could not make (its per-unit list is on stdout), anything else +// is the engine failing. +type mxMergeRunner func(baseDir, mineDir, theirsDir string, shas [3]string) (int, string, error) +type mxDiffRunner func(base, mine, outJSON string) error + +type mxRunners struct { + merge mxMergeRunner + diff mxDiffRunner +} + +// mergeRun is the command without its wiring. runners are injected for tests. +// +// Conflict handling is a three-step flow a human stays in charge of: +// +// 1. `git merge` runs; a merge it resolves itself is annotated and done. +// 2. A model conflict goes to `mx merge` over the three project folders +// first — what the engine auto-resolves keeps both sides, which is almost +// always the best outcome — and is committed with the note. +// 3. What the engine reports as ManualResolutionRequired (exit 2) is never +// merged silently: with no strategy the merge is aborted and both sides' +// changes are printed for review; `--plan` then resolves per unit, and +// `--ours`/`--theirs` take one side of every conflicted unit, hash-gated +// so a side that would drop or orphan documents is refused. Any other +// engine exit is a failure, not a decision: reported, aborted cleanly. +func mergeRun(mprPath, ref, message, strategy, plan string, push bool, runners mxRunners, out io.Writer) error { + switch strategy { + case "", "ours", "theirs": + default: + return fmt.Errorf("unknown strategy %q (use --ours or --theirs)", strategy) + } + abs, err := filepath.Abs(mprPath) + if err != nil { + return err + } + if !mpr.AllowWritesWhileStudioProOpen && os.Getenv(mpr.AllowStudioProOpenEnv) != "1" { + if lock, mod := mpr.StudioProLockFile(abs); lock != "" { + return &mpr.StudioProOpenError{MprPath: abs, LockPath: lock, LockTime: mod} + } + } + repo, err := openGitNoteRepo(mprPath) + if err != nil { + return err + } + defer repo.Close() + root := repo.Root + + branch := strings.TrimSpace(gitOutOr(root, "symbolic-ref", "--quiet", "--short", "HEAD")) + if branch == "" { + return errors.New("detached HEAD: check out a branch before merging") + } + + if plan != "" { + return applyPlan(root, repo, abs, mprPath, ref, plan, message, branch, push, out) + } + + // A merge already in progress is resumed, not re-run: a previous run may + // have stopped on text conflicts, which the human resolved and staged + // since. Re-running git merge would only refuse ("not concluded"). + if _, mhErr := gitOut(root, "rev-parse", "--verify", "--quiet", "MERGE_HEAD"); mhErr != nil { + mergeOut, mergeErr := gitOutIn(root, nil, "merge", "--no-edit", ref) + if mergeErr == nil { + if strings.Contains(mergeOut, "Already up to date") { + fmt.Fprintln(out, "Already up to date.") + return nil + } + head := strings.TrimSpace(gitOutOr(root, "rev-parse", "HEAD")) + fmt.Fprintf(out, "Merged %s without model conflicts (%s).\n", ref, shortCommit(head)) + return finishMerge(out, repo, head, branch, push) + } + // A ref that does not resolve fails before any conflict exists. + if _, mhErr := gitOut(root, "rev-parse", "--verify", "--quiet", "MERGE_HEAD"); mhErr != nil { + return fmt.Errorf("git merge %s: %s", ref, firstLineOf(mergeOut)) + } + } + + conflicted := nulFields(gitOutOr(root, "diff", "--name-only", "--diff-filter=U", "-z")) + var model, other []string + for _, p := range conflicted { + if isModelPath(repo, p) { + model = append(model, p) + } else { + other = append(other, p) + } + } + if len(other) > 0 { + fmt.Fprintln(out, "Non-model conflicts, resolve by hand, then 'git add' them and re-run this command to resolve the model and commit:") + for _, p := range other { + fmt.Fprintf(out, " %s\n", p) + } + return errors.New("merge has non-model conflicts left for manual resolution") + } + + baseSha := strings.TrimSpace(gitOutOr(root, "merge-base", "HEAD", "MERGE_HEAD")) + oursSha := strings.TrimSpace(gitOutOr(root, "rev-parse", "HEAD")) + theirsSha := strings.TrimSpace(gitOutOr(root, "rev-parse", "MERGE_HEAD")) + + // The human has decided: take one side of every conflicted model file. mx + // is not consulted — `git checkout --ours/--theirs` reads the conflicted + // stages directly. The .mpr is taken from the side too, and the result is + // hash-checked before committing: a whole side includes the .mpr's list of + // documents, so when the sides' document sets differ (one side added or + // deleted a unit) committing would orphan the other side's units or list + // units with no file. That refusal aborts the merge — it never commits a + // half-project. + if strategy != "" { + side := "--" + strategy + for _, p := range model { + if _, err := gitOutIn(root, nil, "checkout", side, "--", p); err != nil { + return abortMerge(root, fmt.Errorf("take the %s version of %s: %w", strategy, p, err)) + } + } + if err := repairOrRefuse(root, abs, out); err != nil { + return err + } + return commitTheMerge(root, repo, model, message, branch, push, out) + } + + // The engine first: what it auto-resolves keeps both sides, which is + // almost always better than taking one side wholesale. The base and + // theirs revisions are checked out as folders; MINE is the working tree. + // The engine writes its result into the working tree and owns the .mpr — + // its unit list carries both sides' added and deleted documents, so a + // unit one side added while the other changed existing ones is kept, not + // dropped. + tmp, err := os.MkdirTemp("", "mxcli-gitmerge-") + if err != nil { + return err + } + baseDir := filepath.Join(tmp, "base") + theirsDir := filepath.Join(tmp, "theirs") + if _, err := gitOutIn(root, nil, "worktree", "add", "--detach", baseDir, baseSha); err != nil { + os.RemoveAll(tmp) + return fmt.Errorf("check out the merge base: %w", err) + } + if _, err := gitOutIn(root, nil, "worktree", "add", "--detach", theirsDir, theirsSha); err != nil { + _, _ = gitOutIn(root, nil, "worktree", "remove", "--force", baseDir) + os.RemoveAll(tmp) + return fmt.Errorf("check out %s: %w", ref, err) + } + cleanWorktrees := func() { + _, _ = gitOutIn(root, nil, "worktree", "remove", "--force", baseDir) + _, _ = gitOutIn(root, nil, "worktree", "remove", "--force", theirsDir) + _ = os.RemoveAll(tmp) + } + + version := projectMendixVersion(mprPath) + fmt.Fprintf(out, "Model files conflict (%s); merging the model with mx %s…\n", + strings.Join(shortNames(model), ", "), version) + // Folder mode: the engine takes the three project folders and writes its + // result into MINE — the working tree. + projRel := filepath.FromSlash(filepath.Dir(repo.MPRPath)) // "." at the repo root + code, mxOut, runErr := runners.merge( + filepath.Join(baseDir, projRel), + filepath.Dir(abs), + filepath.Join(theirsDir, projRel), + [3]string{baseSha, oursSha, theirsSha}, + ) + if s := strings.TrimSpace(mxOut); s != "" { + fmt.Fprintln(out, s) + } + if runErr != nil { + cleanWorktrees() + return abortMerge(root, fmt.Errorf("mx merge failed to run: %w", runErr)) + } + if code != 0 && code != 2 { + // A crash, an unreadable base, anything but a merge decision: report + // the engine as failing and leave nothing behind. Only exit 2 asks + // for a decision. + cleanWorktrees() + return abortMerge(root, fmt.Errorf("mx merge failed (exit %d); the merge is aborted — if it persists, resolve in Studio Pro (Version Control › Merge Changes Here)", code)) + } + if code == 2 { + // The review step: what each side changed, at property level (mx + // diff), plus a per-unit plan template. Then leave nothing behind — + // the human decides. + if runners.diff != nil { + oursJSON := filepath.Join(tmp, "ours-diff.json") + theirsJSON := filepath.Join(tmp, "theirs-diff.json") + d1 := runners.diff(filepath.Join(baseDir, filepath.FromSlash(repo.MPRPath)), abs, oursJSON) + d2 := runners.diff(filepath.Join(baseDir, filepath.FromSlash(repo.MPRPath)), filepath.Join(theirsDir, filepath.FromSlash(repo.MPRPath)), theirsJSON) + if d1 == nil && d2 == nil { + fmt.Fprintln(out, propertyReport(oursJSON, theirsJSON, conflictedUnitIDs(model))) + } + } + if err := writePlanTemplate(mprPath, baseSha, oursSha, theirsSha, model); err != nil { + fmt.Fprintf(out, " (could not write the plan template: %v)\n", err) + } else { + fmt.Fprintf(out, "Plan template: %s — set \"take\" per unit, re-run with --plan.\n", filepath.Base(mprPath)+".merge-plan.json") + } + cleanWorktrees() + if _, abortErr := gitOutIn(root, nil, "merge", "--abort"); abortErr != nil { + return fmt.Errorf("the model merge has conflicts and aborting it failed (%v); resolve in Studio Pro", abortErr) + } + fmt.Fprintln(out, "Merge aborted; the working tree is back to the pre-merge state.") + return errors.New("the model merge needs a decision: re-run with --plan to resolve per unit, or resolve in Studio Pro (Version Control › Merge Changes Here)") + } + + // Exit 0: the engine resolved the model into the working tree, .mpr + // included — both sides' document sets are in it. Stage, gate, commit. + args := []string{"add", "--"} + args = append(args, model...) + if _, err := gitOutIn(root, nil, args...); err != nil { + cleanWorktrees() + return abortMerge(root, fmt.Errorf("stage the resolved model files: %w", err)) + } + if err := repairOrRefuse(root, abs, out); err != nil { + cleanWorktrees() + return err + } + if err := commitTheMerge(root, repo, nil, message, branch, push, out); err != nil { + cleanWorktrees() + return err + } + cleanWorktrees() + return nil +} + +// abortMerge backs a failed resolution out entirely — working tree, index and +// MERGE_HEAD return to the pre-merge state — and returns the original error. +// Every refusal on this command ends here: a half-done merge makes Studio Pro +// re-run it (and choke) every time the project opens. +func abortMerge(root string, err error) error { + if _, abortErr := gitOutIn(root, nil, "merge", "--abort"); abortErr != nil { + return fmt.Errorf("%w (aborting the merge failed too: %v — resolve it in Studio Pro)", err, abortErr) + } + return err +} + +// repairOrRefuse is the hash gate every side- or plan-based resolution ends +// with: ContentsHash values are repaired from the files on disk (the files +// are the truth once units have been taken from both sides), and a MISSING or +// ORPHAN entry — a unit the .mpr lists with no file behind it, or a file no +// .mpr entry lists — refuses the commit: committing it would drop or orphan +// documents. The refusal aborts the merge. +func repairOrRefuse(root, mprPath string, out io.Writer) error { + err := fixHashes(mprPath, true, out) + if err == nil { + return nil + } + if !errors.Is(err, errHashIssuesRemain) { + return abortMerge(root, err) + } + return abortMerge(root, errors.New("refusing to commit: the ContentsHash index has MISSING or ORPHAN entries (see above) — the sides' document sets differ, and committing would drop or orphan units; resolve in Studio Pro (Version Control › Merge Changes Here)")) +} + +// commitTheMerge commits the resolved merge (MERGE_HEAD is present), writes +// the note and pushes. stagedPaths, when given, are staged first. +func commitTheMerge(root string, repo *gitNoteRepo, stagedPaths []string, message, branch string, push bool, out io.Writer) error { + if stagedPaths != nil { + args := []string{"add", "--"} + args = append(args, stagedPaths...) + if _, err := gitOutIn(root, nil, args...); err != nil { + return fmt.Errorf("stage the resolved model files: %w", err) + } + } + commitArgs := []string{"commit", "--no-edit"} + if message != "" { + commitArgs = []string{"commit", "-m", message} + } + if _, err := gitOutIn(root, nil, commitArgs...); err != nil { + return fmt.Errorf("commit the merge: %w", err) + } + + head := strings.TrimSpace(gitOutOr(root, "rev-parse", "HEAD")) + fmt.Fprintf(out, "Model merge committed as %s.\n", shortCommit(head)) + return finishMerge(out, repo, head, branch, push) +} + +// finishMerge attaches the mx_metadata note to the finished merge and prints +// (or, with push, runs) the branch-and-notes push. +func finishMerge(out io.Writer, repo *gitNoteRepo, head, branch string, push bool) error { + if err := annotateCommits(out, repo, []string{head}, branch, true, false); err != nil { + return fmt.Errorf("committed %s, but attaching the note failed (%v) — run 'mxcli git note -p … --write' to attach it", shortCommit(head), err) + } + return pushBranchAndNotes(out, repo.Root, branch, push) +} + +func pushBranchAndNotes(out io.Writer, root, branch string, push bool) error { + if push { + if branch == "" { + return errors.New("detached HEAD; cannot push — check out a branch and push manually") + } + if _, err := gitOutIn(root, nil, "push", "origin", branch, mxMetadataRef); err != nil { + return fmt.Errorf("push %s and %s: %w", branch, mxMetadataRef, err) + } + fmt.Fprintf(out, "Pushed %s and %s to origin.\n", branch, mxMetadataRef) + return nil + } + if branch != "" { + fmt.Fprintf(out, "Push with: git push origin %s %s\n", branch, mxMetadataRef) + } + return nil +} + +// isModelPath reports whether a repo-relative path is part of the model: +// the .mpr itself or an .mxunit under mprcontents. +func isModelPath(repo *gitNoteRepo, rel string) bool { + f := filepath.ToSlash(rel) + if f == repo.MPRPath { + return true + } + return strings.HasPrefix(f, repo.Contents+"/") && strings.HasSuffix(f, ".mxunit") +} + +// projectMendixVersion reads the project's _ProductVersion for the mx +// resolution; "" on any error (ResolveMxForVersion then falls back to PATH). +func projectMendixVersion(mprPath string) string { + r, err := mpr.Open(mprPath) + if err != nil { + return "" + } + defer r.Close() + v, err := r.GetMendixVersion() + if err != nil { + return "" + } + return v +} + +// realMxMerge shells out to `mx merge BASE MINE THEIRS` over the three +// project folders, mx resolved for the project's Mendix version. +func realMxMerge(baseDir, mineDir, theirsDir string, shas [3]string) (int, string, error) { + version := "" + if matches, _ := filepath.Glob(filepath.Join(mineDir, "*.mpr")); len(matches) > 0 { + version = projectMendixVersion(matches[0]) + } + mxPath, err := docker.ResolveMxForVersion("", version) + if err != nil { + return 1, "", fmt.Errorf("no mx for Mendix %s: %w (model conflicts need it)", version, err) + } + cmd := exec.Command(mxPath, "merge", baseDir, mineDir, theirsDir) + docker.PrepareMxCommand(cmd) + b, err := cmd.CombinedOutput() + code := 0 + if err != nil { + var ee *exec.ExitError + if errors.As(err, &ee) { + code = ee.ExitCode() + } else { + return 1, string(b), err + } + } + return code, string(b), nil +} + +func shortNames(paths []string) []string { + out := make([]string, 0, len(paths)) + for _, p := range paths { + out = append(out, filepath.Base(p)) + } + return out +} + +// conflictedUnitIDs extracts the unit ids of conflicted .mxunit paths. +func conflictedUnitIDs(model []string) []string { + var ids []string + for _, p := range model { + if strings.HasSuffix(p, ".mxunit") { + ids = append(ids, strings.TrimSuffix(filepath.Base(p), ".mxunit")) + } + } + return ids +} + +// --- property-level conflict report and the per-unit plan ------------------- + +type mxDiffChange struct { + Change string `json:"change"` + ChangeDescription string `json:"changeDescription"` + Property string `json:"property"` + BaseValue string `json:"baseValue"` + MineValue string `json:"mineValue"` + ChangeDetails []mxDiffChange `json:"changeDetails"` +} + +type mxDiffUnit struct { + ID string `json:"id"` + Type string `json:"type"` + Change string `json:"change"` + ChangeDetails []mxDiffChange `json:"changeDetails"` +} + +type mxDiffFile struct { + UnitDifferences []mxDiffUnit `json:"unitDifferences"` +} + +// propertyReport renders the two mx diff files as one property-level report +// over the conflicted units: what each side changed. +func propertyReport(oursJSON, theirsJSON string, unitIDs []string) string { + want := map[string]bool{} + for _, id := range unitIDs { + want[id] = true + } + lines := map[string][]string{} + for _, f := range []struct{ side, path string }{{"Yours", oursJSON}, {"Theirs", theirsJSON}} { + raw, err := os.ReadFile(f.path) + if err != nil { + lines[f.side] = append(lines[f.side], fmt.Sprintf(" (diff unavailable: %v)", err)) + continue + } + var d mxDiffFile + if err := json.Unmarshal(bytes.TrimPrefix(raw, []byte("\xef\xbb\xbf")), &d); err != nil { + lines[f.side] = append(lines[f.side], fmt.Sprintf(" (diff unreadable: %v)", err)) + continue + } + var side []string + for _, u := range d.UnitDifferences { + if !want[u.ID] { + continue + } + side = append(side, flattenChanges(u.ChangeDetails, " ")...) + } + if len(side) == 0 { + side = append(side, " (no change details)") + } + lines[f.side] = side + } + var b strings.Builder + b.WriteString("\nProperty-level changes on the conflicted units:\n") + for _, side := range []string{"Yours", "Theirs"} { + fmt.Fprintf(&b, "%s:\n", side) + for _, l := range lines[side] { + b.WriteString(l) + b.WriteString("\n") + } + } + return b.String() +} + +// flattenChanges walks mx diff's change tree and yields one line per leaf: +// the description, and for property leaves the value movement. +func flattenChanges(changes []mxDiffChange, indent string) []string { + var out []string + for _, c := range changes { + if c.Property != "" { + out = append(out, fmt.Sprintf("%s%s: %s → %s", indent, c.Property, c.BaseValue, c.MineValue)) + continue + } + desc := c.ChangeDescription + if desc == "" { + desc = c.Change + } + out = append(out, fmt.Sprintf("%s%s", indent, desc)) + out = append(out, flattenChanges(c.ChangeDetails, indent+" ")...) + } + return out +} + +// mergePlanTemplate is the file a ManualResolutionRequired run writes beside +// the .mpr; the human sets "take" per unit and re-runs with --plan. +type mergePlanTemplate struct { + Base string `json:"base"` + Ours string `json:"ours"` + Theirs string `json:"theirs"` + Units []planUnit `json:"units"` +} + +type planUnit struct { + ID string `json:"id"` + Take string `json:"take"` +} + +func writePlanTemplate(mprPath, baseSha, oursSha, theirsSha string, model []string) error { + tmpl := mergePlanTemplate{Base: baseSha, Ours: oursSha, Theirs: theirsSha} + for _, id := range conflictedUnitIDs(model) { + tmpl.Units = append(tmpl.Units, planUnit{ID: id, Take: "ours"}) + } + b, err := json.MarshalIndent(tmpl, "", " ") + if err != nil { + return err + } + return os.WriteFile(mprPath+".merge-plan.json", b, 0o644) +} + +// applyPlan re-runs the merge unless one is already in progress (it must +// conflict the same way), checks the plan against those revisions, resolves +// per unit, repairs the .mpr index from the mixed files and commits — with +// the note. Every refusal after the merge started aborts it: a half-done +// merge makes Studio Pro re-run it (and choke) on every open. +func applyPlan(root string, repo *gitNoteRepo, mprAbs, mprPath, ref, planPath, message, branch string, push bool, out io.Writer) error { + raw, err := os.ReadFile(planPath) + if err != nil { + return fmt.Errorf("read plan: %w", err) + } + var plan struct { + Base string `json:"base"` + Ours string `json:"ours"` + Theirs string `json:"theirs"` + Units []planUnit `json:"units"` + } + if err := json.Unmarshal(raw, &plan); err != nil { + return fmt.Errorf("plan %s is not valid JSON: %w", planPath, err) + } + + // An unfinished merge (text conflicts resolved and staged, say) is + // resumed; otherwise the merge runs and must conflict the same way. + if _, mhErr := gitOut(root, "rev-parse", "--verify", "--quiet", "MERGE_HEAD"); mhErr != nil { + mergeOut, mergeErr := gitOutIn(root, nil, "merge", "--no-edit", ref) + if mergeErr == nil { + return errors.New("the merge no longer conflicts — the plan is unnecessary; nothing was done") + } + if _, mhErr := gitOut(root, "rev-parse", "--verify", "--quiet", "MERGE_HEAD"); mhErr != nil { + return fmt.Errorf("git merge %s: %s", ref, firstLineOf(mergeOut)) + } + } + baseSha := strings.TrimSpace(gitOutOr(root, "merge-base", "HEAD", "MERGE_HEAD")) + oursSha := strings.TrimSpace(gitOutOr(root, "rev-parse", "HEAD")) + theirsSha := strings.TrimSpace(gitOutOr(root, "rev-parse", "MERGE_HEAD")) + if plan.Base != baseSha || plan.Ours != oursSha || plan.Theirs != theirsSha { + return abortMerge(root, errors.New("stale plan: the merge revisions moved since it was written; delete it and re-run the merge to regenerate")) + } + + conflicted := nulFields(gitOutOr(root, "diff", "--name-only", "--diff-filter=U", "-z")) + var model []string + for _, p := range conflicted { + if isModelPath(repo, p) { + model = append(model, p) + } + } + + // Every planned unit must be modified on both sides; anything else + // (added on one side, deleted, moved) only Studio Pro can decide. + baseSnap, err := repo.snapshot(baseSha) + if err != nil { + return abortMerge(root, err) + } + oursSnap, err := repo.snapshot(oursSha) + if err != nil { + return abortMerge(root, err) + } + theirsSnap, err := repo.snapshot(theirsSha) + if err != nil { + return abortMerge(root, err) + } + oursStatus := changeStatuses(computeModelChanges(baseSnap, oursSnap, nil)) + theirsStatus := changeStatuses(computeModelChanges(baseSnap, theirsSnap, nil)) + take := map[string]string{} + for _, u := range plan.Units { + if u.Take != "ours" && u.Take != "theirs" { + return abortMerge(root, fmt.Errorf("plan: unit %s has take %q (ours|theirs)", shortCommit(u.ID), u.Take)) + } + if oursStatus[u.ID] != "Modified" || theirsStatus[u.ID] != "Modified" { + return abortMerge(root, fmt.Errorf("plan: unit %s is %s on ours and %s on theirs — only modified-on-both conflicts can be planned; resolve it in Studio Pro", shortCommit(u.ID), statusOrNone(oursStatus[u.ID]), statusOrNone(theirsStatus[u.ID]))) + } + take[u.ID] = u.Take + } + + // Ours everywhere, then the planned units from theirs. + for _, p := range model { + if _, err := gitOutIn(root, nil, "checkout", "--ours", "--", p); err != nil { + return abortMerge(root, fmt.Errorf("take the ours version of %s: %w", p, err)) + } + } + for _, id := range conflictedUnitIDs(model) { + if take[id] != "theirs" { + continue + } + p := path.Join(repo.Contents, id[:2], id[2:4], id+".mxunit") + if _, err := gitOutIn(root, nil, "checkout", "--theirs", "--", p); err != nil { + return abortMerge(root, fmt.Errorf("take the theirs version of %s: %w", p, err)) + } + fmt.Fprintf(out, " %s: theirs\n", shortCommit(id)) + } + // The .mpr index is ours; the mixed unit files have different bytes, so + // the hashes are rewritten from the files — files are the truth. The + // gate refuses (and aborts) when the sides' document sets differ: a unit + // theirs added is not in ours' .mpr, and committing would orphan it. + if err := repairOrRefuse(root, mprPath, out); err != nil { + return err + } + + args := []string{"add", "--"} + args = append(args, model...) + if _, err := gitOutIn(root, nil, args...); err != nil { + return abortMerge(root, fmt.Errorf("stage the resolved model files: %w", err)) + } + return commitTheMerge(root, repo, nil, message, branch, push, out) +} + +func changeStatuses(changes []mxModelChange) map[string]string { + out := map[string]string{} + for _, c := range changes { + out[c.UnitID] = c.Status + } + return out +} + +func statusOrNone(s string) string { + if s == "" { + return "unchanged" + } + return s +} + +// realMxDiff shells out to `mx diff BASE MINE OUTPUT` (resolved like the merge). +func realMxDiff(base, mine, outJSON string) error { + version := projectMendixVersion(mine) + mxPath, err := docker.ResolveMxForVersion("", version) + if err != nil { + return fmt.Errorf("no mx for Mendix %s: %w", version, err) + } + cmd := exec.Command(mxPath, "diff", base, mine, outJSON) + docker.PrepareMxCommand(cmd) + if b, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("mx diff: %v: %s", err, strings.TrimSpace(string(b))) + } + return nil +} diff --git a/cmd/mxcli/gitdoctor.go b/cmd/mxcli/gitdoctor.go index 0d106e9f6f..8e8a2df1b8 100644 --- a/cmd/mxcli/gitdoctor.go +++ b/cmd/mxcli/gitdoctor.go @@ -31,11 +31,23 @@ import ( var errDoctorIssuesRemain = errors.New("issues remain (re-run with --fix to repair)") -// mxConfigKeys is the local git config Studio Pro writes and expects. -var mxConfigKeys = [][2]string{ - {"core.autocrlf", "False"}, - {"mendix.commits-since-gc", "0"}, - {"mendix.lineEndingResetDone", "True"}, +// mxConfigKey is one entry of the local git config Studio Pro writes. +// fallback is what a missing key is backfilled with. presentValuePasses marks +// the keys whose value is Studio Pro's own bookkeeping — measured on a real +// checkout (the mx-git-lab lab, S0), commits-since-gc counts up between gc +// runs and lineEndingResetDone comes back in any case, so only a MISSING key +// is an issue there. core.autocrlf is judged: true (or input) makes a +// checkout rewrite line endings inside .mxunit files on Windows. +type mxConfigKey struct { + key string + fallback string + presentValuePasses bool +} + +var mxConfigKeys = []mxConfigKey{ + {"core.autocrlf", "False", false}, + {"mendix.commits-since-gc", "0", true}, + {"mendix.lineEndingResetDone", "True", true}, } var ( @@ -52,25 +64,32 @@ func doctorRun(root, mprPath, remoteOverride, version string, fix bool, out io.W found, fixed := 0, 0 fmt.Fprintln(out, "Git local config") - for _, kv := range mxConfigKeys { - key, want := kv[0], kv[1] + for _, ck := range mxConfigKeys { + key, want := ck.key, ck.fallback current := gitConfigLocal(root, key) - if strings.EqualFold(current, want) { - fmt.Fprintf(out, " [x] %s = %s\n", key, current) - continue - } - found++ - if current == "" { + switch { + case current == "": + found++ fmt.Fprintf(out, " [ ] %s — missing (expected %s)\n", key, want) - } else { + if fix { + if _, err := gitOut(root, "config", "--local", key, want); err != nil { + return fmt.Errorf("set %s: %w", key, err) + } + fixed++ + fmt.Fprintf(out, " set to %s\n", want) + } + case ck.presentValuePasses || strings.EqualFold(current, "false"): + fmt.Fprintf(out, " [x] %s = %s\n", key, current) + default: + found++ fmt.Fprintf(out, " [ ] %s = %s (expected %s)\n", key, current, want) - } - if fix { - if _, err := gitOut(root, "config", "--local", key, want); err != nil { - return fmt.Errorf("set %s: %w", key, err) + if fix { + if _, err := gitOut(root, "config", "--local", key, want); err != nil { + return fmt.Errorf("set %s: %w", key, err) + } + fixed++ + fmt.Fprintf(out, " set to %s\n", want) } - fixed++ - fmt.Fprintf(out, " set to %s\n", want) } } diff --git a/cmd/mxcli/gitdoctor_test.go b/cmd/mxcli/gitdoctor_test.go index 5457a1b31b..bfc2eef324 100644 --- a/cmd/mxcli/gitdoctor_test.go +++ b/cmd/mxcli/gitdoctor_test.go @@ -95,6 +95,44 @@ func TestGitDoctor_FixWritesConfigAndPasses(t *testing.T) { } } +// Measured on a real Studio Pro checkout (the mx-git-lab lab, S0): Studio Pro +// writes core.autocrlf=false in lowercase, counts commits-since-gc up between +// garbage collections (2 after two commits) and writes lineendingresetdone in +// lowercase. A healthy Studio Pro checkout must pass untouched. +func TestGitDoctor_StudioProValuesPass(t *testing.T) { + dir := t.TempDir() + doctorEnv(t, dir) + gitIn(t, dir, "init", "-q", "-b", "main") + gitIn(t, dir, "config", "--local", "core.autocrlf", "false") + gitIn(t, dir, "config", "--local", "mendix.commits-since-gc", "2") + gitIn(t, dir, "config", "--local", "mendix.lineendingresetdone", "true") + + var out bytes.Buffer + if err := doctorRun(dir, "", "", "", false, &out); err != nil { + t.Fatalf("a healthy Studio Pro checkout must pass: %v\n%s", err, out.String()) + } + if strings.Contains(out.String(), "mendix.commits-since-gc — ") { + t.Errorf("the gc counter must not be judged:\n%s", out.String()) + } +} + +// core.autocrlf=true is the one config value that damages a Mendix checkout: +// a checkout rewrites line endings inside .mxunit files on Windows. +func TestGitDoctor_AutoCRLFTrueIsFixed(t *testing.T) { + dir := t.TempDir() + doctorEnv(t, dir) + gitIn(t, dir, "init", "-q", "-b", "main") + gitIn(t, dir, "config", "--local", "core.autocrlf", "true") + + var out bytes.Buffer + if err := doctorRun(dir, "", "", "", true, &out); err != nil { + t.Fatalf("--fix must repair autocrlf=true: %v\n%s", err, out.String()) + } + if got := configLocal(t, dir, "core.autocrlf"); !strings.EqualFold(got, "false") { + t.Errorf("core.autocrlf = %q, want false", got) + } +} + // --- doctor: remote URL scheme ---------------------------------------------- func TestGitDoctor_SSHRemoteRewrittenByFix(t *testing.T) { @@ -153,7 +191,7 @@ func TestGitDoctor_InsteadOfRewriteIsNotAnSSHRemote(t *testing.T) { t.Setenv("GIT_SSH_COMMAND", "false") // the rewritten URL is SSH: never reach the network gitIn(t, dir, "init", "-q", "-b", "main") for _, kv := range mxConfigKeys { - gitIn(t, dir, "config", "--local", kv[0], kv[1]) + gitIn(t, dir, "config", "--local", kv.key, kv.fallback) } gitIn(t, dir, "config", "--local", "url.git@git.example.com:.insteadOf", "https://git.example.com/") gitIn(t, dir, "remote", "add", "origin", "https://git.example.com/org/app.git") @@ -197,10 +235,20 @@ func TestGitDoctor_ConfiguredSSHRemoteFoundBehindRewrite(t *testing.T) { // doctorNoteRepo makes a two-commit repository holding an MPR v2 project: // the base commit, then a page retitle. Returns dir and the .mpr path. +// +// The fixture's git is hermetic: no system or global config leaks in from the +// machine this runs on (runner images, developer setups and CI images differ), +// bytes are committed exactly as written, and no repository above the temp +// dir is discovered. Without this, git-merge fixtures see a different merge +// on each machine and fail with the merge's own evidence thrown away. func doctorNoteRepo(t *testing.T) (string, string) { t.Helper() dir := t.TempDir() + t.Setenv("GIT_CONFIG_GLOBAL", os.DevNull) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") + t.Setenv("GIT_CEILING_DIRECTORIES", filepath.Dir(dir)) gitIn(t, dir, "init", "-q", "-b", "feature") + gitIn(t, dir, "config", "core.autocrlf", "false") units := baseUnits(t) writeProject(t, dir, "11.13.0", units) gitIn(t, dir, "add", "-A") @@ -284,7 +332,7 @@ func TestGitDoctor_UnfetchedNotesAreNotMissing(t *testing.T) { clone := filepath.Join(t.TempDir(), "clone") gitIn(t, filepath.Dir(clone), "clone", "-q", "-b", "feature", remote, clone) for _, kv := range mxConfigKeys { - gitIn(t, clone, "config", "--local", kv[0], kv[1]) + gitIn(t, clone, "config", "--local", kv.key, kv.fallback) } mprPath := filepath.Join(clone, "App.mpr") head := strings.TrimSpace(gitOutOr(clone, "rev-parse", "HEAD")) @@ -747,6 +795,32 @@ func TestGitCommit_DryRunLeavesHashDriftUntouched(t *testing.T) { } } +// The staged-files summary must list what the commit will hold, not the junk +// that was just unstaged (measured on the mx-git-lab lab: the list opened with +// 10 .mxcli/ paths while the commit held 2 model files). +func TestGitCommit_StagedListOmitsJunk(t *testing.T) { + dir, mprPath := doctorNoteRepo(t) + doctorEnv(t, dir) + units := baseUnits(t) + units[tPage] = tUnit{tFolder, unitDoc(t, "Forms$Page", "Home", bson.E{Key: "Title", Value: "List"})} + writeProject(t, dir, "11.13.0", units) + if err := os.WriteFile(filepath.Join(dir, "project-settings.user.json"), []byte("{}"), 0o644); err != nil { + t.Fatal(err) + } + + var out bytes.Buffer + if err := commitRun(mprPath, "clean list", false, false, &out); err != nil { + t.Fatalf("commitRun: %v\n%s", err, out.String()) + } + section := out.String()[strings.Index(out.String(), "Staged"):] + if strings.Contains(section, "project-settings.user.json") { + t.Errorf("the staged list names junk that was left out:\n%s", section) + } + if !strings.Contains(section, "App.mpr") { + t.Errorf("the staged list must name the model files:\n%s", section) + } +} + func TestGitCommit_DryRunWithPushRefused(t *testing.T) { dir, mprPath := doctorNoteRepo(t) doctorEnv(t, dir) diff --git a/cmd/mxcli/gitmerge_test.go b/cmd/mxcli/gitmerge_test.go new file mode 100644 index 0000000000..22ec07ba95 --- /dev/null +++ b/cmd/mxcli/gitmerge_test.go @@ -0,0 +1,629 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "go.mongodb.org/mongo-driver/v2/bson" +) + +func bsonKeyValue(key, value string) bson.E { + return bson.E{Key: key, Value: value} +} + +// `mxcli git merge ` — merge a branch with model-aware resolution: git +// merge first, and on a model-file conflict `mx merge` (the same engine +// Studio Pro uses). The mx run is injected as runMx so the orchestration is +// testable without a Mendix installation; the real runner shells out to mx. + +// modelConflictRepo makes a repository whose merge of "side" into "feature" +// conflicts on both model files: each side changed the same DomainModel unit. +func modelConflictRepo(t *testing.T) (string, string) { + t.Helper() + dir, mprPath := doctorNoteRepo(t) + units := baseUnits(t) + units[tDM] = tUnit{tMod, unitDoc(t, "DomainModels$DomainModel", "", mainSideKey)} + writeProject(t, dir, "11.13.0", units) + gitIn(t, dir, "add", "-A") + gitIn(t, dir, "commit", "-q", "-m", "main side") + gitIn(t, dir, "switch", "-q", "-c", "side", "HEAD~1") + side := baseUnits(t) + side[tDM] = tUnit{tMod, unitDoc(t, "DomainModels$DomainModel", "", sideKey)} + writeProject(t, dir, "11.13.0", side) + gitIn(t, dir, "add", "-A") + gitIn(t, dir, "commit", "-q", "-m", "side change") + gitIn(t, dir, "switch", "-q", "feature") + mergeOut, err := gitInE(t, dir, "merge", "side") + if err != nil && !strings.Contains(err.Error(), "exit status") { + t.Fatalf("fixture: expected a conflicted merge, got %v\n%s", err, mergeOut) + } + if uu := gitOutT(t, dir, "diff", "--name-only", "--diff-filter=U"); !strings.Contains(uu, ".mxunit") { + t.Fatalf("fixture: expected unresolved model files, got %q\nmerge said:\n%s", uu, mergeOut) + } + return dir, mprPath +} + +// twoUnitConflictRepo is a modelConflictRepo where BOTH the domain model and +// the microflow unit were changed on each side — two conflicted units, so a +// plan can take one unit from ours and the other from theirs. +func twoUnitConflictRepo(t *testing.T) (string, string) { + t.Helper() + dir, mprPath := doctorNoteRepo(t) + units := baseUnits(t) + units[tDM] = tUnit{tMod, unitDoc(t, "DomainModels$DomainModel", "", mainSideKey)} + units[tMf] = tUnit{tMod, unitDoc(t, "Microflows$Microflow", "ACT_Save", mainSideKey)} + writeProject(t, dir, "11.13.0", units) + gitIn(t, dir, "add", "-A") + gitIn(t, dir, "commit", "-q", "-m", "main side") + gitIn(t, dir, "switch", "-q", "-c", "side", "HEAD~1") + side := baseUnits(t) + side[tDM] = tUnit{tMod, unitDoc(t, "DomainModels$DomainModel", "", sideKey)} + side[tMf] = tUnit{tMod, unitDoc(t, "Microflows$Microflow", "ACT_Save", sideKey)} + writeProject(t, dir, "11.13.0", side) + gitIn(t, dir, "add", "-A") + gitIn(t, dir, "commit", "-q", "-m", "side change") + gitIn(t, dir, "switch", "-q", "feature") + mergeOut, err := gitInE(t, dir, "merge", "side") + if err != nil && !strings.Contains(err.Error(), "exit status") { + t.Fatalf("fixture: expected a conflicted merge, got %v\n%s", err, mergeOut) + } + return dir, mprPath +} + +// addConflictRepo is a repository where each side ADDED a different unit: only +// the .mpr conflicts, and the sides' document sets differ — the class a whole +// side must refuse, never commit. +func addConflictRepo(t *testing.T) (string, string) { + t.Helper() + dir, mprPath := doctorNoteRepo(t) + ours := baseUnits(t) + oursAdd := "11111111-0000-0000-0000-000000000009" + ours[oursAdd] = tUnit{tMod, unitDoc(t, "Microflows$Microflow", "ACT_Ours")} + writeProject(t, dir, "11.13.0", ours) + gitIn(t, dir, "add", "-A") + gitIn(t, dir, "commit", "-q", "-m", "ours adds a microflow") + gitIn(t, dir, "switch", "-q", "-c", "side", "HEAD~1") + theirs := baseUnits(t) + theirsAdd := "11111111-0000-0000-0000-00000000000a" + theirs[theirsAdd] = tUnit{tMod, unitDoc(t, "Microflows$Microflow", "ACT_Theirs")} + writeProject(t, dir, "11.13.0", theirs) + gitIn(t, dir, "add", "-A") + gitIn(t, dir, "commit", "-q", "-m", "theirs adds a microflow") + gitIn(t, dir, "switch", "-q", "feature") + mergeOut, err := gitInE(t, dir, "merge", "side") + if err != nil && !strings.Contains(err.Error(), "exit status") { + t.Fatalf("fixture: expected a conflicted merge, got %v\n%s", err, mergeOut) + } + if uu := gitOutT(t, dir, "diff", "--name-only", "--diff-filter=U"); !strings.Contains(uu, ".mpr") { + t.Fatalf("fixture: expected the .mpr conflict, got %q\nmerge said:\n%s", uu, mergeOut) + } + return dir, mprPath +} + +// unitRel is the mprcontents path of a unit id inside a work tree. +func unitRel(id string) string { + return "mprcontents/" + id[:2] + "/" + id[2:4] + "/" + id + ".mxunit" +} + +var ( + mainSideKey = bsonKeyValue("DocA", "main") + sideKey = bsonKeyValue("DocB", "side") +) + +func okRunner(out string) mxMergeRunner { + return func(base, mine, theirs string, shas [3]string) (int, string, error) { + return 0, out, nil + } +} + +func TestGitMerge_CleanMergeTextFileWritesNote(t *testing.T) { + dir, mprPath := doctorNoteRepo(t) + doctorEnv(t, dir) + gitIn(t, dir, "branch", "docs", "HEAD~1") + gitIn(t, dir, "switch", "-q", "docs") + if err := os.WriteFile(filepath.Join(dir, "NOTES.txt"), []byte("docs\n"), 0o644); err != nil { + t.Fatal(err) + } + gitIn(t, dir, "add", "NOTES.txt") + gitIn(t, dir, "commit", "-q", "-m", "docs side") + gitIn(t, dir, "switch", "-q", "feature") + + noMx := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + t.Fatal("mx must not run on a merge git resolves itself") + return 0, "", nil + }} + var out bytes.Buffer + if err := mergeRun(mprPath, "docs", "", "", "", false, noMx, &out); err != nil { + t.Fatalf("mergeRun: %v\n%s", err, out.String()) + } + parents := strings.Fields(gitOutT(t, dir, "rev-list", "--parents", "-n", "1", "HEAD")) + if len(parents) != 3 { + t.Fatalf("want a two-parent merge commit, got %v", parents) + } + var n mxNote + if err := json.Unmarshal([]byte(noteOn(t, dir, "HEAD")), &n); err != nil { + t.Fatalf("merge commit has no valid note: %v", err) + } + if !n.HasModelerVersion || n.BranchName != "feature" { + t.Errorf("note header: %+v", n) + } +} + +func TestGitMerge_ModelConflictAutoResolvedByMx(t *testing.T) { + dir, mprPath := modelConflictRepo(t) + doctorEnv(t, dir) + + var gotBase, gotMine, gotTheirs string + var gotShas [3]string + var baseHasMpr, theirsHasMpr bool + runner := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + gotBase, gotMine, gotTheirs, gotShas = base, mine, theirs, shas + // The worktrees are removed when the merge finishes; check inside. + _, baseHasMprErr := os.Stat(filepath.Join(base, "App.mpr")) + _, theirsHasMprErr := os.Stat(filepath.Join(theirs, "App.mpr")) + baseHasMpr, theirsHasMpr = baseHasMprErr == nil, theirsHasMprErr == nil + return 0, "Merge successful.", nil + }} + var out bytes.Buffer + if err := mergeRun(mprPath, "side", "", "", "", false, runner, &out); err != nil { + t.Fatalf("mergeRun: %v\n%s", err, out.String()) + } + // Folder mode: the three args are project folders, and MINE is the + // working tree — the engine writes its result there. + if gotMine != dir { + t.Errorf("runner's mine = %q, want the working project folder %q", gotMine, dir) + } + if !baseHasMpr { + t.Errorf("runner's base folder %q must hold the project's .mpr", gotBase) + } + if !theirsHasMpr { + t.Errorf("runner's theirs folder %q must hold the project's .mpr", gotTheirs) + } + if gotShas[0] == "" || gotShas[1] == gotShas[0] || gotShas[2] == gotShas[1] { + t.Errorf("shas must be base/ours/theirs distinct: %v", gotShas) + } + parents := strings.Fields(gitOutT(t, dir, "rev-list", "--parents", "-n", "1", "HEAD")) + if len(parents) != 3 { + t.Fatalf("want a two-parent merge commit, got %v", parents) + } + uu := gitOutT(t, dir, "diff", "--name-only", "--diff-filter=U") + if strings.TrimSpace(uu) != "" { + t.Errorf("conflicts still unresolved:\n%s", uu) + } + if !strings.Contains(noteOn(t, dir, "HEAD"), `"HasModelerVersion":true`) { + t.Errorf("merge commit has no note:\n%s", noteOn(t, dir, "HEAD")) + } +} + +func TestGitMerge_ModelConflictUnresolvedAborts(t *testing.T) { + dir, mprPath := modelConflictRepo(t) + doctorEnv(t, dir) + before := commitSubject(t, dir) + + fails := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + return 2, `[{"ConflictResolutionDetails":"ManualResolutionRequired"}]`, nil + }} + var out bytes.Buffer + err := mergeRun(mprPath, "side", "", "", "", false, fails, &out) + if err == nil || !strings.Contains(err.Error(), "Studio Pro") { + t.Fatalf("want a Studio Pro referral, got %v", err) + } + if got := commitSubject(t, dir); got != before { + t.Errorf("HEAD moved despite the abort: %q", got) + } + if _, err := gitOut(dir, "rev-parse", "--verify", "--quiet", "MERGE_HEAD"); err == nil { + t.Error("MERGE_HEAD still present: the merge was not aborted") + } + uu := gitOutT(t, dir, "diff", "--name-only", "--diff-filter=U") + if strings.TrimSpace(uu) != "" { + t.Errorf("conflicted files remain:\n%s", uu) + } +} + +func TestGitMerge_NonModelConflictLeftAlone(t *testing.T) { + dir, mprPath := doctorNoteRepo(t) + doctorEnv(t, dir) + gitIn(t, dir, "checkout", "-q", "-b", "side", "HEAD~1") + if err := os.WriteFile(filepath.Join(dir, "NOTES.txt"), []byte("side\n"), 0o644); err != nil { + t.Fatal(err) + } + gitIn(t, dir, "add", "NOTES.txt") + gitIn(t, dir, "commit", "-q", "-m", "side notes") + gitIn(t, dir, "checkout", "-q", "feature") + if err := os.WriteFile(filepath.Join(dir, "NOTES.txt"), []byte("main\n"), 0o644); err != nil { + t.Fatal(err) + } + gitIn(t, dir, "add", "NOTES.txt") + gitIn(t, dir, "commit", "-q", "-m", "main notes") + _, err := gitInE(t, dir, "merge", "side") + if err != nil && !strings.Contains(err.Error(), "exit status") { + t.Fatalf("fixture: expected a conflict, got %v", err) + } + + calls := 0 + counting := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + calls++ + return 0, "", nil + }} + var out bytes.Buffer + merr := mergeRun(mprPath, "side", "", "", "", false, counting, &out) + if merr == nil { + t.Fatal("a non-model conflict must be reported, not silently passed") + } + if calls != 0 { + t.Errorf("mx must not run for non-model conflicts (ran %d times)", calls) + } + uu := gitOutT(t, dir, "diff", "--name-only", "--diff-filter=U") + if !strings.Contains(uu, "NOTES.txt") { + t.Errorf("the conflicted text file must be left for manual resolution:\n%s", uu) + } + gitInE(t, dir, "merge", "--abort") // clean up the fixture +} + +func TestGitMerge_AlreadyUpToDate(t *testing.T) { + dir, mprPath := doctorNoteRepo(t) + doctorEnv(t, dir) + before := commitSubject(t, dir) + + noMx := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + t.Fatal("mx must not run") + return 0, "", nil + }} + var out bytes.Buffer + if err := mergeRun(mprPath, "HEAD~1", "", "", "", false, noMx, &out); err != nil { + t.Fatalf("mergeRun: %v", err) + } + if got := commitSubject(t, dir); got != before { + t.Errorf("HEAD moved: %q", got) + } + if !strings.Contains(out.String(), "up to date") { + t.Errorf("must say so:\n%s", out.String()) + } +} + +// The conflict report is the review material: on ManualResolutionRequired the +// merge is aborted and both sides' changes are printed, so a human can decide +// before anything lands. mx is not even needed for the report. +func TestGitMerge_ConflictReportShowsBothSidesAndAborts(t *testing.T) { + dir, mprPath := modelConflictRepo(t) + doctorEnv(t, dir) + before := commitSubject(t, dir) + + noMx := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + return 2, `[{"ConflictResolutionDetails":"ManualResolutionRequired"}]`, nil + }} + var out bytes.Buffer + err := mergeRun(mprPath, "side", "", "", "", false, noMx, &out) + if err == nil || !strings.Contains(err.Error(), "--plan") { + t.Fatalf("the report must point at the resolution flags, got %v", err) + } + report := out.String() + for _, want := range []string{"Plan template", "merge-plan.json", "Merge aborted"} { + if !strings.Contains(report, want) { + t.Errorf("report must contain %q:\n%s", want, report) + } + } + if got := commitSubject(t, dir); got != before { + t.Errorf("HEAD moved: %q", got) + } + if _, err := gitOut(dir, "rev-parse", "--verify", "--quiet", "MERGE_HEAD"); err == nil { + t.Error("MERGE_HEAD still present") + } + if uu := gitOutT(t, dir, "diff", "--name-only", "--diff-filter=U"); strings.TrimSpace(uu) != "" { + t.Errorf("working tree not restored:\n%s", uu) + } +} + +// --ours / --theirs take the decided side for every conflicted model file, +// commit the merge and write the note — deterministically, without mx. +func TestGitMerge_SideTakingResolves(t *testing.T) { + dir, mprPath := modelConflictRepo(t) + doctorEnv(t, dir) + + noMx := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + t.Fatal("--ours must not consult mx") + return 0, "", nil + }} + var out bytes.Buffer + if err := mergeRun(mprPath, "side", "", "ours", "", false, noMx, &out); err != nil { + t.Fatalf("mergeRun --ours: %v\n%s", err, out.String()) + } + parents := strings.Fields(gitOutT(t, dir, "rev-list", "--parents", "-n", "1", "HEAD")) + if len(parents) != 3 { + t.Fatalf("want a two-parent merge commit, got %v", parents) + } + if !strings.Contains(noteOn(t, dir, "HEAD"), `"HasModelerVersion":true`) { + t.Errorf("merge commit has no note") + } + page := filepath.Join(dir, "mprcontents", tDM[:2], tDM[2:4], tDM+".mxunit") + raw, err := os.ReadFile(page) + if err != nil { + t.Fatal(err) + } + if !bytes.Contains(raw, []byte("DocA")) || bytes.Contains(raw, []byte("DocB")) { + t.Error("--ours must leave our side of the unit on disk") + } + + // --theirs flips to the other side, deterministically. + dir2, mprPath2 := modelConflictRepo(t) + theirs := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + t.Fatal("--theirs must not consult mx") + return 0, "", nil + }} + var out2 bytes.Buffer + if err := mergeRun(mprPath2, "side", "", "theirs", "", false, theirs, &out2); err != nil { + t.Fatalf("mergeRun --theirs: %v\n%s", err, out2.String()) + } + page2 := filepath.Join(dir2, "mprcontents", tDM[:2], tDM[2:4], tDM+".mxunit") + raw2, err := os.ReadFile(page2) + if err != nil { + t.Fatal(err) + } + if !bytes.Contains(raw2, []byte("DocB")) || bytes.Contains(raw2, []byte("DocA")) { + t.Error("--theirs must leave their side of the unit on disk") + } +} + +// A plan resolves per unit: tDM from ours, tMf from theirs — one merge commit, +// hashes repaired from the mixed files, note attached. +func TestGitMerge_PlanMixesPerUnit(t *testing.T) { + dir, mprPath := twoUnitConflictRepo(t) + doctorEnv(t, dir) + base := strings.TrimSpace(gitOutT(t, dir, "merge-base", "HEAD", "side")) + ours := strings.TrimSpace(gitOutT(t, dir, "rev-parse", "HEAD")) + theirs := strings.TrimSpace(gitOutT(t, dir, "rev-parse", "side")) + plan := fmt.Sprintf(`{"base":%q,"ours":%q,"theirs":%q,"units":[{"id":%q,"take":"ours"},{"id":%q,"take":"theirs"}]}`, + base, ours, theirs, tDM, tMf) + planPath := filepath.Join(dir, "merge-plan.json") + if err := os.WriteFile(planPath, []byte(plan), 0o644); err != nil { + t.Fatal(err) + } + + noMx := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + t.Fatal("--plan must not consult mx") + return 0, "", nil + }} + var out bytes.Buffer + if err := mergeRun(mprPath, "side", "", "", planPath, false, noMx, &out); err != nil { + t.Fatalf("mergeRun --plan: %v\n%s", err, out.String()) + } + assertUnitHas := func(id, key string, want bool) { + t.Helper() + raw, err := os.ReadFile(filepath.Join(dir, unitRel(id))) + if err != nil { + t.Fatal(err) + } + if got := bytes.Contains(raw, []byte(key)); got != want { + t.Errorf("%s contains %q = %v, want %v", id, key, got, want) + } + } + assertUnitHas(tDM, "DocA", true) + assertUnitHas(tDM, "DocB", false) + assertUnitHas(tMf, "DocB", true) + assertUnitHas(tMf, "DocA", false) + parents := strings.Fields(gitOutT(t, dir, "rev-list", "--parents", "-n", "1", "HEAD")) + if len(parents) != 3 { + t.Fatalf("want a two-parent merge commit, got %v", parents) + } + var out2 bytes.Buffer + if err := fixHashes(mprPath, false, &out2); err != nil { + t.Errorf("hashes drift after the plan merge: %v\n%s", err, out2.String()) + } +} + +// A plan whose revisions no longer match the merge state is stale and refused. +func TestGitMerge_PlanStaleRefused(t *testing.T) { + dir, mprPath := modelConflictRepo(t) + doctorEnv(t, dir) + plan := `{"base":"0000000000000000000000000000000000000000","ours":"0000000000000000000000000000000000000001","theirs":"0000000000000000000000000000000000000002","units":[]}` + planPath := filepath.Join(dir, "merge-plan.json") + if err := os.WriteFile(planPath, []byte(plan), 0o644); err != nil { + t.Fatal(err) + } + noMx := mxRunners{} + var out bytes.Buffer + err := mergeRun(mprPath, "side", "", "", planPath, false, noMx, &out) + if err == nil || !strings.Contains(err.Error(), "stale") { + t.Fatalf("want a stale-plan refusal, got %v", err) + } + gitInE(t, dir, "merge", "--abort") // clean up the fixture +} + +// On ManualResolutionRequired the report is property-level (from mx diff) and +// a plan template is written beside the .mpr. +func TestGitMerge_ManualWritesPlanAndPropertyReport(t *testing.T) { + dir, mprPath := modelConflictRepo(t) + doctorEnv(t, dir) + fixture := `{"unitDifferences":[{"id":"` + tDM + `","type":"DomainModels$DomainModel","change":"Modified","changeDetails":[{"change":"Modified","changeDescription":"Attribute 'MyFirstModule.CreatedByMxcli.Nickname'","changeDetails":[{"property":"New type > Length","baseValue":"100","mineValue":"150"}]}]}]}` + mx := mxRunners{ + merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + return 2, `[{"ConflictResolutionDetails":"ManualResolutionRequired"}]`, nil + }, + diff: func(base, mine, outJSON string) error { + return os.WriteFile(outJSON, []byte(fixture), 0o644) + }, + } + var out bytes.Buffer + err := mergeRun(mprPath, "side", "", "", "", false, mx, &out) + if err == nil { + t.Fatal("a manual conflict must end with the decision error") + } + report := out.String() + for _, want := range []string{"Length", "100", "150", "Nickname"} { + if !strings.Contains(report, want) { + t.Errorf("report must contain %q:\n%s", want, report) + } + } + planRaw, err := os.ReadFile(mprPath + ".merge-plan.json") + if err != nil { + t.Fatalf("plan template not written: %v", err) + } + var plan struct { + Units []struct { + Id string `json:"id"` + Take string `json:"take"` + } `json:"units"` + } + if err := json.Unmarshal(planRaw, &plan); err != nil { + t.Fatal(err) + } + if len(plan.Units) != 1 || plan.Units[0].Id != tDM || plan.Units[0].Take != "ours" { + t.Errorf("plan template: %+v", plan) + } + gitInE(t, dir, "merge", "--abort") // clean up the fixture +} + +// A whole side of a project includes the .mpr's list of documents. When the +// sides' document sets differ (one side added a unit), taking --theirs would +// leave ours' unit on disk with no .mpr entry: the commit is refused and the +// merge aborted, never silently half-committed. +func TestGitMerge_SideTakingRefusesWhenDocumentSetsDiffer(t *testing.T) { + dir, mprPath := addConflictRepo(t) + doctorEnv(t, dir) + before := commitSubject(t, dir) + + noMx := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + t.Fatal("--theirs must not consult mx") + return 0, "", nil + }} + var out bytes.Buffer + err := mergeRun(mprPath, "side", "", "theirs", "", false, noMx, &out) + if err == nil || !strings.Contains(err.Error(), "refusing to commit") { + t.Fatalf("--theirs must refuse a side that drops ours' unit, got %v\n%s", err, out.String()) + } + if got := commitSubject(t, dir); got != before { + t.Errorf("HEAD moved: %q", got) + } + if _, err := gitOut(dir, "rev-parse", "--verify", "--quiet", "MERGE_HEAD"); err == nil { + t.Error("MERGE_HEAD still present: the refusal must abort the merge") + } + if uu := gitOutT(t, dir, "diff", "--name-only", "--diff-filter=U"); strings.TrimSpace(uu) != "" { + t.Errorf("working tree not restored:\n%s", uu) + } +} + +// Exit 129 is the engine failing, not a merge decision: report it, abort +// cleanly, and never write a plan template. +func TestGitMerge_EngineCrashAborts(t *testing.T) { + dir, mprPath := modelConflictRepo(t) + doctorEnv(t, dir) + before := commitSubject(t, dir) + + crash := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + return 129, "Exception in MergeLib", nil + }} + var out bytes.Buffer + err := mergeRun(mprPath, "side", "", "", "", false, crash, &out) + if err == nil || !strings.Contains(err.Error(), "exit 129") { + t.Fatalf("want the engine-failure report, got %v\n%s", err, out.String()) + } + if strings.Contains(out.String(), "Plan template") { + t.Errorf("a crash must not produce a plan:\n%s", out.String()) + } + if got := commitSubject(t, dir); got != before { + t.Errorf("HEAD moved: %q", got) + } + if _, err := gitOut(dir, "rev-parse", "--verify", "--quiet", "MERGE_HEAD"); err == nil { + t.Error("MERGE_HEAD still present: the merge was not aborted") + } + if uu := gitOutT(t, dir, "diff", "--name-only", "--diff-filter=U"); strings.TrimSpace(uu) != "" { + t.Errorf("working tree not restored:\n%s", uu) + } + if _, err := os.Stat(mprPath + ".merge-plan.json"); !os.IsNotExist(err) { + t.Errorf("a crash must not write a plan template: %v", err) + } +} + +// mx not being found or not starting is the engine failing to run: the merge +// aborts with the reason, and nothing is left behind. +func TestGitMerge_EngineStartFailureAborts(t *testing.T) { + dir, mprPath := modelConflictRepo(t) + doctorEnv(t, dir) + before := commitSubject(t, dir) + + missing := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + return 0, "", errors.New("no mx for Mendix 11.13.0: not on PATH or in the cache (model conflicts need it)") + }} + var out bytes.Buffer + err := mergeRun(mprPath, "side", "", "", "", false, missing, &out) + if err == nil || !strings.Contains(err.Error(), "failed to run") { + t.Fatalf("want the mx failure, got %v\n%s", err, out.String()) + } + if _, err := gitOut(dir, "rev-parse", "--verify", "--quiet", "MERGE_HEAD"); err == nil { + t.Error("MERGE_HEAD still present: the merge was not aborted") + } + if got := commitSubject(t, dir); got != before { + t.Errorf("HEAD moved: %q", got) + } +} + +// A plan whose take value is not ours/theirs is refused, and the refusal +// aborts the merge: nothing may be left half-done. +func TestGitMerge_PlanRefusalAborts(t *testing.T) { + dir, mprPath := modelConflictRepo(t) + doctorEnv(t, dir) + base := strings.TrimSpace(gitOutT(t, dir, "merge-base", "HEAD", "side")) + ours := strings.TrimSpace(gitOutT(t, dir, "rev-parse", "HEAD")) + theirs := strings.TrimSpace(gitOutT(t, dir, "rev-parse", "side")) + plan := fmt.Sprintf(`{"base":%q,"ours":%q,"theirs":%q,"units":[{"id":%q,"take":"sideways"}]}`, + base, ours, theirs, tDM) + planPath := filepath.Join(dir, "merge-plan.json") + if err := os.WriteFile(planPath, []byte(plan), 0o644); err != nil { + t.Fatal(err) + } + noMx := mxRunners{} + var out bytes.Buffer + err := mergeRun(mprPath, "side", "", "", planPath, false, noMx, &out) + if err == nil || !strings.Contains(err.Error(), "ours|theirs") { + t.Fatalf("want the invalid-take refusal, got %v\n%s", err, out.String()) + } + if _, err := gitOut(dir, "rev-parse", "--verify", "--quiet", "MERGE_HEAD"); err == nil { + t.Error("MERGE_HEAD still present: the refusal must abort the merge") + } + if uu := gitOutT(t, dir, "diff", "--name-only", "--diff-filter=U"); strings.TrimSpace(uu) != "" { + t.Errorf("working tree not restored:\n%s", uu) + } +} + +// Re-entry: with the merge already in progress (a previous run stopped and +// the human resolved what they handle by hand), re-running resolves the model +// and finishes — it does not re-run git merge and die on MERGE_HEAD. +func TestGitMerge_ResumesAMergeAlreadyInProgress(t *testing.T) { + dir, mprPath := modelConflictRepo(t) + doctorEnv(t, dir) + + calls := 0 + runner := mxRunners{merge: func(base, mine, theirs string, shas [3]string) (int, string, error) { + calls++ + return 0, "Merge successful.", nil + }} + var out bytes.Buffer + if err := mergeRun(mprPath, "side", "", "", "", false, runner, &out); err != nil { + t.Fatalf("mergeRun: %v\n%s", err, out.String()) + } + if calls != 1 { + t.Errorf("engine runs = %d, want 1", calls) + } + parents := strings.Fields(gitOutT(t, dir, "rev-list", "--parents", "-n", "1", "HEAD")) + if len(parents) != 3 { + t.Fatalf("want a two-parent merge commit, got %v", parents) + } +} + +// The plan template is working material: `mxcli git commit` never stages it. +func TestJunkPath_CoversMergePlanTemplate(t *testing.T) { + if !isJunkPath("App.mpr.merge-plan.json", ".") { + t.Error("App.mpr.merge-plan.json must be junk for staging") + } + if !isJunkPath("app/Project.mpr.merge-plan.json", "app") { + t.Error("a project-folder plan template must be junk too") + } +} diff --git a/docs-site/src/tools/outside-studio-pro.md b/docs-site/src/tools/outside-studio-pro.md index e63779ea77..193301bedb 100644 --- a/docs-site/src/tools/outside-studio-pro.md +++ b/docs-site/src/tools/outside-studio-pro.md @@ -233,6 +233,58 @@ ContentsHash drift, which it does not repair — a dry run writes nothing), and reports what it would commit without committing. It stages into a copy of the index, so whatever you had staged is left exactly as it was. +## Merging branches: `mxcli git merge` + +`git merge` cannot merge Mendix models: the `.mpr` is SQLite and every +`.mxunit` is BSON, so two sides changing the model always meet a binary +`CONFLICT (content)` — there is nothing for git's textual merge to work on. +Studio Pro's *Merge Changes Here* resolves it with a model-aware merge, and +that engine ships as a command: `mx merge BASE MINE THEIRS` over three +project folders (it runs on Linux devcontainers too; the older `mx git-merge` +file form is Windows-bound). + +`mxcli git merge ` puts the two together: + +```bash +mxcli git merge feature/orders -p app.mpr +mxcli git merge origin/main -p app.mpr --push +``` + +`git merge ` runs first; a merge git resolves itself (disjoint files, +a fast-forward) is annotated with the mx_metadata note like `mxcli git +commit` does. On a model-file conflict the base and theirs revisions are +checked out into temporary worktrees and the three project folders go to +`mx merge` (resolved for the project's Mendix version); the engine writes +its result into the working tree and owns the `.mpr`: + +- what the engine auto-resolves is staged, hash-checked and committed as the + normal two-parent merge commit, with the note attached. The engine's + `.mpr` carries both sides' document sets, so a unit one side added while + the other changed existing ones is kept, not dropped; +- a conflict the engine reports as `ManualResolutionRequired` (exit 2) is + never merged silently: the merge is aborted, and **both sides' changes are + printed for review** — at property level, from `mx diff` ("New type > + Length: 100 → 150" on ours, "→ 200" on theirs). Then decide: + `--plan ` resolves **per unit** from the plan template written beside + the .mpr (`.merge-plan.json`, which `mxcli git commit` never stages): + set `"take"` to `ours`/`theirs` per unit, and the `.mpr` index is repaired + from the mixed files (modified-on-both units only; added/deleted/moved + conflicts go to Studio Pro). `--ours` or `--theirs` takes one side of every + conflicted unit, hash-gated: when the sides' document sets differ (one side + added or deleted a unit), committing a whole side would orphan the other + side's documents, so the commit is refused and the merge aborted. Any other + engine exit is mx failing, not a decision: it is reported and the merge is + aborted cleanly. Resolutions that must mix both sides *inside* one unit + stay in Studio Pro (*Version Control › Merge Changes Here*); +- a conflict in non-model files (text sources) is left in place for manual + resolution, like plain git: resolve them, `git add` them, and re-run the + command — it finishes the merge (model conflicts included) and commits. + +Merging is still never done by plain `git merge` on a Mendix repository: use +`mxcli git merge` or Studio Pro, never a textual merge of `mprcontents/`. +Like every mxcli write, the command is refused while Studio Pro has the +project open. + ## Auditing a repository: `mxcli git doctor` A repository that has known only plain git misses several things Studio Pro @@ -247,7 +299,7 @@ mxcli git doctor --fix --version 10.21.0 | Check | `--fix` | |-------|---------| -| local git config: `core.autocrlf=False` (a checkout must not rewrite line endings inside `.mxunit` files), `mendix.commits-since-gc`, `mendix.lineEndingResetDone` | sets them | +| local git config: `core.autocrlf=False` (a checkout must not rewrite line endings inside `.mxunit` files; any other value is rewritten), `mendix.commits-since-gc`, `mendix.lineEndingResetDone` — the `mendix.*` values are Studio Pro's own bookkeeping and are only backfilled when missing | sets/backs them | | the remote URL as configured: Studio Pro can only use HTTPS | rewrites SSH (`git@host:org/app.git`, `ssh://…`) to HTTPS; other non-HTTPS URLs are reported unchanged. A `url..insteadOf` rule in your git config that makes git use another URL is reported, and left alone | | the remote's `refs/notes/mx_metadata` | reported only — push the notes ref together with the branch | | the local `refs/notes/mx_metadata` holds the remote's notes: `git clone` and a plain `git fetch` never fetch them, so a fresh clone or a CI checkout has none | fetches them, as a fast-forward only; refused when the local ref has notes the remote does not | diff --git a/docs/plans/2026-10-30-git-merge-mx-folder-mode.md b/docs/plans/2026-10-30-git-merge-mx-folder-mode.md new file mode 100644 index 0000000000..85de356f7c --- /dev/null +++ b/docs/plans/2026-10-30-git-merge-mx-folder-mode.md @@ -0,0 +1,57 @@ +# Rework `mxcli git merge` per ako's review (PR #1674) + +Decisions (user, 2026-10-30): switch the engine call to `mx merge` (folder mode); +keep `--ours`/`--theirs` but limited to conflicted units with a hash gate; one +push after the rework passes `make ci-local`. + +## Review claims → verified + +1. --ours/--theirs take App.mpr wholesale → orphan/missing documents. TRUE (strategy path, cmd_git_merge.go). +2. --plan takes ours' App.mpr → refuses on theirs' adds; error paths leave MERGE_HEAD. TRUE (applyPlan; abort only on stale). +3. Every non-zero mx exit treated as decision-needed. TRUE (code != 0 → plan + decision error; 129 = crash). +4. mx git-merge broken on Linux ("Git not found", 129). Reviewer-measured; repo finding corroborates mx merge works (0/2/129). +5. Should-fix: upstream merge conflicts (DONE, 2ef7e1068); merge-plan.json not junk-pathed; mixed-conflict advice; PrepareMxCommand nit. + +## Changes + +### A. Engine interface (cmd_git_merge.go) +- `mxMergeRunner func(baseDir, mineDir, theirsDir string, shas [3]string) (int, string, error)` — folders. +- realMxMerge: `mx merge ` + docker.PrepareMxCommand (11.10 LD_PRELOAD nit). +- baseDir/theirsDir: temp worktrees (unchanged); project dir inside = filepath.Join(dir, projDirRel). +- mineDir = the working project dir (filepath.Dir(abs mpr)). + +### B. Exit-code honesty +- 0 → stage model paths → fixHashes gate (MISSING/ORPHAN → refuse + git merge --abort) → commitTheMerge. +- 2 → property report (mx diff) + plan template (units = engine JSON's manual units; fallback git's conflicted ids) + git merge --abort + decision error. +- other (129…) → "mx merge failed (exit N)" + git merge --abort. Never a plan, never a decision suggestion. + +### C. --ours/--theirs limited +- No engine call (unchanged). Conflicted UNITS only from git's list (skip the .mpr). +- App.mpr takes the flag's side; then fixHashes gate: MISSING/ORPHAN → refuse + git merge --abort (loud, Studio Pro pointer). No silent drops. + +### D. --plan aborts everywhere +- invalid JSON / invalid take / not-modified-on-both / fixHashes refusal → git merge --abort before returning. +- stale-plan path already aborts (keep). + +### E. Re-entry +- At entry (non-plan): if MERGE_HEAD already exists, skip `git merge` and go straight to conflict classification. Supports: mixed conflict → resolve text files → `git add` → re-run command finishes the merge. + +### F. Mixed-conflict advice +- Early-return kept, reworded: resolve the text files, `git add` them, then re-run `mxcli git merge ` (it resumes, resolves the model, commits). + +### G. merge-plan.json is junk +- isJunkPath: `.merge-plan.json` never staged by `mxcli git commit`. + +### H. Words +- Long help, CHANGELOG git-merge entry, docs-site outside-studio-pro.md, teamserver-git SKILL.md "aborts the merge cleanly" — match the new reality. + +### I. Tests (gitmerge_test.go) +- Runner fixtures → folder form (assert dirs end in the project folder, contain App.mpr). +- exit 129 → no plan text, MERGE_HEAD gone, tree restored, error names mx. +- --theirs with ours-added unit → refuse + abort (no ORPHAN commit, MERGE_HEAD gone). +- --plan error paths → MERGE_HEAD gone. +- re-entry: staged text resolution + MERGE_HEAD → finishes. +- isJunkPath: x.merge-plan.json → junk. + +## Gate +- go vet ./cmd/mxcli/; focused go test; then make ci-local; then one push.