diff --git a/cmd/gomtree/cmd/validate.go b/cmd/gomtree/cmd/validate.go index f3b5fa6..7186be3 100644 --- a/cmd/gomtree/cmd/validate.go +++ b/cmd/gomtree/cmd/validate.go @@ -362,11 +362,15 @@ func validateAction(c *cli.Context) error { if err != nil { return err } - if res != nil { - if isTarSpec(specDh) || c.String("tar") != "" { - res = filterMissingKeywords(res) - } + // Apply filters. + var filters []deltaFilterFn + if isTarSpec(specDh) || c.String("tar") != "" { + filters = append(filters, tarKeywordFilter) + } + res = filterDeltas(res, filters...) + + if len(res) > 0 { out := formatFunc(res) if _, err := os.Stdout.Write([]byte(out)); err != nil { return err @@ -430,48 +434,51 @@ func isDirEntry(e mtree.Entry) bool { return false } -// filterMissingKeywords is a fairly annoying hack to get around the fact that -// tar archive manifest generation has certain unsolveable problems regarding -// certain keywords. For example, the size=... keyword cannot be implemented -// for directories in a tar archive (which causes Missing errors for that -// keyword). -// -// This function just removes all instances of Missing errors for keywords. -// This makes certain assumptions about the type of issues tar archives have. -// Only call this on tar archive manifest comparisons. -func filterMissingKeywords(diffs []mtree.InodeDelta) []mtree.InodeDelta { - newDiffs := []mtree.InodeDelta{} -loop: - for _, diff := range diffs { - if diff.Type() == mtree.Modified { - // We only apply this filtering to directories. - // NOTE: This will probably break if someone drops the size keyword. - if isDirEntry(*diff.Old()) || isDirEntry(*diff.New()) { - // If this applies to '.' then we just filter everything - // (meaning we remove this entry). This is because note all tar - // archives include a '.' entry. Which makes checking this not - // practical. - if diff.Path() == "." { - continue - } +// tarKeywordFilter is a filter for diffs produced where one half is a tar +// archive. tar archive manifests do not have a "size" key associated with +// directories (due to limitations in manifest generation for tar archives) and +// so any deltas due to size missing should be removed. +func tarKeywordFilter(delta *mtree.InodeDelta) bool { + if delta.Path() == "." { + // Not all tar archives include a root entry so we should skip that + // entry if we run into a diff that claims there is an issue with + // it. + return false + } + if delta.Type() != mtree.Modified { + return true + } + // Strip out "size" entries for directory entries. + if isDirEntry(*delta.Old()) || isDirEntry(*delta.New()) { + keys := delta.DiffPtr() + *keys = slices.DeleteFunc(*keys, func(kd mtree.KeyDelta) bool { + return kd.Name() == "size" + }) + } + return true +} - // Only filter out the size keyword. - keys := diff.DiffPtr() - *keys = slices.DeleteFunc(*keys, func(kd mtree.KeyDelta) bool { - return kd.Name() == "size" - }) - // If there are no key deltas left after filtering, the entry - // should be filtered out entirely. - if len(*keys) == 0 { - continue loop - } +type deltaFilterFn func(*mtree.InodeDelta) bool + +// filterDeltas takes the set of deltas generated by mtree and applies the +// given set of filters to it. +func filterDeltas(deltas []mtree.InodeDelta, filters ...deltaFilterFn) []mtree.InodeDelta { + filtered := make([]mtree.InodeDelta, 0, len(deltas)) +next: + for _, delta := range deltas { + for _, filter := range filters { + if !filter(&delta) { + continue next } } - - // If we got here, append to the new set. - newDiffs = append(newDiffs, diff) + // Some filters might modify the entry to remove keyword deltas -- + // if there are no deltas left then we should skip the entry entirely. + if delta.Type() == mtree.Modified && len(delta.Diff()) == 0 { + continue next + } + filtered = append(filtered, delta) } - return newDiffs + return filtered } // isTarSpec returns whether the spec provided came from the tar generator.