Skip to content

Commit 88077af

Browse files
rsaunderson-i3vclaude
authored andcommitted
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) <noreply@anthropic.com>
1 parent ea98064 commit 88077af

3 files changed

Lines changed: 268 additions & 0 deletions

File tree

cmd/mxcli/docker/check.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,66 @@ func updateWidgetsPathArg(p string) string {
5050
return p
5151
}
5252

53+
// snapshotStorageFormat backs up the MPRv2 storage files (.mpr index + mprcontents/)
54+
// to a temp directory and returns a restore function that puts them back, undoing
55+
// any v2 -> v1 conversion performed by an intervening `mx update-widgets`. The
56+
// restore function removes the temp directory and is safe to defer; it best-effort
57+
// restores and never panics. mprPath and contentsDir come from an mpr.Reader on a
58+
// project already known to be MPRv2.
59+
func snapshotStorageFormat(mprPath, contentsDir string) (restore func(), err error) {
60+
tmp, err := os.MkdirTemp("", "mxcli-mpr-snapshot-*")
61+
if err != nil {
62+
return nil, err
63+
}
64+
65+
mprBackup := filepath.Join(tmp, filepath.Base(mprPath))
66+
if err := copyFile(mprPath, mprBackup); err != nil {
67+
os.RemoveAll(tmp)
68+
return nil, err
69+
}
70+
71+
contentsBackup := filepath.Join(tmp, "mprcontents")
72+
if err := copyDir(contentsDir, contentsBackup); err != nil {
73+
os.RemoveAll(tmp)
74+
return nil, err
75+
}
76+
77+
restore = func() {
78+
defer os.RemoveAll(tmp)
79+
// Restore the v2 index file.
80+
_ = copyFile(mprBackup, mprPath)
81+
// update-widgets deletes mprcontents/; drop whatever is there now (nothing,
82+
// after a conversion) and restore the backed-up tree.
83+
_ = os.RemoveAll(contentsDir)
84+
_ = copyDir(contentsBackup, contentsDir)
85+
}
86+
return restore, nil
87+
}
88+
89+
// copyFile copies a single file from src to dst, preserving the source file mode.
90+
func copyFile(src, dst string) error {
91+
info, err := os.Stat(src)
92+
if err != nil {
93+
return err
94+
}
95+
in, err := os.Open(src)
96+
if err != nil {
97+
return err
98+
}
99+
defer in.Close()
100+
101+
out, err := os.OpenFile(dst, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, info.Mode())
102+
if err != nil {
103+
return err
104+
}
105+
defer out.Close()
106+
107+
if _, err := io.Copy(out, in); err != nil {
108+
return err
109+
}
110+
return out.Close()
111+
}
112+
53113
// Check runs 'mx check' on the project to validate it before building.
54114
func Check(opts CheckOptions) error {
55115
w := opts.Stdout
@@ -76,6 +136,35 @@ func Check(opts CheckOptions) error {
76136
}
77137
fmt.Fprintf(w, "Using mx: %s\n", mxPath)
78138

139+
// `mx update-widgets` rewrites an MPRv2 project into the self-contained MPRv1
140+
// storage format: it inlines every unit into the .mpr and deletes mprcontents/.
141+
// A `check` must not mutate the on-disk storage format — silently doing so
142+
// desyncs the working tree from a Git repository that tracks the mprcontents/
143+
// files and can leave Studio Pro unable to open the project. So when the project
144+
// is MPRv2, snapshot the .mpr + mprcontents/ before update-widgets and restore
145+
// them after the check. The check still runs against the widget-normalized model
146+
// (so CE0463 false positives are still suppressed); only the on-disk format is
147+
// preserved. MPRv1 projects are already single-file and need no protection.
148+
if !opts.SkipUpdateWidgets && opts.ProjectPath != "" {
149+
if reader, err := mpr.Open(opts.ProjectPath); err == nil {
150+
isV2 := reader.Version() == mpr.MPRVersionV2
151+
contentsDir := reader.ContentsDir()
152+
reader.Close()
153+
if isV2 {
154+
restore, snapErr := snapshotStorageFormat(opts.ProjectPath, contentsDir)
155+
if snapErr != nil {
156+
// Can't protect the format — skip update-widgets rather than risk
157+
// an unrecoverable v2 -> v1 conversion. A CE0463 false positive is
158+
// the lesser evil than a silent, unrestorable format change.
159+
fmt.Fprintf(w, "Warning: could not snapshot MPRv2 storage (skipping update-widgets to avoid a v2->v1 conversion): %v\n", snapErr)
160+
opts.SkipUpdateWidgets = true
161+
} else {
162+
defer restore()
163+
}
164+
}
165+
}
166+
}
167+
79168
// Run mx update-widgets to normalize pluggable widget definitions.
80169
// This prevents false CE0463 ("widget definition changed") errors caused
81170
// by mismatch between widget Object properties and Type PropertyTypes.
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
//go:build integration
4+
5+
package docker
6+
7+
import (
8+
"bytes"
9+
"os"
10+
"os/exec"
11+
"path/filepath"
12+
"testing"
13+
14+
"github.com/mendixlabs/mxcli/sdk/mpr"
15+
)
16+
17+
// TestCheck_PreservesMPRv2StorageFormat is the end-to-end guard for
18+
// mendixlabs/mxcli#763. `mx update-widgets`, which Check runs before `mx check`,
19+
// rewrites an MPRv2 project into the self-contained MPRv1 format (inlining every
20+
// unit into the .mpr and deleting mprcontents/). Check must leave the on-disk
21+
// storage format exactly as it found it. Both mx 11.9.0 (the version the CI
22+
// integration job installs) and 11.12.1 were observed to perform this conversion,
23+
// so on a v2 project this test genuinely exercises the bug: without the snapshot/
24+
// restore fix the project would be MPRv1 after Check.
25+
//
26+
// Requires a resolvable mx (provided by the CI integration job's
27+
// `mxcli setup mxbuild`); skips otherwise. Uses `mx create-project`, whose output
28+
// is MPRv2, as the fixture — the same scaffolding the executor roundtrip suite uses.
29+
func TestCheck_PreservesMPRv2StorageFormat(t *testing.T) {
30+
mxPath, err := ResolveMx("")
31+
if err != nil {
32+
t.Skipf("mx not resolvable: %v", err)
33+
}
34+
35+
// Scaffold a fresh project. `mx create-project` (no template arg) writes App.mpr
36+
// into the working directory and produces MPRv2 storage.
37+
dir := t.TempDir()
38+
scaffold := exec.Command(mxPath, "create-project")
39+
scaffold.Dir = dir
40+
if out, err := scaffold.CombinedOutput(); err != nil {
41+
t.Skipf("mx create-project failed, cannot scaffold fixture: %v\n%s", err, out)
42+
}
43+
mprPath := filepath.Join(dir, "App.mpr")
44+
if _, err := os.Stat(mprPath); err != nil {
45+
t.Skipf("mx create-project did not produce App.mpr: %v", err)
46+
}
47+
48+
// Precondition: the fixture must be MPRv2, or the test proves nothing.
49+
if v := mprStorageVersion(t, mprPath); v != mpr.MPRVersionV2 {
50+
t.Skipf("scaffolded project is %v, not MPRv2 — nothing to protect", v)
51+
}
52+
53+
var stdout, stderr bytes.Buffer
54+
if err := Check(CheckOptions{
55+
ProjectPath: mprPath,
56+
Stdout: &stdout,
57+
Stderr: &stderr,
58+
}); err != nil {
59+
t.Fatalf("Check failed: %v\nstdout:\n%s\nstderr:\n%s", err, stdout.String(), stderr.String())
60+
}
61+
62+
// Postcondition: still MPRv2. Without the fix, update-widgets would have left
63+
// it MPRv1.
64+
if v := mprStorageVersion(t, mprPath); v != mpr.MPRVersionV2 {
65+
t.Errorf("Check converted the project to %v; the MPRv2 storage format must be preserved (#763)", v)
66+
}
67+
if _, err := os.Stat(filepath.Join(dir, "mprcontents")); err != nil {
68+
t.Errorf("mprcontents/ missing after Check, storage format was not preserved: %v", err)
69+
}
70+
}
71+
72+
// mprStorageVersion opens the .mpr and returns its detected storage-format version.
73+
func mprStorageVersion(t *testing.T, mprPath string) mpr.MPRVersion {
74+
t.Helper()
75+
reader, err := mpr.Open(mprPath)
76+
if err != nil {
77+
t.Fatalf("mpr.Open(%s): %v", mprPath, err)
78+
}
79+
defer reader.Close()
80+
return reader.Version()
81+
}

cmd/mxcli/docker/check_test.go

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,104 @@ func TestCheck_SkipUpdateWidgets(t *testing.T) {
3131
}
3232
}
3333

