From 12e06a8699596e5f2a56bd18e3dd4c4190640690 Mon Sep 17 00:00:00 2001 From: Ryan Saunderson Date: Fri, 17 Jul 2026 11:35:37 -0500 Subject: [PATCH] fix(docker): preserve MPRv2 storage format across docker check `mxcli docker check` runs `mx update-widgets` before `mx check` to avoid false CE0463 ("widget definition changed") errors. On an MPRv2 project that step rewrites the project into the self-contained MPRv1 format: it inlines every unit into the .mpr and deletes the mprcontents/ directory. A command named `check` should not change the on-disk storage format. On a project under Git this desyncs the working tree from the tracked mprcontents/ files and can leave Studio Pro unable to open the project (it crashed in its Git version-control provider in the case that motivated this). ## Changes - Detect the storage format before running update-widgets. When the project is MPRv2, snapshot the .mpr and mprcontents/ to a temp directory and restore them after the check. The check still runs against the widget-normalized model, so CE0463 false positives stay suppressed; only the on-disk format is preserved. MPRv1 projects are single-file and left untouched. - If the snapshot cannot be taken, skip update-widgets rather than risk an unrecoverable v2 to v1 conversion. ## Tests - Unit regression test for the snapshot/restore round-trip (deletes mprcontents/ to mimic the conversion, asserts a byte-for-byte restore). - Integration test (build tag `integration`) that scaffolds a real MPRv2 project with `mx create-project`, runs the real `Check()`, and asserts the project is still MPRv2 afterwards. ## Benefits - `docker check` no longer mutates the storage format of the project it checks. - Removes the Git working-tree desync and the Studio Pro open failure that followed from it. ## Statistics - 3 files changed: 1 fix, 2 tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/mxcli/docker/check.go | 89 ++++++++++++++++++++ cmd/mxcli/docker/check_integration_test.go | 81 ++++++++++++++++++ cmd/mxcli/docker/check_test.go | 98 ++++++++++++++++++++++ 3 files changed, 268 insertions(+) create mode 100644 cmd/mxcli/docker/check_integration_test.go diff --git a/cmd/mxcli/docker/check.go b/cmd/mxcli/docker/check.go index e84cd01dc..8696bed3b 100644 --- a/cmd/mxcli/docker/check.go +++ b/cmd/mxcli/docker/check.go @@ -50,6 +50,66 @@ func updateWidgetsPathArg(p string) string { return p } +// snapshotStorageFormat backs up the MPRv2 storage files (.mpr index + mprcontents/) +// to a temp directory and returns a restore function that puts them back, undoing +// any v2 -> v1 conversion performed by an intervening `mx update-widgets`. The +// restore function removes the temp directory and is safe to defer; it best-effort +// restores and never panics. mprPath and contentsDir come from an mpr.Reader on a +// project already known to be MPRv2. +func snapshotStorageFormat(mprPath, contentsDir string) (restore func(), err error) { + tmp, err := os.MkdirTemp("", "mxcli-mpr-snapshot-*") + if err != nil { + return nil, err + } + + mprBackup := filepath.Join(tmp, filepath.Base(mprPath)) + if err := copyFile(mprPath, mprBackup); err != nil { + os.RemoveAll(tmp) + return nil, err + } + + contentsBackup := filepath.Join(tmp, "mprcontents") + if err := copyDir(contentsDir, contentsBackup); err != nil { + os.RemoveAll(tmp) + return nil, err + } + + restore = func() { + defer os.RemoveAll(tmp) + // Restore the v2 index file. + _ = copyFile(mprBackup, mprPath) + // update-widgets deletes mprcontents/; drop whatever is there now (nothing, + // after a conversion) and restore the backed-up tree. + _ = os.RemoveAll(contentsDir) + _ = copyDir(contentsBackup, contentsDir) + } + return restore, nil +} + +// copyFile copies a single file from src to dst, preserving the source file mode. +func copyFile(src, dst string) error { + info, err := os.Stat(src) + if err != nil { + return err + } + in, err := os.Open(src) + if err != nil { + return err + } + defer in.Close() + + out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, info.Mode()) + if err != nil { + return err + } + defer out.Close() + + if _, err := io.Copy(out, in); err != nil { + return err + } + return out.Close() +} + // Check runs 'mx check' on the project to validate it before building. func Check(opts CheckOptions) error { w := opts.Stdout @@ -76,6 +136,35 @@ func Check(opts CheckOptions) error { } fmt.Fprintf(w, "Using mx: %s\n", mxPath) + // `mx update-widgets` rewrites an MPRv2 project into the self-contained MPRv1 + // storage format: it inlines every unit into the .mpr and deletes mprcontents/. + // A `check` must not mutate the on-disk storage format — silently doing so + // desyncs the working tree from a Git repository that tracks the mprcontents/ + // files and can leave Studio Pro unable to open the project. So when the project + // is MPRv2, snapshot the .mpr + mprcontents/ before update-widgets and restore + // them after the check. The check still runs against the widget-normalized model + // (so CE0463 false positives are still suppressed); only the on-disk format is + // preserved. MPRv1 projects are already single-file and need no protection. + if !opts.SkipUpdateWidgets && opts.ProjectPath != "" { + if reader, err := mpr.Open(opts.ProjectPath); err == nil { + isV2 := reader.Version() == mpr.MPRVersionV2 + contentsDir := reader.ContentsDir() + reader.Close() + if isV2 { + restore, snapErr := snapshotStorageFormat(opts.ProjectPath, contentsDir) + if snapErr != nil { + // Can't protect the format — skip update-widgets rather than risk + // an unrecoverable v2 -> v1 conversion. A CE0463 false positive is + // the lesser evil than a silent, unrestorable format change. + fmt.Fprintf(w, "Warning: could not snapshot MPRv2 storage (skipping update-widgets to avoid a v2->v1 conversion): %v\n", snapErr) + opts.SkipUpdateWidgets = true + } else { + defer restore() + } + } + } + } + // Run mx update-widgets to normalize pluggable widget definitions. // This prevents false CE0463 ("widget definition changed") errors caused // by mismatch between widget Object properties and Type PropertyTypes. diff --git a/cmd/mxcli/docker/check_integration_test.go b/cmd/mxcli/docker/check_integration_test.go new file mode 100644 index 000000000..405f7cfe7 --- /dev/null +++ b/cmd/mxcli/docker/check_integration_test.go @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build integration + +package docker + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/mendixlabs/mxcli/sdk/mpr" +) + +// TestCheck_PreservesMPRv2StorageFormat is the end-to-end guard for +// mendixlabs/mxcli#763. `mx update-widgets`, which Check runs before `mx check`, +// rewrites an MPRv2 project into the self-contained MPRv1 format (inlining every +// unit into the .mpr and deleting mprcontents/). Check must leave the on-disk +// storage format exactly as it found it. Both mx 11.9.0 (the version the CI +// integration job installs) and 11.12.1 were observed to perform this conversion, +// so on a v2 project this test genuinely exercises the bug: without the snapshot/ +// restore fix the project would be MPRv1 after Check. +// +// Requires a resolvable mx (provided by the CI integration job's +// `mxcli setup mxbuild`); skips otherwise. Uses `mx create-project`, whose output +// is MPRv2, as the fixture — the same scaffolding the executor roundtrip suite uses. +func TestCheck_PreservesMPRv2StorageFormat(t *testing.T) { + mxPath, err := ResolveMx("") + if err != nil { + t.Skipf("mx not resolvable: %v", err) + } + + // Scaffold a fresh project. `mx create-project` (no template arg) writes App.mpr + // into the working directory and produces MPRv2 storage. + dir := t.TempDir() + scaffold := exec.Command(mxPath, "create-project") + scaffold.Dir = dir + if out, err := scaffold.CombinedOutput(); err != nil { + t.Skipf("mx create-project failed, cannot scaffold fixture: %v\n%s", err, out) + } + mprPath := filepath.Join(dir, "App.mpr") + if _, err := os.Stat(mprPath); err != nil { + t.Skipf("mx create-project did not produce App.mpr: %v", err) + } + + // Precondition: the fixture must be MPRv2, or the test proves nothing. + if v := mprStorageVersion(t, mprPath); v != mpr.MPRVersionV2 { + t.Skipf("scaffolded project is %v, not MPRv2 — nothing to protect", v) + } + + var stdout, stderr bytes.Buffer + if err := Check(CheckOptions{ + ProjectPath: mprPath, + Stdout: &stdout, + Stderr: &stderr, + }); err != nil { + t.Fatalf("Check failed: %v\nstdout:\n%s\nstderr:\n%s", err, stdout.String(), stderr.String()) + } + + // Postcondition: still MPRv2. Without the fix, update-widgets would have left + // it MPRv1. + if v := mprStorageVersion(t, mprPath); v != mpr.MPRVersionV2 { + t.Errorf("Check converted the project to %v; the MPRv2 storage format must be preserved (#763)", v) + } + if _, err := os.Stat(filepath.Join(dir, "mprcontents")); err != nil { + t.Errorf("mprcontents/ missing after Check, storage format was not preserved: %v", err) + } +} + +// mprStorageVersion opens the .mpr and returns its detected storage-format version. +func mprStorageVersion(t *testing.T, mprPath string) mpr.MPRVersion { + t.Helper() + reader, err := mpr.Open(mprPath) + if err != nil { + t.Fatalf("mpr.Open(%s): %v", mprPath, err) + } + defer reader.Close() + return reader.Version() +} diff --git a/cmd/mxcli/docker/check_test.go b/cmd/mxcli/docker/check_test.go index 95c885c7a..2cabda87f 100644 --- a/cmd/mxcli/docker/check_test.go +++ b/cmd/mxcli/docker/check_test.go @@ -31,6 +31,104 @@ func TestCheck_SkipUpdateWidgets(t *testing.T) { } } +// TestSnapshotStorageFormat_RestoresV2AfterConversion guards the fix for the bug +// where `mxcli docker check` silently converted an MPRv2 project to MPRv1. The +// `mx update-widgets` step that docker check runs before `mx check` inlines every +// unit into the .mpr and deletes mprcontents/. snapshotStorageFormat backs up the +// v2 storage files first, and the restore func it returns must put the project +// back byte-for-byte, undoing the conversion. See mendixlabs/mxcli#763. +func TestSnapshotStorageFormat_RestoresV2AfterConversion(t *testing.T) { + dir := t.TempDir() + mprPath := filepath.Join(dir, "App.mpr") + contentsDir := filepath.Join(dir, "mprcontents") + + // Seed a fake MPRv2 project: an .mpr index plus a nested mprcontents/ tree + // mirroring the real XX/YY/UUID.mxunit layout. The .mpr uses mode 0600 so the + // restore's mode preservation is observable (the simulated conversion below + // rewrites it as 0644). + mprV2 := []byte("MPRv2-index-bytes") + if err := os.WriteFile(mprPath, mprV2, 0600); err != nil { + t.Fatal(err) + } + units := map[string][]byte{ + "ab/cd/unit-1.mxunit": []byte("unit-1"), + "ab/cd/unit-2.mxunit": []byte("unit-2"), + "ef/01/unit-3.mxunit": []byte("unit-3-contents"), + } + for rel, content := range units { + p := filepath.Join(contentsDir, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(p), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, content, 0644); err != nil { + t.Fatal(err) + } + } + + snapshotGlob := filepath.Join(os.TempDir(), "mxcli-mpr-snapshot-*") + leakBefore, _ := filepath.Glob(snapshotGlob) + + restore, err := snapshotStorageFormat(mprPath, contentsDir) + if err != nil { + t.Fatalf("snapshotStorageFormat: %v", err) + } + + // Simulate `mx update-widgets`: rewrite the .mpr as a v1 self-contained file + // (different bytes, different mode) and delete the whole mprcontents/ tree. + if err := os.WriteFile(mprPath, []byte("MPRv1-self-contained-inlined-and-larger"), 0644); err != nil { + t.Fatal(err) + } + if err := os.RemoveAll(contentsDir); err != nil { + t.Fatal(err) + } + + restore() + + // The .mpr index must be restored byte-identically to the original v2 file... + got, err := os.ReadFile(mprPath) + if err != nil { + t.Fatalf("read restored .mpr: %v", err) + } + if !bytes.Equal(got, mprV2) { + t.Errorf(".mpr not restored to v2 form:\n got %q\nwant %q", got, mprV2) + } + // ...and with its original file mode, not the mode from the conversion write. + if info, err := os.Stat(mprPath); err != nil { + t.Fatalf("stat restored .mpr: %v", err) + } else if perm := info.Mode().Perm(); perm != 0600 { + t.Errorf("restored .mpr mode = %o, want 0600", perm) + } + + // Every unit file must be back with its original content, and nothing extra. + for rel, want := range units { + p := filepath.Join(contentsDir, filepath.FromSlash(rel)) + got, err := os.ReadFile(p) + if err != nil { + t.Errorf("unit %s not restored: %v", rel, err) + continue + } + if !bytes.Equal(got, want) { + t.Errorf("unit %s content mismatch: got %q want %q", rel, got, want) + } + } + var restoredUnits int + _ = filepath.Walk(contentsDir, func(_ string, info os.FileInfo, err error) error { + if err == nil && !info.IsDir() { + restoredUnits++ + } + return nil + }) + if restoredUnits != len(units) { + t.Errorf("restored %d unit files, want %d", restoredUnits, len(units)) + } + + // The snapshot's temp directory must be cleaned up by restore(). + leakAfter, _ := filepath.Glob(snapshotGlob) + if len(leakAfter) > len(leakBefore) { + t.Errorf("snapshot temp dir leaked: %d before, %d after restore", len(leakBefore), len(leakAfter)) + } +} + // createFakeMxDir creates a temp directory with fake mx and mxbuild scripts // that log their first argument to a file. func createFakeMxDir(t *testing.T) (dir, logFile string) {