diff --git a/internal/setup/setup.go b/internal/setup/setup.go index e1e4d60e..5cc74c86 100644 --- a/internal/setup/setup.go +++ b/internal/setup/setup.go @@ -122,6 +122,44 @@ type Selection struct { Slices []*Slice } +// Perfers uses the prefer relationships and returns a map from each path to +// the package where it should be extracted from. If there is no relationship +// for a given path then it will not be present on the map. +func (s *Selection) Prefers() (map[string]*Package, error) { + prefers, err := s.Release.prefers() + if err != nil { + return nil, err + } + + pathPreferredPkg := make(map[string]*Package) + for _, slice := range s.Slices { + for path := range slice.Contents { + _, hasPrefers := prefers[preferKey{preferSource, path, ""}] + if !hasPrefers { + continue + } + old, ok := pathPreferredPkg[path] + if !ok { + pathPreferredPkg[path] = s.Release.Packages[slice.Package] + continue + } + if old.Name == slice.Package { + // Skip if the package was already recorded. + continue + } + preferred, err := preferredPathPackage(path, old.Name, slice.Package, prefers) + if err != nil { + // Note: we have checked above that the path has prefers and + // they are different packages so the error cannot be + // preferNone. + return nil, err + } + pathPreferredPkg[path] = s.Release.Packages[preferred] + } + } + return pathPreferredPkg, nil +} + func ReadRelease(dir string) (*Release, error) { logDir := dir if strings.Contains(dir, "/.cache/") { @@ -148,6 +186,7 @@ func (r *Release) validate() error { } keys := []SliceKey(nil) + pathToSlice := make(map[preferKey]*Slice) // Check for info conflicts and prepare for following checks. A conflict // means that two slices attempt to extract different files or directories @@ -166,6 +205,11 @@ func (r *Release) validate() error { for _, new := range pkg.Slices { keys = append(keys, SliceKey{pkg.Name, new.Name}) for newPath, newInfo := range new.Contents { + if _, hasPrefers := prefers[preferKey{preferSource, newPath, ""}]; hasPrefers { + // If this is part of a prefer chain, store a sample slice that + // contains the path. + pathToSlice[preferKey{path: newPath, pkg: pkg.Name}] = new + } if old, ok := paths[newPath]; ok { if new.Package != old.Package { if p, err := preferredPathPackage(newPath, new.Package, old.Package, prefers); err == nil { @@ -199,22 +243,52 @@ func (r *Release) validate() error { } } + // Check for invalid prefer relationships where the package does not have + // the path. + for skey, source := range prefers { + if skey.side == preferSource && skey.pkg != "" { + // Process only the preferSource to traverse the graph only once + // and avoid repeated work. + if _, ok := pathToSlice[preferKey{path: skey.path, pkg: skey.pkg}]; !ok { + sourceSlice := pathToSlice[preferKey{path: skey.path, pkg: source}] + return fmt.Errorf("slice %s prefers package %q which does not contain path %s", sourceSlice, skey.pkg, skey.path) + } + } + } + // Check for glob and generate conflicts. for oldPath, old := range globs { oldInfo := old.Contents[oldPath] for newPath, new := range paths { if oldPath == newPath { - // Identical paths have been filtered earlier. This must be the + // Identical globs have been filtered earlier. This must be the // exact same entry. continue } - newInfo := new.Contents[newPath] - if oldInfo.Kind == GlobPath && (newInfo.Kind == GlobPath || newInfo.Kind == CopyPath) { - if new.Package == old.Package { - continue + if !strdist.GlobPath(newPath, oldPath) { + continue + } + toCheck := []*Slice{new} + pkg := new.Package + // Add all packages in the prefer relationship. If there is no + // relationship only the current package gets added. + for { + pkg = prefers[preferKey{preferSource, newPath, pkg}] + if pkg == "" { + break } + slice := pathToSlice[preferKey{path: newPath, pkg: pkg}] + toCheck = append(toCheck, slice) } - if strdist.GlobPath(newPath, oldPath) { + for _, new := range toCheck { + // It is okay to check only one slice per package because the + // content has been validated to be the same earlier. + newInfo := new.Contents[newPath] + if oldInfo.Kind == GlobPath && (newInfo.Kind == GlobPath || newInfo.Kind == CopyPath) { + if new.Package == old.Package { + continue + } + } if (old.Package > new.Package) || (old.Package == new.Package && old.Name > new.Name) || (old.Package == new.Package && old.Name == new.Name && oldPath > newPath) { old, new = new, old @@ -452,22 +526,27 @@ func (r *Release) prefers() (map[preferKey]string, error) { return prefers, nil } +// preferredPathPackage returns pkg1 if it can be reached from pkg2 following +// prefer relationships, and conversely for pkg2. If none are reachable it +// returns the preferNone error. +// +// If there is a cycle, an error is returned. func preferredPathPackage(path, pkg1, pkg2 string, prefers map[preferKey]string) (choice string, err error) { pkg1, pkg2 = sortPair(pkg1, pkg2) - prefer1, err := findPrefer(path, pkg1, pkg2, prefers) + preferred1, err := findPrefer(path, pkg2, pkg1, prefers) if err != nil { return "", err } - prefer2, err := findPrefer(path, pkg2, pkg1, prefers) + preferred2, err := findPrefer(path, pkg1, pkg2, prefers) if err != nil { return "", err } - if prefer1 && prefer2 { + if preferred1 && preferred2 { return "", fmt.Errorf("package %q is part of a prefer loop on %s", pkg1, path) - } else if prefer1 { - return pkg1, nil - } else if prefer2 { + } else if preferred2 { return pkg2, nil + } else if preferred1 { + return pkg1, nil } sample, enforce := prefers[preferKey{preferSource, path, ""}] if enforce { diff --git a/internal/setup/setup_test.go b/internal/setup/setup_test.go index 9804678a..7d5f7830 100644 --- a/internal/setup/setup_test.go +++ b/internal/setup/setup_test.go @@ -23,6 +23,7 @@ type setupTest struct { input map[string]string release *setup.Release relerror string + prefers map[string]string selslices []setup.SliceKey selection *setup.Selection selerror string @@ -2058,6 +2059,12 @@ var setupTests = []setupTest{{ relerror: "slice mypkg1_myslice1 cannot 'prefer' its own package for path /file", }, { summary: "Path conflicts with 'prefer'", + selslices: []setup.SliceKey{ + {"mypkg1", "myslice1"}, + {"mypkg1", "myslice2"}, + {"mypkg2", "myslice1"}, + {"mypkg3", "myslice1"}, + }, input: map[string]string{ "slices/mydir/mypkg1.yaml": ` package: mypkg1 @@ -2147,6 +2154,49 @@ var setupTests = []setupTest{{ }, }, }, + prefers: map[string]string{ + "/path": "mypkg3", + "/link": "mypkg1", + }, +}, { + summary: "Path conflicts with 'prefer' depends on selection", + selslices: []setup.SliceKey{ + {"mypkg1", "myslice1"}, + {"mypkg1", "myslice2"}, + {"mypkg2", "myslice1"}, + }, + input: map[string]string{ + "slices/mydir/mypkg1.yaml": ` + package: mypkg1 + slices: + myslice1: + contents: + /path: {prefer: mypkg2} + /link: {symlink: /file1} + myslice2: + contents: + /path: {prefer: mypkg2} + `, + "slices/mydir/mypkg2.yaml": ` + package: mypkg2 + slices: + myslice1: + contents: + /path: {prefer: mypkg3} + /link: {symlink: /file2, prefer: mypkg1} + `, + "slices/mydir/mypkg3.yaml": ` + package: mypkg3 + slices: + myslice1: + contents: + /path: + `, + }, + prefers: map[string]string{ + "/path": "mypkg2", + "/link": "mypkg1", + }, }, { summary: "Cannot specify same package in 'prefer'", input: map[string]string{ @@ -2172,24 +2222,21 @@ var setupTests = []setupTest{{ }, relerror: `slice mypkg_myslice path /path 'prefer' refers to undefined package "non-existent"`, }, { - /* - TODO: handle this case: - summary: "Path prefers package, but package does not have path", - input: map[string]string{ - "slices/mydir/mypkg1.yaml": ` - package: mypkg1 - slices: - myslice: - contents: - /path: {prefer: mypkg2} - `, - "slices/mydir/mypkg2.yaml": ` - package: mypkg2 - `, - }, - relerror: `slice mypkg1_myslice path /path has invalid 'prefer' "mypkg2": package does not have path /path`, - }, { - */ + summary: "Path prefers package, but package does not have path", + input: map[string]string{ + "slices/mydir/mypkg1.yaml": ` + package: mypkg1 + slices: + myslice: + contents: + /path: {prefer: mypkg2} + `, + "slices/mydir/mypkg2.yaml": ` + package: mypkg2 + `, + }, + relerror: `slice mypkg1_myslice prefers package "mypkg2" which does not contain path /path`, +}, { summary: "Path has 'prefer' cycle", input: map[string]string{ "slices/mydir/mypkg1.yaml": ` @@ -2327,29 +2374,54 @@ var setupTests = []setupTest{{ }, relerror: `packages "mypkg1" and "mypkg2" cannot both prefer "mypkg3" for /path`, }, { - /* - TODO: handle this case: - summary: "Glob paths can conflict with 'prefer' chain", - input: map[string]string{ - "slices/mydir/mypkg1.yaml": ` - package: mypkg1 - slices: - myslice: - contents: - /**: - /path: {prefer: mypkg2} - `, - "slices/mydir/mypkg2.yaml": ` - package: mypkg2 - slices: - myslice: - contents: - /path: - `, - }, - relerror: `slices mypkg1_myslice and mypkg2_myslice conflict on /\*\* and /path`, - }, { - */ + summary: "Glob paths can conflict with 'prefer' chain", + input: map[string]string{ + "slices/mydir/mypkg1.yaml": ` + package: mypkg1 + slices: + myslice1: + contents: + /**: + myslice2: + contents: + /path: {prefer: mypkg2} + `, + "slices/mydir/mypkg2.yaml": ` + package: mypkg2 + slices: + myslice: + contents: + /path: + `, + }, + // This test and the following one together ensure that both mypkg2_myslice + // and mypkg1_myslice2 are checked against the glob. + relerror: `slices mypkg1_myslice1 and mypkg2_myslice conflict on /\*\* and /path`, +}, { + summary: "Glob paths can conflict with 'prefer' chain (reverse dependency)", + input: map[string]string{ + "slices/mydir/mypkg1.yaml": ` + package: mypkg1 + slices: + myslice1: + contents: + /**: + myslice2: + contents: + /path: + `, + "slices/mydir/mypkg2.yaml": ` + package: mypkg2 + slices: + myslice: + contents: + /path: {prefer: mypkg1} + `, + }, + // This test and the previous one together ensure that both mypkg2_myslice + // and mypkg1_myslice2 are checked against the glob. + relerror: `slices mypkg1_myslice1 and mypkg2_myslice conflict on /\*\* and /path`, +}, { summary: "Slices of same package cannot have different 'prefer'", input: map[string]string{ "slices/mydir/mypkg1.yaml": ` @@ -2421,6 +2493,9 @@ func runParseReleaseTests(c *C, tests []setupTest) { if _, ok := test.input["chisel.yaml"]; !ok { test.input["chisel.yaml"] = string(defaultChiselYaml) } + if test.prefers == nil { + test.prefers = make(map[string]string) + } dir := c.MkDir() for path, data := range test.input { @@ -2457,6 +2532,13 @@ func runParseReleaseTests(c *C, tests []setupTest) { c.Assert(err, IsNil) } c.Assert(selection.Release, Equals, release) + rawPrefers, err := selection.Prefers() + c.Assert(err, IsNil) + prefers := make(map[string]string) + for path, pkg := range rawPrefers { + prefers[path] = pkg.Name + } + c.Assert(prefers, DeepEquals, test.prefers) selection.Release = nil if test.selection != nil { c.Assert(selection, DeepEquals, test.selection) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index b720f6ac..7cb47ccb 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -95,6 +95,11 @@ func Run(options *RunOptions) error { return err } + prefers, err := options.Selection.Prefers() + if err != nil { + return err + } + // Build information to process the selection. extract := make(map[string]map[string][]deb.ExtractInfo) for _, slice := range options.Selection.Slices { @@ -111,6 +116,9 @@ func Run(options *RunOptions) error { if len(pathInfo.Arch) > 0 && !slices.Contains(pathInfo.Arch, arch) { continue } + if preferredPkg, ok := prefers[targetPath]; ok && preferredPkg.Name != slice.Package { + continue + } if pathInfo.Kind == setup.CopyPath || pathInfo.Kind == setup.GlobPath { sourcePath := pathInfo.Info @@ -254,6 +262,9 @@ func Run(options *RunOptions) error { pathInfo.Kind == setup.GeneratePath { continue } + if preferredPkg, ok := prefers[relPath]; ok && preferredPkg.Name != slice.Package { + continue + } relPaths[relPath] = append(relPaths[relPath], slice) } } diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 2fcb9d9e..ce4b0c00 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -1783,6 +1783,67 @@ var slicerTests = []slicerTest{{ `, }, error: `cannot extract from package "test-package": cannot create path /[a-z0-9\-\/]*/file outside of root /[a-z0-9\-\/]*`, +}, { + summary: "Extract conflicting paths with prefer from proper package", + slices: []setup.SliceKey{ + {"test-package1", "myslice"}, + {"test-package2", "myslice"}, + }, + pkgs: []*testutil.TestPackage{{ + Name: "test-package1", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "foo"), + }), + }, { + Name: "test-package2", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + testutil.Reg(0644, "./file", "bar"), + }), + }, { + Name: "test-package3", + Data: testutil.MustMakeDeb([]testutil.TarEntry{ + testutil.Dir(0755, "./"), + }), + }}, + release: map[string]string{ + "slices/mydir/test-package1.yaml": ` + package: test-package1 + slices: + myslice: + contents: + /file: {prefer: test-package2} + /link: {symlink: /file1} + /text: {text: foo, prefer: test-package3} + `, + "slices/mydir/test-package2.yaml": ` + package: test-package2 + slices: + myslice: + contents: + /file: + /link: {symlink: /file2, prefer: test-package3} + `, + "slices/mydir/test-package3.yaml": ` + package: test-package3 + slices: + myslice: + contents: + /link: {symlink: /file2, prefer: test-package1} + /text: {text: bar} + `, + }, + filesystem: map[string]string{ + "/file": "file 0644 fcde2b2e", + "/link": "symlink /file1", + "/text": "file 0644 2c26b46b", + }, + manifestPaths: map[string]string{ + "/file": "file 0644 fcde2b2e {test-package2_myslice}", + "/link": "symlink /file1 {test-package1_myslice}", + "/text": "file 0644 2c26b46b {test-package1_myslice}", + }, }} var defaultChiselYaml = `