34+
// TestSnapshotStorageFormat_RestoresV2AfterConversion guards the fix for the bug
35+
// where `mxcli docker check` silently converted an MPRv2 project to MPRv1. The
36+
// `mx update-widgets` step that docker check runs before `mx check` inlines every
37+
// unit into the .mpr and deletes mprcontents/. snapshotStorageFormat backs up the
38+
// v2 storage files first, and the restore func it returns must put the project
39+
// back byte-for-byte, undoing the conversion. See mendixlabs/mxcli#763.
40+
func TestSnapshotStorageFormat_RestoresV2AfterConversion(t *testing.T) {
41+
dir := t.TempDir()
42+
mprPath := filepath.Join(dir, "App.mpr")
43+
contentsDir := filepath.Join(dir, "mprcontents")
44+
45+
// Seed a fake MPRv2 project: an .mpr index plus a nested mprcontents/ tree
46+
// mirroring the real XX/YY/UUID.mxunit layout. The .mpr uses mode 0600 so the
47+
// restore's mode preservation is observable (the simulated conversion below
48+
// rewrites it as 0644).
49+
mprV2 := []byte("MPRv2-index-bytes")
50+
if err := os.WriteFile(mprPath, mprV2, 0600); err != nil {
51+
t.Fatal(err)
52+
}
53+
units := map[string][]byte{
54+
"ab/cd/unit-1.mxunit": []byte("unit-1"),
55+
"ab/cd/unit-2.mxunit": []byte("unit-2"),
56+
"ef/01/unit-3.mxunit": []byte("unit-3-contents"),
57+
}
58+
for rel, content := range units {
59+
p := filepath.Join(contentsDir, filepath.FromSlash(rel))
60+
if err := os.MkdirAll(filepath.Dir(p), 0755); err != nil {
61+
t.Fatal(err)
62+
}
63+
if err := os.WriteFile(p, content, 0644); err != nil {
64+
t.Fatal(err)
65+
}
66+
}
67+
68+
snapshotGlob := filepath.Join(os.TempDir(), "mxcli-mpr-snapshot-*")
69+
leakBefore, _ := filepath.Glob(snapshotGlob)
70+
71+
restore, err := snapshotStorageFormat(mprPath, contentsDir)
72+
if err != nil {
73+
t.Fatalf("snapshotStorageFormat: %v", err)
74+
}
75+
76+
// Simulate `mx update-widgets`: rewrite the .mpr as a v1 self-contained file
77+
// (different bytes, different mode) and delete the whole mprcontents/ tree.
78+
if err := os.WriteFile(mprPath, []byte("MPRv1-self-contained-inlined-and-larger"), 0644); err != nil {
79+
t.Fatal(err)
80+
}
81+
if err := os.RemoveAll(contentsDir); err != nil {
82+
t.Fatal(err)
83+
}
84+
85+
restore()
86+
87+
// The .mpr index must be restored byte-identically to the original v2 file...
88+
got, err := os.ReadFile(mprPath)
89+
if err != nil {
90+
t.Fatalf("read restored .mpr: %v", err)
91+
}
92+
if !bytes.Equal(got, mprV2) {
93+
t.Errorf(".mpr not restored to v2 form:\n got %q\nwant %q", got, mprV2)
94+
}
95+
// ...and with its original file mode, not the mode from the conversion write.
96+
if info, err := os.Stat(mprPath); err != nil {
97+
t.Fatalf("stat restored .mpr: %v", err)
98+
} else if perm := info.Mode().Perm(); perm != 0600 {
99+
t.Errorf("restored .mpr mode = %o, want 0600", perm)
100+
}
101+
102+
// Every unit file must be back with its original content, and nothing extra.
103+
for rel, want := range units {
104+
p := filepath.Join(contentsDir, filepath.FromSlash(rel))
105+
got, err := os.ReadFile(p)
106+
if err != nil {
107+
t.Errorf("unit %s not restored: %v", rel, err)
108+
continue
109+
}
110+
if !bytes.Equal(got, want) {
111+
t.Errorf("unit %s content mismatch: got %q want %q", rel, got, want)
112+
}
113+
}
114+
var restoredUnits int
115+
_ = filepath.Walk(contentsDir, func(_ string, info os.FileInfo, err error) error {
116+
if err == nil && !info.IsDir() {
117+
restoredUnits++
118+
}
119+
return nil
120+
})
121+
if restoredUnits != len(units) {
122+
t.Errorf("restored %d unit files, want %d", restoredUnits, len(units))
123+
}
124+
125+
// The snapshot's temp directory must be cleaned up by restore().
126+
leakAfter, _ := filepath.Glob(snapshotGlob)
127+
if len(leakAfter) > len(leakBefore) {
128+
t.Errorf("snapshot temp dir leaked: %d before, %d after restore", len(leakBefore), len(leakAfter))
129+
}
130+
}
131+
34132
// createFakeMxDir creates a temp directory with fake mx and mxbuild scripts
35133
// that log their first argument to a file.
36134
func createFakeMxDir(t *testing.T) (dir, logFile string) {

0 commit comments

Comments
 (0)