Skip to content

Commit

Permalink
Map target paths and target exclude paths for archive/git refs (#3003)
Browse files Browse the repository at this point in the history
In CLI versions pre-1.32.0, for archive and git refs, we would map
the root of the bucket of archive and git refs to the `subDirPath`,
which is inconsistent with the behavior for dir refs (which maps
target paths and target exclude paths to the execution context).

In 1.32.0, we made changes to handle subDirPath, target paths,
and target exclude paths in a unified way, however this is not
backwards compatible with pre-1.32.0 versions. So, in order
to be backwards compatible, we need to remap the target
paths and target exclude paths.

I explored an option to check of the subDirPath contains
the target paths and target exclude paths before remapping,
however this is brittle, since we could hit an edge case with
directories that have the same name nested:

```
.
└── proto
    └── foo
        └── foo
```

Tests have been adjusted to reflect this change.

---------

Co-authored-by: Nick Snyder <[email protected]>
Co-authored-by: bufdev <[email protected]>
  • Loading branch information
3 people authored May 21, 2024
1 parent d88de38 commit a6ff911
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 9 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

## [Unreleased]

- No changes yet.
- Fix archive and git inputs so that `--path` and `--exclude-path` paths are relative to
the `#subdir` rather than the root of the input. This fixes an unintended behavior change
that was introduced in `v1.32.0`.

## [v1.32.0] - 2024-05-16

Expand Down
4 changes: 2 additions & 2 deletions data/template/buf.go.gen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ inputs:
tag: v27.0-rc1
subdir: src
paths:
- src/google/protobuf/cpp_features.proto
- google/protobuf/cpp_features.proto
- git_repo: https://github.com/protocolbuffers/protobuf.git
tag: v27.0-rc1
subdir: java/core/src/main/resources
paths:
- java/core/src/main/resources/google/protobuf/java_features.proto
- google/protobuf/java_features.proto
32 changes: 32 additions & 0 deletions private/buf/buffetch/internal/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,15 @@ func getReadBucketCloserForBucket(
if err := validatePaths(inputSubDirPath, targetPaths, targetExcludePaths); err != nil {
return nil, nil, err
}
// For archive and git refs, target paths and target exclude paths are expected to be
// mapped to the inputSubDirPath rather than the execution context.
// This affects buftarget when checking and mapping paths against the controlling
// workspace, so we need to ensure that all paths are properly mapped.
targetPaths, targetExcludePaths = mapTargetPathsAndTargetExcludePathsForArchiveAndGitRefs(
inputSubDirPath,
targetPaths,
targetExcludePaths,
)
bucketTargeting, err := buftarget.NewBucketTargeting(
ctx,
logger,
Expand Down Expand Up @@ -904,6 +913,29 @@ func validatePaths(
return nil
}

func mapTargetPathsAndTargetExcludePathsForArchiveAndGitRefs(
inputSubDirPath string,
targetPaths []string,
targetExcludePaths []string,
) ([]string, []string) {
// No need to remap
if inputSubDirPath == "." {
return targetPaths, targetExcludePaths
}
return slicesext.Map(
targetPaths,
func(targetPath string) string {
return normalpath.Join(inputSubDirPath, targetPath)
},
),
slicesext.Map(
targetExcludePaths,
func(targetExcludePath string) string {
return normalpath.Join(inputSubDirPath, targetExcludePath)
},
)
}

type getFileOptions struct {
keepFileCompression bool
}
Expand Down
4 changes: 2 additions & 2 deletions private/buf/cmd/buf/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ func TestWorkspaceArchiveDir(t *testing.T) {
"lint",
filepath.Join(zipDir, "archive.zip#subdir=proto"),
"--path",
filepath.Join("proto", "rpc.proto"),
filepath.Join("rpc.proto"),
)
}
}
Expand Down Expand Up @@ -342,7 +342,7 @@ func TestWorkspaceNestedArchive(t *testing.T) {
"lint",
filepath.Join(zipDir, "archive.zip#subdir=proto/internal"),
"--path",
filepath.Join("proto", "internal", "internal.proto"),
filepath.Join("internal.proto"),
)
}
}
Expand Down
8 changes: 4 additions & 4 deletions private/buf/cmd/buf/workspace_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func TestWorkspaceGit(t *testing.T) {
"build",
"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
"--path",
filepath.Join("private", "buf", "cmd", "buf", "testdata", "workspace", "success", "dir", "proto", "rpc.proto"),
filepath.Join("rpc.proto"),
)
testRunStdout(
t,
Expand Down Expand Up @@ -188,7 +188,7 @@ func TestWorkspaceGit(t *testing.T) {
"lint",
"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/dir/proto",
"--path",
filepath.Join("private", "buf", "cmd", "buf", "testdata", "workspace", "success", "dir", "proto", "rpc.proto"),
filepath.Join("rpc.proto"),
)
testRunStdout(
t,
Expand All @@ -206,7 +206,7 @@ func TestWorkspaceGit(t *testing.T) {
"build",
"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/v2/dir/proto",
"--path",
filepath.Join("private", "buf", "cmd", "buf", "testdata", "workspace", "success", "v2", "dir", "proto", "rpc.proto"),
filepath.Join("rpc.proto"),
)
testRunStdout(
t,
Expand Down Expand Up @@ -234,6 +234,6 @@ func TestWorkspaceGit(t *testing.T) {
"lint",
"../../../../.git#ref=HEAD,subdir=private/buf/cmd/buf/testdata/workspace/success/v2/dir/proto",
"--path",
filepath.Join("private", "buf", "cmd", "buf", "testdata", "workspace", "success", "v2", "dir", "proto", "rpc.proto"),
filepath.Join("rpc.proto"),
)
}

0 comments on commit a6ff911

Please sign in to comment.