From 901aad78e3dcc45e40dafaae7dd29c82f4647ca9 Mon Sep 17 00:00:00 2001 From: Daniel Mikusa Date: Thu, 28 Mar 2024 23:00:44 -0400 Subject: [PATCH] Implement more robust member parser The metadata for the format of the workspace members has changed in Rust 1.77.0. This change makes the parser more robust and enables it to parse both the old and new format. Signed-off-by: Daniel Mikusa --- runner/runners.go | 44 ++++++++++-- runner/runners_test.go | 157 ++++++++++++++++++++++++++++++++--------- 2 files changed, 161 insertions(+), 40 deletions(-) diff --git a/runner/runners.go b/runner/runners.go index 92ba9f2..701456f 100644 --- 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#function@0.1.0` +// +// 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 index 04bf483..d686e30 100644 --- 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#basics@2.0.0", + "path+file:///workspace/todo#todo@1.2.0", + "path+file:///workspace/routes#routes@0.5.0", + "path+file:///workspace/jokes#jokes@1.5.6", + }) - 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#basics@2.0.0") + 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 {