File 0024-compare-move-FreeBSD-loose-keyword-comparisons-to-go.patch of Package go-mtree
From 1ce6aa8db51fa0c3b76be2cff88252a28c5284b5 Mon Sep 17 00:00:00 2001
From: Aleksa Sarai <cyphar@cyphar.com>
Date: Tue, 16 Sep 2025 22:27:20 +1000
Subject: [PATCH 24/25] compare: move FreeBSD loose keyword comparisons to
gomtree command
FreeBSD has quite unfortunate behaviour when dealing with keywords that
are missing in one of the manifests being compared -- namely, they
ignore these instances.
Commit 21723a3974bc ("*: fix comparison of missing keywords") re-added
this behaviour after the introduction of the Compare API, but
unfortunately it was implemented in the Compare API itself -- meaning
that library users (which didn't want this behaviour) were silently
opted into it.
This patch moves the behaviour to the command-line, where it belongs
(a future patch in this series will allow users to opt-out of this
unfortunate behaviour, as well as some other unfortunate FreeBSD
compatibility behaviours).
Fixes: 21723a3974bc ("*: fix comparison of missing keywords")
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
---
cmd/gomtree/cmd/validate.go | 20 +++++++++++
compare.go | 26 --------------
compare_test.go | 72 +++++++++++++++++++++++++++++++++++++
3 files changed, 92 insertions(+), 26 deletions(-)
diff --git a/cmd/gomtree/cmd/validate.go b/cmd/gomtree/cmd/validate.go
index 7186be3fbe3f..bbff281057d4 100644
--- a/cmd/gomtree/cmd/validate.go
+++ b/cmd/gomtree/cmd/validate.go
@@ -368,6 +368,7 @@ func validateAction(c *cli.Context) error {
if isTarSpec(specDh) || c.String("tar") != "" {
filters = append(filters, tarKeywordFilter)
}
+ filters = append(filters, freebsdCompatKeywordFilter)
res = filterDeltas(res, filters...)
if len(res) > 0 {
@@ -458,6 +459,25 @@ func tarKeywordFilter(delta *mtree.InodeDelta) bool {
return true
}
+// freebsdCompatKeywordFilter removes any deltas where a key is not present in
+// both manifests being compared. This is necessary for compatibility with
+// FreeBSD's mtree(8) but is generally undesireable for most users.
+func freebsdCompatKeywordFilter(delta *mtree.InodeDelta) bool {
+ if delta.Type() != mtree.Modified {
+ return true
+ }
+ keys := delta.DiffPtr()
+ *keys = slices.DeleteFunc(*keys, func(kd mtree.KeyDelta) bool {
+ if kd.Name().Prefix() == "xattr" {
+ // Even in FreeBSD compatibility mode, any xattr changes should
+ // still be treated as a proper change and not filtered out.
+ return false
+ }
+ return kd.Type() != mtree.Modified
+ })
+ return true
+}
+
type deltaFilterFn func(*mtree.InodeDelta) bool
// filterDeltas takes the set of deltas generated by mtree and applies the
diff --git a/compare.go b/compare.go
index 67d92fc81604..90916dd60711 100644
--- a/compare.go
+++ b/compare.go
@@ -4,7 +4,6 @@ import (
"encoding/json"
"fmt"
"iter"
- "maps"
"slices"
"github.com/sirupsen/logrus"
@@ -247,31 +246,6 @@ func compareEntry(oldEntry, newEntry Entry) ([]KeyDelta, error) {
newKeys = newEntry.allKeysMap()
)
- // Delete any keys which are not present in both maps.
- keyFilterFn := func(otherMap map[Keyword]KeyVal) func(Keyword, KeyVal) bool {
- return func(k Keyword, _ KeyVal) bool {
- switch {
- case k.Prefix() == "xattr":
- // xattrs are presented as one keyword to users but are actually
- // implemented as separate keywords and so we should always include
- // them (even if the same xattr is not present in both sides).
- // TODO: I actually think this is not inline with the original
- // purpose of this code, but I'm leaving it as-is to not not
- // introduce bugs.
- return false
- case k == "time" || k == "tar_time":
- // These have special handling later.
- return false
- default:
- // Drop keys which do not exist in the other entry.
- _, ok := otherMap[k]
- return !ok
- }
- }
- }
- maps.DeleteFunc(oldKeys, keyFilterFn(newKeys))
- maps.DeleteFunc(newKeys, keyFilterFn(oldKeys))
-
// If both tar_time and time were specified in the set of keys, we have to
// convert the "time" entries to "tar_time" to allow for tar archive
// manifests to be compared with proper filesystem manifests.
diff --git a/compare_test.go b/compare_test.go
index d8e07107af7e..19539aa1012e 100644
--- a/compare_test.go
+++ b/compare_test.go
@@ -7,6 +7,7 @@ import (
"io"
"os"
"path/filepath"
+ "slices"
"testing"
"time"
@@ -230,6 +231,77 @@ func TestCompareKeySubset(t *testing.T) {
}
}
+func TestCompareKeyDelta(t *testing.T) {
+ dir := t.TempDir()
+
+ // Create a bunch of objects.
+ tmpfile := filepath.Join(dir, "tmpfile")
+ require.NoError(t, os.WriteFile(tmpfile, []byte("some content here"), 0666))
+
+ tmpdir := filepath.Join(dir, "testdir")
+ require.NoError(t, os.Mkdir(tmpdir, 0755))
+
+ tmpsubfile := filepath.Join(tmpdir, "anotherfile")
+ require.NoError(t, os.WriteFile(tmpsubfile, []byte("aaa"), 0666))
+
+ // Walk the current state.
+ manifestKeywords := append(DefaultKeywords[:], "sha1digest")
+ old, err := Walk(dir, nil, manifestKeywords, nil)
+ require.NoErrorf(t, err, "walk %s", dir)
+
+ t.Run("Extra-Key", func(t *testing.T) {
+ extraKeyword := Keyword("sha256digest")
+ newManifestKeywords := append(manifestKeywords[:], extraKeyword)
+
+ new, err := Walk(dir, nil, newManifestKeywords, nil)
+ require.NoErrorf(t, err, "walk %s", dir)
+
+ diffs, err := Compare(old, new, nil)
+ require.NoError(t, err, "compare")
+
+ assert.NotEmpty(t, diffs, "extra keys in manifest should result in deltas")
+ for _, diff := range diffs {
+ if assert.Equal(t, Modified, diff.Type(), "extra keyword diff element should be 'modified'") {
+ kds := diff.Diff()
+ if assert.Len(t, kds, 1, "should only get a single key delta") {
+ kd := kds[0]
+ assert.Equalf(t, Extra, kd.Type(), "key %q", kd.Name())
+ assert.Equal(t, extraKeyword, kd.Name())
+ assert.Nil(t, kd.Old(), "Old for extra keyword delta")
+ assert.NotNil(t, kd.New(), "New for extra keyword delta")
+ }
+ }
+ }
+ })
+
+ t.Run("Missing-Key", func(t *testing.T) {
+ missingKeyword := Keyword("sha1digest")
+ newManifestKeywords := slices.DeleteFunc(manifestKeywords[:], func(kw Keyword) bool {
+ return kw == missingKeyword
+ })
+
+ new, err := Walk(dir, nil, newManifestKeywords, nil)
+ require.NoErrorf(t, err, "walk %s", dir)
+
+ diffs, err := Compare(old, new, nil)
+ require.NoError(t, err, "compare")
+
+ assert.NotEmpty(t, diffs, "missing keys in manifest should result in deltas")
+ for _, diff := range diffs {
+ if assert.Equal(t, Modified, diff.Type(), "missing keyword diff element should be 'modified'") {
+ kds := diff.Diff()
+ if assert.Len(t, kds, 1, "should only get a single key delta") {
+ kd := kds[0]
+ assert.Equalf(t, Missing, kd.Type(), "key %q", kd.Name())
+ assert.Equal(t, missingKeyword, kd.Name())
+ assert.NotNil(t, kd.Old(), "Old for missing keyword delta")
+ assert.Nil(t, kd.New(), "New for missing keyword delta")
+ }
+ }
+ }
+ })
+}
+
//gocyclo:ignore
func TestTarCompare(t *testing.T) {
dir := t.TempDir()
--
2.51.0
