Merge pull request #44 from woky/list-implicit-directories

niemeyer · web-flow · commit bd27f8700cd7 · 2023-01-25T07:24:16.000+02:00
slicer: Fix checking directory paths for listing
diff --git a/internal/scripts/scripts.go b/internal/scripts/scripts.go
@@ -1,8 +1,8 @@
 package scripts
 
 import (
-	"go.starlark.net/starlark"
 	"go.starlark.net/resolve"
+	"go.starlark.net/starlark"
 
 	"fmt"
 	"io/ioutil"
@@ -98,7 +98,7 @@ func (c *ContentValue) RealPath(path string, what Check) (string, error) {
 		return "", fmt.Errorf("content path must be absolute, got: %s", path)
 	}
 	cpath := filepath.Clean(path)
-	if strings.HasSuffix(path, "/") {
+	if cpath != "/" && strings.HasSuffix(path, "/") {
 		cpath += "/"
 	}
 	if c.CheckRead != nil && what&CheckRead != 0 {
diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go
@@ -33,20 +33,24 @@ func Run(options *RunOptions) error {
 	knownPaths["/"] = true
 
 	addKnownPath := func(path string) {
-		path = filepath.Clean(path)
 		if path[0] != '/' {
 			panic("bug: tried to add relative path to known paths")
 		}
+		cleanPath := filepath.Clean(path)
+		slashPath := cleanPath
+		if path[len(path)-1] == '/' && cleanPath != "/" {
+			slashPath += "/"
+		}
 		for {
-			if _, ok := knownPaths[path]; ok {
+			if _, ok := knownPaths[slashPath]; ok {
 				break
 			}
-			knownPaths[path] = true
-			path = filepath.Dir(path)
-			if path == "/" {
+			knownPaths[slashPath] = true
+			cleanPath = filepath.Dir(cleanPath)
+			if cleanPath == "/" {
 				break
 			}
-			path += "/"
+			slashPath = cleanPath + "/"
 		}
 	}
 
@@ -84,6 +88,7 @@ func Run(options *RunOptions) error {
 		}
 		arch := archives[slice.Package].Options().Arch
 		copyrightPath := "/usr/share/doc/" + slice.Package + "/copyright"
+		addKnownPath(copyrightPath)
 		hasCopyright := false
 		for targetPath, pathInfo := range slice.Contents {
 			if targetPath == "" {
@@ -92,7 +97,9 @@ func Run(options *RunOptions) error {
 			if len(pathInfo.Arch) > 0 && !contains(pathInfo.Arch, arch) {
 				continue
 			}
-			addKnownPath(targetPath)
+			if pathInfo.Kind != setup.GlobPath {
+				addKnownPath(targetPath)
+			}
 			pathInfos[targetPath] = pathInfo
 			if pathInfo.Kind == setup.CopyPath || pathInfo.Kind == setup.GlobPath {
 				sourcePath := pathInfo.Info
@@ -225,10 +232,27 @@ func Run(options *RunOptions) error {
 		return nil
 	}
 	checkRead := func(path string) error {
+		var err error
 		if !knownPaths[path] {
-			return fmt.Errorf("cannot read file which is not selected: %s", path)
+			// we assume that path is clean and ends with slash if it designates a directory
+			if path[len(path)-1] == '/' {
+				if path == "/" {
+					panic("internal error: content root (\"/\") is not selected")
+				}
+				if knownPaths[path[:len(path)-1]] {
+					err = fmt.Errorf("content is not a directory: %s", path[:len(path)-1])
+				} else {
+					err = fmt.Errorf("cannot list directory which is not selected: %s", path)
+				}
+			} else {
+				if knownPaths[path+"/"] {
+					err = fmt.Errorf("content is not a file: %s", path)
+				} else {
+					err = fmt.Errorf("cannot read file which is not selected: %s", path)
+				}
+			}
 		}
-		return nil
+		return err
 	}
 	content := &scripts.ContentValue{
 		RootDir:    targetDirAbs,
diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go
@@ -383,7 +383,7 @@ var slicerTests = []slicerTest{{
 						content.list("/a/d")
 		`,
 	},
-	error: `slice base-files_myslice: cannot read file which is not selected: /a/d`,
+	error: `slice base-files_myslice: cannot list directory which is not selected: /a/d/`,
 }, {
 	summary: "Cannot list file path as a directory",
 	slices:  []setup.SliceKey{{"base-files", "myslice"}},
@@ -398,7 +398,7 @@ var slicerTests = []slicerTest{{
 						content.list("/a/b/c")
 		`,
 	},
-	error: `slice base-files_myslice: readdirent /a/b/c: not a directory`,
+	error: `slice base-files_myslice: content is not a directory: /a/b/c`,
 }, {
 	summary: "Can list parent directories of globs",
 	slices:  []setup.SliceKey{{"base-files", "myslice"}},
@@ -427,7 +427,7 @@ var slicerTests = []slicerTest{{
 						content.list("/etc")
 		`,
 	},
-	error: `slice base-files_myslice: cannot read file which is not selected: /etc`,
+	error: `slice base-files_myslice: cannot list directory which is not selected: /etc/`,
 }, {
 	summary: "Duplicate copyright symlink is ignored",
 	slices:  []setup.SliceKey{{"copyright-symlink-openssl", "bins"}},
@@ -453,6 +453,40 @@ var slicerTests = []slicerTest{{
 						/etc/ssl/openssl.cnf:
 		`,
 	},
+}, {
+	summary: "Can list unclean directory paths",
+	slices:  []setup.SliceKey{{"base-files", "myslice"}},
+	release: map[string]string{
+		"slices/mydir/base-files.yaml": `
+			package: base-files
+			slices:
+				myslice:
+					contents:
+						/a/b/c: {text: foo}
+						/x/y/: {make: true}
+					mutate: |
+						content.list("/////")
+						content.list("/a/")
+						content.list("/a/b/../b/")
+						content.list("/x///")
+						content.list("/x/./././y")
+		`,
+	},
+}, {
+	summary: "Cannot read directories",
+	slices:  []setup.SliceKey{{"base-files", "myslice"}},
+	release: map[string]string{
+		"slices/mydir/base-files.yaml": `
+			package: base-files
+			slices:
+				myslice:
+					contents:
+						/x/y/: {make: true}
+					mutate: |
+						content.read("/x/y")
+		`,
+	},
+	error: `slice base-files_myslice: content is not a file: /x/y`,
 }}
 
 const defaultChiselYaml = `
