Merge pull request #295 from paketo-community/gh-293

Implement more robust member parser
paketo-community · Mar 29, 2024 · a408716 · a408716
2 parents 446a292 + 901aad7
commit a408716
Show file tree

Hide file tree

Showing 2 changed files with 161 additions and 40 deletions.
diff --git a/runner/runners.go b/runner/runners.go
@@ -221,13 +221,15 @@ func (c CargoRunner) WorkspaceMembers(srcDir string, destLayer libcnb.Layer) ([]
 
 	var paths []url.URL
 	for _, workspace := range m.WorkspaceMembers {
-		// This is OK because the workspace member format is `package-name package-version (url)` and
-		//   none of name, version or URL may contain a space & be valid
-		parts := strings.SplitN(workspace, " ", 3)
-		if len(filterMap) > 0 && filterMap[strings.TrimSpace(parts[0])] || len(filterMap) == 0 {
-			path, err := url.Parse(strings.TrimSuffix(strings.TrimPrefix(parts[2], "("), ")"))
+		pkgName, _, pathUrl, err := ParseWorkspaceMember(workspace)
+		if err != nil {
+			return nil, fmt.Errorf("unable to parse: %w", err)
+		}
+
+		if len(filterMap) > 0 && filterMap[strings.TrimSpace(pkgName)] || len(filterMap) == 0 {
+			path, err := url.Parse(pathUrl)
 			if err != nil {
-				return nil, fmt.Errorf("unable to parse URL %s: %w", workspace, err)
+				return nil, fmt.Errorf("unable to parse path URL %s: %w", workspace, err)
 			}
 			paths = append(paths, *path)
 		}
@@ -236,6 +238,36 @@ func (c CargoRunner) WorkspaceMembers(srcDir string, destLayer libcnb.Layer) ([]
 	return paths, nil
 }
 
+// parseWorkspaceMember parses a workspace member which can be in a couple of different formats
+//
+//	pre-1.77: `package-name package-version (url)`, like `function 0.1.0 (path+file:///Users/dmikusa/Downloads/fn-rs)`
+//	1.77+: `url#package-name@package-version` like `path+file:///Users/dmikusa/Downloads/fn-rs#[email protected]`
+//
+// returns the package name, version, URL, and optional error in that order
+func ParseWorkspaceMember(workspaceMember string) (string, string, string, error) {
+	if strings.HasPrefix(workspaceMember, "path+file://") {
+		half := strings.SplitN(workspaceMember, "#", 2)
+		if len(half) != 2 {
+			return "", "", "", fmt.Errorf("unable to parse workspace member [%s], missing `#`", workspaceMember)
+		}
+
+		otherHalf := strings.SplitN(half[1], "@", 2)
+		if len(otherHalf) != 2 {
+			return "", "", "", fmt.Errorf("unable to parse workspace member [%s], missing `@`", workspaceMember)
+		}
+
+		return strings.TrimSpace(otherHalf[0]), strings.TrimSpace(otherHalf[1]), strings.TrimSpace(half[0]), nil
+	} else {
+		// This is OK because the workspace member format is `package-name package-version (url)` and
+		//   none of name, version or URL may contain a space & be valid
+		parts := strings.SplitN(workspaceMember, " ", 3)
+		if len(parts) != 3 {
+			return "", "", "", fmt.Errorf("unable to parse workspace member [%s], unexpected format", workspaceMember)
+		}
+		return strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]), strings.TrimSuffix(strings.TrimPrefix(parts[2], "("), ")"), nil
+	}
+}
+
 // ProjectTargets loads the members from the project workspace
 func (c CargoRunner) ProjectTargets(srcDir string) ([]string, error) {
 	m, err := c.fetchCargoMetadata(srcDir)

diff --git a/runner/runners_test.go b/runner/runners_test.go
@@ -377,52 +377,104 @@ func testRunners(t *testing.T, context spec.G, it spec.S) {
 		})
 
 		context("and there is metadata", func() {
-			it("parses the member paths from metadata", func() {
-				logBuf := bytes.Buffer{}
-				logger := bard.NewLogger(&logBuf)
+			context("pre-rust 1.77.0", func() {
+				it("parses the member paths from metadata", func() {
+					logBuf := bytes.Buffer{}
+					logger := bard.NewLogger(&logBuf)
 
-				metadata := BuildMetadata("/workspace",
-					[]string{
-						"basics 2.0.0 (path+file:///workspace/basics)",
-						"todo 1.2.0 (path+file:///workspace/todo)",
-						"routes 0.5.0 (path+file:///workspace/routes)",
-						"jokes 1.5.6 (path+file:///workspace/jokes)",
+					metadata := BuildMetadata("/workspace",
+						[]string{
+							"basics 2.0.0 (path+file:///workspace/basics)",
+							"todo 1.2.0 (path+file:///workspace/todo)",
+							"routes 0.5.0 (path+file:///workspace/routes)",
+							"jokes 1.5.6 (path+file:///workspace/jokes)",
+						})
+
+					executor.On("Execute", mock.MatchedBy(func(ex effect.Execution) bool {
+						Expect(ex.Args).To(Equal([]string{"metadata", "--format-version=1", "--no-deps"}))
+						return true
+					})).Return(func(ex effect.Execution) error {
+						_, err := ex.Stdout.Write([]byte(metadata))
+						Expect(err).ToNot(HaveOccurred())
+						return nil
 					})
 
-				executor.On("Execute", mock.MatchedBy(func(ex effect.Execution) bool {
-					Expect(ex.Args).To(Equal([]string{"metadata", "--format-version=1", "--no-deps"}))
-					return true
-				})).Return(func(ex effect.Execution) error {
-					_, err := ex.Stdout.Write([]byte(metadata))
+					runner := runner.NewCargoRunner(
+						runner.WithCargoHome(cargoHome),
+						runner.WithExecutor(executor),
+						runner.WithLogger(logger))
+
+					urls, err := runner.WorkspaceMembers(workingDir, destLayer)
 					Expect(err).ToNot(HaveOccurred())
-					return nil
+
+					Expect(urls).To(HaveLen(4))
+
+					url, err := url.Parse("path+file:///workspace/basics")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(urls[0]).To(Equal(*url))
+
+					url, err = url.Parse("path+file:///workspace/todo")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(urls[1]).To(Equal(*url))
+
+					url, err = url.Parse("path+file:///workspace/routes")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(urls[2]).To(Equal(*url))
+
+					url, err = url.Parse("path+file:///workspace/jokes")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(urls[3]).To(Equal(*url))
 				})
+			})
 
-				runner := runner.NewCargoRunner(
-					runner.WithCargoHome(cargoHome),
-					runner.WithExecutor(executor),
-					runner.WithLogger(logger))
+			context("post-rust 1.77.0", func() {
+				it("parses the member paths from metadata", func() {
+					logBuf := bytes.Buffer{}
+					logger := bard.NewLogger(&logBuf)
 
-				urls, err := runner.WorkspaceMembers(workingDir, destLayer)
-				Expect(err).ToNot(HaveOccurred())
+					metadata := BuildMetadata("/workspace",
+						[]string{
+							"path+file:///workspace/basics#[email protected]",
+							"path+file:///workspace/todo#[email protected]",
+							"path+file:///workspace/routes#[email protected]",
+							"path+file:///workspace/jokes#[email protected]",
+						})
 
-				Expect(urls).To(HaveLen(4))
+					executor.On("Execute", mock.MatchedBy(func(ex effect.Execution) bool {
+						Expect(ex.Args).To(Equal([]string{"metadata", "--format-version=1", "--no-deps"}))
+						return true
+					})).Return(func(ex effect.Execution) error {
+						_, err := ex.Stdout.Write([]byte(metadata))
+						Expect(err).ToNot(HaveOccurred())
+						return nil
+					})
 
-				url, err := url.Parse("path+file:///workspace/basics")
-				Expect(err).ToNot(HaveOccurred())
-				Expect(urls[0]).To(Equal(*url))
+					runner := runner.NewCargoRunner(
+						runner.WithCargoHome(cargoHome),
+						runner.WithExecutor(executor),
+						runner.WithLogger(logger))
 
-				url, err = url.Parse("path+file:///workspace/todo")
-				Expect(err).ToNot(HaveOccurred())
-				Expect(urls[1]).To(Equal(*url))
+					urls, err := runner.WorkspaceMembers(workingDir, destLayer)
+					Expect(err).ToNot(HaveOccurred())
 
-				url, err = url.Parse("path+file:///workspace/routes")
-				Expect(err).ToNot(HaveOccurred())
-				Expect(urls[2]).To(Equal(*url))
+					Expect(urls).To(HaveLen(4))
 
-				url, err = url.Parse("path+file:///workspace/jokes")
-				Expect(err).ToNot(HaveOccurred())
-				Expect(urls[3]).To(Equal(*url))
+					url, err := url.Parse("path+file:///workspace/basics")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(urls[0]).To(Equal(*url))
+
+					url, err = url.Parse("path+file:///workspace/todo")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(urls[1]).To(Equal(*url))
+
+					url, err = url.Parse("path+file:///workspace/routes")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(urls[2]).To(Equal(*url))
+
+					url, err = url.Parse("path+file:///workspace/jokes")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(urls[3]).To(Equal(*url))
+				})
 			})
 
 			context("member filter is set", func() {
@@ -720,6 +772,43 @@ func testRunners(t *testing.T, context spec.G, it spec.S) {
 			})
 		})
 	})
+
+	context("parses workspace members", func() {
+		context("pre-rust 1.77.0", func() {
+			it("parses them", func() {
+				pkgName, version, url, err := runner.ParseWorkspaceMember("basics 2.0.0 (path+file:///workspace/basics)")
+				Expect(err).ToNot(HaveOccurred())
+				Expect(pkgName).To(Equal("basics"))
+				Expect(version).To(Equal("2.0.0"))
+				Expect(url).To(Equal("path+file:///workspace/basics"))
+			})
+
+			it("fails to parse because not enough spaces", func() {
+				_, _, _, err := runner.ParseWorkspaceMember("basics 2.0.0")
+				Expect(err).To(MatchError("unable to parse workspace member [basics 2.0.0], unexpected format"))
+			})
+		})
+
+		context("pre-rust 1.77.0", func() {
+			it("parses them", func() {
+				pkgName, version, url, err := runner.ParseWorkspaceMember("path+file:///workspace/basics#[email protected]")
+				Expect(err).ToNot(HaveOccurred())
+				Expect(pkgName).To(Equal("basics"))
+				Expect(version).To(Equal("2.0.0"))
+				Expect(url).To(Equal("path+file:///workspace/basics"))
+			})
+
+			it("fails to parse because there is no hash sign", func() {
+				_, _, _, err := runner.ParseWorkspaceMember("path+file:///workspace/basics")
+				Expect(err).To(MatchError("unable to parse workspace member [path+file:///workspace/basics], missing `#`"))
+			})
+
+			it("fails to parse because there is no at sign", func() {
+				_, _, _, err := runner.ParseWorkspaceMember("path+file:///workspace/basics#foo")
+				Expect(err).To(MatchError("unable to parse workspace member [path+file:///workspace/basics#foo], missing `@`"))
+			})
+		})
+	})
 }
 
 type buildMetadata struct {