From 5d615bc848ae61cc53de0078c0aa485017b0223b Mon Sep 17 00:00:00 2001 From: Atanas Dinov Date: Thu, 7 May 2026 15:38:47 +0300 Subject: [PATCH] Detect conflicts during merge conflict Signed-off-by: Atanas Dinov --- pkg/merge/merge.go | 257 ++++++++++++++++++ pkg/merge/merge_suite_test.go | 30 ++ pkg/merge/merge_test.go | 245 +++++++++++++++++ pkg/transaction/snapper_upgrade_helper.go | 73 ++++- .../snapper_upgrade_helper_test.go | 24 +- pkg/transaction/transaction.go | 1 + 6 files changed, 626 insertions(+), 4 deletions(-) create mode 100644 pkg/merge/merge.go create mode 100644 pkg/merge/merge_suite_test.go create mode 100644 pkg/merge/merge_test.go diff --git a/pkg/merge/merge.go b/pkg/merge/merge.go new file mode 100644 index 00000000..49b108e4 --- /dev/null +++ b/pkg/merge/merge.go @@ -0,0 +1,257 @@ +/* +Copyright © 2025-2026 SUSE LLC +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package merge + +import ( + "bytes" + "fmt" + "io" + iofs "io/fs" + "os" + "path/filepath" + "slices" + "strings" + + "github.com/suse/elemental/v3/pkg/sys/vfs" +) + +// ChangeType represents the kind of change made to a file relative to a +// common ancestor. The zero value is the empty string, deliberately not a +// valid change, so an uninitialised ChangeType reads as obviously missing. +type ChangeType string + +const ( + ChangeTypeAdded ChangeType = "added" + ChangeTypeModified ChangeType = "modified" + ChangeTypeDeleted ChangeType = "deleted" +) + +const MaxContentCompareSize = 16 * 1024 * 1024 + +// Conflict represents a file the user and the OS both changed relative to +// the common ancestor. +type Conflict struct { + Path string + UserChange ChangeType + OSChange ChangeType +} + +func (c Conflict) String() string { + return fmt.Sprintf(" %s — user: %s, OS: %s", c.Path, c.UserChange, c.OSChange) +} + +var skippedDirNames = map[string]bool{ + ".snapshots": true, +} + +// DetectConflicts walks oldRoot and newRoot to derive the OS defaults delta, +// then returns the files that also appear in userChanges (i.e. that both the +// user and the OS modified relative to the common ancestor). +// +// userChanges is keyed by rel-path with a leading "/" (e.g. "/sshd_config"), +// matching what the walk produces. +// +// Regular files are compared by size and then content (up to +// MaxContentCompareSize -- larger matched-size files are conservatively +// flagged as modified without being read). Symlinks compared by target. +// Type changes (file->dir, file->symlink, ...) reported as modified. +// Directories whose basename is in skippedDirNames (e.g. snapper's +// ".snapshots") are not descended into. +func DetectConflicts(fs vfs.FS, userChanges map[string]ChangeType, oldRoot, newRoot string) ([]Conflict, error) { + osChanges, err := computeOSChanges(fs, oldRoot, newRoot) + if err != nil { + return nil, err + } + + var conflicts []Conflict + for path, userChange := range userChanges { + osChange, ok := osChanges[path] + if !ok { + continue + } + conflicts = append(conflicts, Conflict{ + Path: path, + UserChange: userChange, + OSChange: osChange, + }) + } + slices.SortFunc(conflicts, func(a, b Conflict) int { + return strings.Compare(a.Path, b.Path) + }) + return conflicts, nil +} + +// FormatConflictSummary returns a human-readable summary of detected +// conflicts; returns "" if there are none. +func FormatConflictSummary(volumePath string, conflicts []Conflict) string { + if len(conflicts) == 0 { + return "" + } + + var b strings.Builder + + _, _ = fmt.Fprintf(&b, "Merge conflicts detected for %s (%d file(s)), user version kept:\n", + volumePath, len(conflicts)) + for _, c := range conflicts { + _, _ = fmt.Fprintf(&b, "%s\n", c) + } + + return b.String() +} + +func computeOSChanges(fs vfs.FS, oldRoot, newRoot string) (map[string]ChangeType, error) { + oldEntries, err := walkEntries(fs, oldRoot) + if err != nil { + return nil, fmt.Errorf("walking %s: %w", oldRoot, err) + } + + newEntries, err := walkEntries(fs, newRoot) + if err != nil { + return nil, fmt.Errorf("walking %s: %w", newRoot, err) + } + + changes := make(map[string]ChangeType) + for rel, newInfo := range newEntries { + oldInfo, ok := oldEntries[rel] + if !ok { + if !newInfo.IsDir() { + changes[rel] = ChangeTypeAdded + } + continue + } + delete(oldEntries, rel) + + differs, err := entriesDiffer(fs, oldInfo, newInfo, filepath.Join(oldRoot, rel), filepath.Join(newRoot, rel)) + if err != nil { + return nil, err + } + if differs { + changes[rel] = ChangeTypeModified + } + } + + for rel, oldInfo := range oldEntries { + if oldInfo.IsDir() { + continue + } + changes[rel] = ChangeTypeDeleted + } + return changes, nil +} + +// walkEntries walks root and returns a map of rel-path → FileInfo. The root +// itself is excluded and directories listed in skippedDirNames are not +// descended into. Returns an empty map (no error) if root does not exist. +func walkEntries(fs vfs.FS, root string) (map[string]iofs.FileInfo, error) { + entries := make(map[string]iofs.FileInfo) + if _, err := fs.Stat(root); err != nil { + if os.IsNotExist(err) { + return entries, nil + } + return nil, err + } + err := vfs.WalkDirFs(fs, root, func(path string, d iofs.DirEntry, err error) error { + if err != nil { + return err + } + if path == root { + return nil + } + if d.IsDir() && skippedDirNames[d.Name()] { + return filepath.SkipDir + } + rel := strings.TrimPrefix(path, root) + info, err := d.Info() + if err != nil { + return err + } + entries[rel] = info + return nil + }) + if err != nil { + return nil, err + } + return entries, nil +} + +// entriesDiffer compares two entries that exist at the same relative path on +// both sides and reports whether they should be considered modified. +func entriesDiffer(fs vfs.FS, oldInfo, newInfo iofs.FileInfo, oldPath, newPath string) (bool, error) { + if oldInfo.Mode().Type() != newInfo.Mode().Type() { + return true, nil + } + if oldInfo.IsDir() { + return false, nil + } + if oldInfo.Mode()&os.ModeSymlink != 0 { + oldTarget, err := fs.Readlink(oldPath) + if err != nil { + return false, err + } + newTarget, err := fs.Readlink(newPath) + if err != nil { + return false, err + } + return oldTarget != newTarget, nil + } + if !oldInfo.Mode().IsRegular() { + return false, nil + } + if oldInfo.Size() != newInfo.Size() { + return true, nil + } + if oldInfo.Size() > MaxContentCompareSize { + // Bigger than the cap — conservatively report modified instead of + // reading the file. Worst case is a false-positive conflict warning. + return true, nil + } + return regularFilesContentDiffer(fs, oldPath, newPath) +} + +func regularFilesContentDiffer(fs vfs.FS, oldPath, newPath string) (bool, error) { + oldF, err := fs.Open(oldPath) + if err != nil { + return false, err + } + defer oldF.Close() + newF, err := fs.Open(newPath) + if err != nil { + return false, err + } + defer newF.Close() + + const bufSize = 32 * 1024 + bufA := make([]byte, bufSize) + bufB := make([]byte, bufSize) + for { + nA, errA := io.ReadFull(oldF, bufA) + nB, errB := io.ReadFull(newF, bufB) + if nA != nB || !bytes.Equal(bufA[:nA], bufB[:nB]) { + return true, nil + } + if errA == io.EOF || errA == io.ErrUnexpectedEOF { + return false, nil + } + if errA != nil { + return false, errA + } + if errB != nil { + return false, errB + } + } +} diff --git a/pkg/merge/merge_suite_test.go b/pkg/merge/merge_suite_test.go new file mode 100644 index 00000000..5e0d00c9 --- /dev/null +++ b/pkg/merge/merge_suite_test.go @@ -0,0 +1,30 @@ +/* +Copyright © 2025-2026 SUSE LLC +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package merge_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestMergeSuite(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Merge test suite") +} diff --git a/pkg/merge/merge_test.go b/pkg/merge/merge_test.go new file mode 100644 index 00000000..8cc576b9 --- /dev/null +++ b/pkg/merge/merge_test.go @@ -0,0 +1,245 @@ +/* +Copyright © 2025-2026 SUSE LLC +SPDX-License-Identifier: Apache-2.0 + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package merge_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/suse/elemental/v3/pkg/merge" + sysmock "github.com/suse/elemental/v3/pkg/sys/mock" + "github.com/suse/elemental/v3/pkg/sys/vfs" +) + +var _ = Describe("DetectConflicts", Label("merge"), func() { + var ( + tfs vfs.FS + cleanup func() + ) + + BeforeEach(func() { + var err error + tfs, cleanup, err = sysmock.TestFS(nil) + Expect(err).NotTo(HaveOccurred()) + Expect(vfs.MkdirAll(tfs, "/old", vfs.DirPerm)).To(Succeed()) + Expect(vfs.MkdirAll(tfs, "/new", vfs.DirPerm)).To(Succeed()) + }) + + AfterEach(func() { + cleanup() + }) + + It("returns no conflicts when user changes don't overlap the OS delta", func() { + // OS modified /os-only; user touched a different path. + Expect(tfs.WriteFile("/old/os-only", []byte("v1"), vfs.FilePerm)).To(Succeed()) + Expect(tfs.WriteFile("/new/os-only", []byte("v2"), vfs.FilePerm)).To(Succeed()) + userChanges := map[string]merge.ChangeType{"/user-only": merge.ChangeTypeAdded} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(BeEmpty()) + }) + + It("returns no conflicts when the OS tree is unchanged", func() { + Expect(tfs.WriteFile("/old/shared", []byte("hello"), vfs.FilePerm)).To(Succeed()) + Expect(tfs.WriteFile("/new/shared", []byte("hello"), vfs.FilePerm)).To(Succeed()) + // User modified it, but the OS did not; not a conflict. + userChanges := map[string]merge.ChangeType{"/shared": merge.ChangeTypeModified} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(BeEmpty()) + }) + + It("flags a both-modified file", func() { + Expect(tfs.WriteFile("/old/shared", []byte("baseline"), vfs.FilePerm)).To(Succeed()) + Expect(tfs.WriteFile("/new/shared", []byte("os-updated"), vfs.FilePerm)).To(Succeed()) + userChanges := map[string]merge.ChangeType{"/shared": merge.ChangeTypeModified} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(ConsistOf(merge.Conflict{ + Path: "/shared", + UserChange: merge.ChangeTypeModified, + OSChange: merge.ChangeTypeModified, + })) + }) + + It("flags OS-deleted vs user-modified", func() { + Expect(tfs.WriteFile("/old/gone", []byte("v1"), vfs.FilePerm)).To(Succeed()) + // /new/gone is absent — OS removed it + userChanges := map[string]merge.ChangeType{"/gone": merge.ChangeTypeModified} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(ConsistOf(merge.Conflict{ + Path: "/gone", + UserChange: merge.ChangeTypeModified, + OSChange: merge.ChangeTypeDeleted, + })) + }) + + It("flags both-added paths", func() { + Expect(tfs.WriteFile("/new/added", []byte("v"), vfs.FilePerm)).To(Succeed()) + userChanges := map[string]merge.ChangeType{"/added": merge.ChangeTypeAdded} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(ConsistOf(merge.Conflict{ + Path: "/added", + UserChange: merge.ChangeTypeAdded, + OSChange: merge.ChangeTypeAdded, + })) + }) + + It("returns conflicts sorted by path", func() { + Expect(tfs.WriteFile("/old/a.conf", []byte("v1"), vfs.FilePerm)).To(Succeed()) + Expect(tfs.WriteFile("/new/a.conf", []byte("v2"), vfs.FilePerm)).To(Succeed()) + Expect(tfs.WriteFile("/new/m.conf", []byte("m"), vfs.FilePerm)).To(Succeed()) + Expect(tfs.WriteFile("/old/z.conf", []byte("v1"), vfs.FilePerm)).To(Succeed()) + Expect(tfs.WriteFile("/new/z.conf", []byte("v2"), vfs.FilePerm)).To(Succeed()) + userChanges := map[string]merge.ChangeType{ + "/z.conf": merge.ChangeTypeModified, + "/a.conf": merge.ChangeTypeDeleted, + "/m.conf": merge.ChangeTypeAdded, + } + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(Equal([]merge.Conflict{ + {Path: "/a.conf", UserChange: merge.ChangeTypeDeleted, OSChange: merge.ChangeTypeModified}, + {Path: "/m.conf", UserChange: merge.ChangeTypeAdded, OSChange: merge.ChangeTypeAdded}, + {Path: "/z.conf", UserChange: merge.ChangeTypeModified, OSChange: merge.ChangeTypeModified}, + })) + }) + + It("detects edits that don't change file size", func() { + // Both files are the same length -- the size check short-circuits to "equal" + // here, so reaching the conflict requires the content compare to + // notice the bytes actually differ. + Expect(tfs.WriteFile("/old/cfg", []byte("abc"), vfs.FilePerm)).To(Succeed()) + Expect(tfs.WriteFile("/new/cfg", []byte("xyz"), vfs.FilePerm)).To(Succeed()) + userChanges := map[string]merge.ChangeType{"/cfg": merge.ChangeTypeModified} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(ConsistOf(merge.Conflict{ + Path: "/cfg", + UserChange: merge.ChangeTypeModified, + OSChange: merge.ChangeTypeModified, + })) + }) + + It("detects symlink target changes", func() { + Expect(tfs.Symlink("/old/target-a", "/old/link")).To(Succeed()) + Expect(tfs.Symlink("/new/target-b", "/new/link")).To(Succeed()) + userChanges := map[string]merge.ChangeType{"/link": merge.ChangeTypeModified} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(ConsistOf(merge.Conflict{ + Path: "/link", + UserChange: merge.ChangeTypeModified, + OSChange: merge.ChangeTypeModified, + })) + }) + + It("reports type changes (dir→file) as modified", func() { + // /x existed in old as a directory; the new image replaced it with + // a regular file. The user independently edited the directory's + // stand-in (e.g. dropped a file into it that maps to the same rel + // path), so we expect a both-modified conflict. + Expect(vfs.MkdirAll(tfs, "/old/x", vfs.DirPerm)).To(Succeed()) + Expect(tfs.WriteFile("/new/x", []byte("now a file"), vfs.FilePerm)).To(Succeed()) + userChanges := map[string]merge.ChangeType{"/x": merge.ChangeTypeModified} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(ConsistOf(merge.Conflict{ + Path: "/x", + UserChange: merge.ChangeTypeModified, + OSChange: merge.ChangeTypeModified, + })) + }) + + It("does not descend into nested .snapshots directories", func() { + // snapper-style nested snapshot tree: the walk must skip it even when + // its contents differ wildly between old and new. + Expect(vfs.MkdirAll(tfs, "/old/.snapshots/1/snapshot", vfs.DirPerm)).To(Succeed()) + Expect(tfs.WriteFile("/old/.snapshots/1/snapshot/inner", []byte("old"), vfs.FilePerm)).To(Succeed()) + Expect(vfs.MkdirAll(tfs, "/new/.snapshots/9/snapshot", vfs.DirPerm)).To(Succeed()) + Expect(tfs.WriteFile("/new/.snapshots/9/snapshot/inner", []byte("different"), vfs.FilePerm)).To(Succeed()) + userChanges := map[string]merge.ChangeType{"/.snapshots/1/snapshot/inner": merge.ChangeTypeModified} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(BeEmpty()) + }) + + It("flags same-size files above the cap as modified", func() { + // Both files hold identical bytes. Without the size cap the content + // compare would return "equal" and there would be no conflict; a + // conflict appearing confirms the cap branch fired instead. + big := make([]byte, merge.MaxContentCompareSize+1) + Expect(tfs.WriteFile("/old/huge", big, vfs.FilePerm)).To(Succeed()) + Expect(tfs.WriteFile("/new/huge", big, vfs.FilePerm)).To(Succeed()) + userChanges := map[string]merge.ChangeType{"/huge": merge.ChangeTypeModified} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/old", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(ConsistOf(merge.Conflict{ + Path: "/huge", + UserChange: merge.ChangeTypeModified, + OSChange: merge.ChangeTypeModified, + })) + }) + + It("treats a missing old root as an empty tree", func() { + // With no old tree, every file in /new is an addition. A user file + // at the same path collides → both-added conflict, exactly as if the + // old tree existed and was empty. + Expect(tfs.WriteFile("/new/a", []byte("a"), vfs.FilePerm)).To(Succeed()) + userChanges := map[string]merge.ChangeType{"/a": merge.ChangeTypeAdded} + + conflicts, err := merge.DetectConflicts(tfs, userChanges, "/does-not-exist", "/new") + Expect(err).NotTo(HaveOccurred()) + Expect(conflicts).To(ConsistOf(merge.Conflict{ + Path: "/a", + UserChange: merge.ChangeTypeAdded, + OSChange: merge.ChangeTypeAdded, + })) + }) +}) + +var _ = Describe("FormatConflictSummary", Label("merge"), func() { + It("returns empty string when no conflicts", func() { + Expect(merge.FormatConflictSummary("/etc", nil)).To(Equal("")) + Expect(merge.FormatConflictSummary("/etc", []merge.Conflict{})).To(Equal("")) + }) + + It("formats a summary with conflicts", func() { + conflicts := []merge.Conflict{ + {Path: "/etc/foo", UserChange: merge.ChangeTypeModified, OSChange: merge.ChangeTypeModified}, + {Path: "/etc/bar", UserChange: merge.ChangeTypeDeleted, OSChange: merge.ChangeTypeModified}, + } + summary := merge.FormatConflictSummary("/etc", conflicts) + Expect(summary).To(ContainSubstring("Merge conflicts detected for /etc (2 file(s))")) + Expect(summary).To(ContainSubstring("/etc/foo — user: modified, OS: modified")) + Expect(summary).To(ContainSubstring("/etc/bar — user: deleted, OS: modified")) + }) +}) diff --git a/pkg/transaction/snapper_upgrade_helper.go b/pkg/transaction/snapper_upgrade_helper.go index 4b040883..c81c5e54 100644 --- a/pkg/transaction/snapper_upgrade_helper.go +++ b/pkg/transaction/snapper_upgrade_helper.go @@ -30,6 +30,7 @@ import ( "github.com/suse/elemental/v3/pkg/chroot" "github.com/suse/elemental/v3/pkg/deployment" "github.com/suse/elemental/v3/pkg/fstab" + "github.com/suse/elemental/v3/pkg/merge" "github.com/suse/elemental/v3/pkg/rsync" "github.com/suse/elemental/v3/pkg/snapper" "github.com/suse/elemental/v3/pkg/sys/vfs" @@ -197,13 +198,14 @@ func (sc snapperContext) configureRWVolumes(trans *Transaction) error { description := fmt.Sprintf("stock %s contents", rwVol.Path) metadata := map[string]string{"stock": "true"} - _, err = sc.snap.CreateSnapshot("/", config, 0, false, description, metadata) + newStockID, err := sc.snap.CreateSnapshot("/", config, 0, false, description, metadata) if err != nil { return fmt.Errorf("creating snapshot '%s': %w", rwVol.Path, err) } if _, ok := trans.Merges[rwVol.Path]; ok { trans.Merges[rwVol.Path].New = filepath.Join(trans.Path, rwVol.Path) + trans.Merges[rwVol.Path].NewStock = filepath.Join(trans.Path, rwVol.Path, fmt.Sprintf(snapshotPathTmpl, newStockID)) } } return nil @@ -212,8 +214,8 @@ func (sc snapperContext) configureRWVolumes(trans *Transaction) error { } // merge runs a 3 way merge for snapshotted RW volumes. -// Current implementation solves potential conflicts by always keeping -// custom changes over changes coming from the OS image. +// User changes are always applied over OS changes. When both sides modified the +// same file, user version wins and the conflict is reported to the user. func (sc snapperContext) merge(trans *Transaction) (err error) { var status, tmpDir string @@ -240,6 +242,29 @@ func (sc snapperContext) merge(trans *Transaction) (err error) { return err } + userChanges, err := sc.parseSnapperStatus(status, rwVol.Path) + if err != nil { + return fmt.Errorf("parsing user changes for %s: %w", rwVol.Path, err) + } + + // Compute the defaults delta (old stock -> new stock) via a file-system + // walk, intersected with userChanges. Unlike the user delta above -- + // where both snapshots live in the same snapper config (old root, IDs + // comparable with `snapper status`) -- the old and new stocks live in + // independent snapper configs (old root vs the new root's freshly + // created config), so their IDs aren't comparable through snapper. + // Both trees are plain directories on disk, though, so we diff them + // directly. + if m.NewStock != "" { + conflicts, err := merge.DetectConflicts(sc.s.FS(), userChanges, m.Old, m.NewStock) + if err != nil { + sc.s.Logger().Warn("Could not compute OS changes for %s: %v", rwVol.Path, err) + } else if len(conflicts) > 0 { + conflictSummary := merge.FormatConflictSummary(rwVol.Path, conflicts) + sc.s.Logger().Warn(conflictSummary) + } + } + err = sc.applyCustomChanges(status, rwVol.Path, m) if err != nil { return err @@ -274,6 +299,48 @@ func (sc snapperContext) customChangesStatus(volPath string, merge *Merge, outpu return nil } +// parseSnapperStatus parses snapper status output into a map of rel-path → +// ChangeType describing the user's customisations delta. +func (sc snapperContext) parseSnapperStatus(statusFile, rwVolPath string) (map[string]merge.ChangeType, error) { + statusF, err := sc.s.FS().OpenFile(statusFile, os.O_RDONLY, vfs.FilePerm) + if err != nil { + return nil, err + } + defer statusF.Close() + + changes := make(map[string]merge.ChangeType) + r := regexp.MustCompile(`(([-+ct.])[p.][u.][g.][x.][a.])\s+(.*)`) + + scanner := bufio.NewScanner(statusF) + for scanner.Scan() { + line := scanner.Text() + match := r.FindStringSubmatch(line) + + if len(match) == 0 { + continue + } + if strings.HasPrefix(match[1], "....") { + // Ignore lines whose only differences are in xattr/ACL columns + // (positions 5-6). The old stock snapshot was taken before SELinux + // relabelling, so xattr-only diffs would otherwise list almost every + // file. + continue + } + + filePath := strings.TrimPrefix(match[3], rwVolPath) + switch match[2] { + case "+": + changes[filePath] = merge.ChangeTypeAdded + case "-": + changes[filePath] = merge.ChangeTypeDeleted + case "c", "t", ".": + changes[filePath] = merge.ChangeTypeModified + } + } + + return changes, nil +} + // applyCustomChanges reads the given status file and applies reported changes in to the target destination. // This method is the responsible of applying customizations to the new volume func (sc snapperContext) applyCustomChanges(status, rwVolPath string, merge *Merge) (err error) { diff --git a/pkg/transaction/snapper_upgrade_helper_test.go b/pkg/transaction/snapper_upgrade_helper_test.go index d2f301bf..4d439bc3 100644 --- a/pkg/transaction/snapper_upgrade_helper_test.go +++ b/pkg/transaction/snapper_upgrade_helper_test.go @@ -195,7 +195,29 @@ var _ = Describe("SnapperUpgradeHelper", Label("transaction"), func() { Expect(tfs.WriteFile(filepath.Join(etcMerge.Modified, "unmodifiedFile"), []byte("non modified file"), vfs.FilePerm)).To(Succeed()) Expect(vfs.MkdirAll(tfs, trans.Merges["/home"].Modified, vfs.DirPerm)).To(Succeed()) - // New defaults + // Old stock (the pre-upgrade /etc baseline). modifiedFile is the + // shared baseline with the customisations; relabelledFile and + // unmodifiedFile match the user version (no user delta on these + // in the modified vs old comparison). + Expect(vfs.MkdirAll(tfs, etcMerge.Old, vfs.DirPerm)).To(Succeed()) + Expect(tfs.WriteFile(filepath.Join(etcMerge.Old, "modifiedFile"), []byte("old defaults modified file"), vfs.FilePerm)).To(Succeed()) + + // New stock (snapshot of the new image's /etc, captured BEFORE + // applyCustomChanges). configureRWVolumes sets etcMerge.NewStock + // during Merge — predict the path here so we can seed it: the + // mocked snapper.create returns id 5, so NewStock lands at + // /etc/.snapshots/5/snapshot. modifiedFile differs + // from old → defaults delta touches it, colliding with the user + // customisation. + etcNewStock := filepath.Join(root, ".snapshots/5/snapshot/etc/.snapshots/5/snapshot") + homeNewStock := filepath.Join(root, ".snapshots/5/snapshot/home/.snapshots/5/snapshot") + Expect(vfs.MkdirAll(tfs, etcNewStock, vfs.DirPerm)).To(Succeed()) + Expect(tfs.WriteFile(filepath.Join(etcNewStock, "modifiedFile"), []byte("new defaults modified file"), vfs.FilePerm)).To(Succeed()) + + Expect(vfs.MkdirAll(tfs, trans.Merges["/home"].Old, vfs.DirPerm)).To(Succeed()) + Expect(vfs.MkdirAll(tfs, homeNewStock, vfs.DirPerm)).To(Succeed()) + + // New defaults already unpacked into the new snapshot tree. Expect(vfs.MkdirAll(tfs, newEtc, vfs.DirPerm)).To(Succeed()) Expect(tfs.WriteFile(filepath.Join(newEtc, "deletedFile"), []byte("to be deleted file"), vfs.FilePerm)).To(Succeed()) Expect(tfs.WriteFile(filepath.Join(newEtc, "modifiedFile"), []byte("new defaults modified file"), vfs.FilePerm)).To(Succeed()) diff --git a/pkg/transaction/transaction.go b/pkg/transaction/transaction.go index a765de96..45f5a46c 100644 --- a/pkg/transaction/transaction.go +++ b/pkg/transaction/transaction.go @@ -52,6 +52,7 @@ type Merge struct { Old string // old unmodified tree New string // new base tree where modifications should be applied Modified string // modified tree on top of the old tree + NewStock string // new stock snapshot path (for conflict detection) } type Transaction struct {