feat: finish support for path conflicts with 'prefer'

internal/setup/setup.go

@@ -122,6 +122,46 @@ 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
+			}
+			if preferred == old.Name {
+				pathPreferredPkg[path] = s.Release.Packages[slice.Package]
+			}
+		}
+	}
+	return pathPreferredPkg, nil
+}
+
 func ReadRelease(dir string) (*Release, error) {
 	logDir := dir
 	if strings.Contains(dir, "/.cache/") {
@@ -148,6 +188,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 +207,8 @@ func (r *Release) validate() error {
 		for _, new := range pkg.Slices {
 			keys = append(keys, SliceKey{pkg.Name, new.Name})
 			for newPath, newInfo := range new.Contents {
+				// 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 +242,49 @@ 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 == "" {
+			// preferTarget packages have the path by construction.
+			continue
+		}
+		if _, ok := pathToSlice[preferKey{path: skey.path, pkg: skey.pkg}]; !ok {
+			sourceSlice := pathToSlice[preferKey{path: skey.path, pkg: source}]
+			return fmt.Errorf("slice %s path %s 'prefer' refers to invalid package %q: package does not have path %s", sourceSlice, skey.path, 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
 			}
-			if strdist.GlobPath(newPath, oldPath) {
+			toCheck := []*Slice{}
+			pkg := new.Package
+			// Add all packages in the prefer relationship. If there is no
+			// relationship only the current package gets added.
+			for pkg != "" {
+				slice, _ := pathToSlice[preferKey{path: newPath, pkg: pkg}]
+				toCheck = append(toCheck, slice)
+				pkg = prefers[preferKey{preferTarget, newPath, pkg}]
+			}
+			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,6 +522,11 @@ func (r *Release) prefers() (map[preferKey]string, error) {
 	return prefers, nil
 }
 
+// preferredPathPackage returns pkg1 if one can reach pkg2 from pkg1 using
+// 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)

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 path /path 'prefer' refers to invalid package "mypkg2": package does not have path /path`,
+}, {
 	summary: "Path has 'prefer' cycle",
 	input: map[string]string{
 		"slices/mydir/mypkg1.yaml": `
@@ -2327,29 +2374,26 @@ 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:
+				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: "Slices of same package cannot have different 'prefer'",
 	input: map[string]string{
 		"slices/mydir/mypkg1.yaml": `
@@ -2421,6 +2465,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 +2504,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)

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)
 		}
 	}