Skip to content

Commit

Permalink
Return a more user-friendly error in workspace targeting for overlaps
Browse files Browse the repository at this point in the history
  • Loading branch information
doriable committed May 15, 2024
1 parent bcf427c commit e7606d7
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 3 deletions.
27 changes: 24 additions & 3 deletions private/buf/bufworkspace/workspace_targeting.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func v2WorkspaceTargeting(
// Check if the input is overlapping within a module dir path. If so, return a nicer
// error. In the future, we want to remove special treatment for input dir, and it
// should be treated just like any target path.
return nil, checkForOverlap(bucketTargeting.SubDirPath(), moduleDirPaths)
return nil, checkForOverlap(ctx, bucket, bucketTargeting.SubDirPath(), moduleDirPaths)
}
if !hadIsTargetModule {
// It would be nice to have a better error message than this in the long term.
Expand Down Expand Up @@ -327,7 +327,7 @@ func v1WorkspaceTargeting(
// Check if the input is overlapping within a module dir path. If so, return a nicer
// error. In the future, we want to remove special treatment for input dir, and it
// should be treated just like any target path.
return nil, checkForOverlap(bucketTargeting.SubDirPath(), moduleDirPaths)
return nil, checkForOverlap(ctx, bucket, bucketTargeting.SubDirPath(), moduleDirPaths)
}
if !hadIsTargetModule {
// It would be nice to have a better error message than this in the long term.
Expand Down Expand Up @@ -681,9 +681,30 @@ func checkForControllingWorkspaceOrV1Module(
return fallbackV1Module, nil
}

func checkForOverlap(inputPath string, moduleDirPaths []string) error {
func checkForOverlap(
ctx context.Context,
bucket storage.ReadBucket,
inputPath string,
moduleDirPaths []string,
) error {
for _, moduleDirPath := range moduleDirPaths {
if normalpath.ContainsPath(moduleDirPath, inputPath, normalpath.Relative) {
// In the case where the inputPath would appear to be relative to moduleDirPath,
// but does not exist, for example, moduleDirPath == "." and inputPath == "fake-path",
// or moduleDirPath == "real-path" and inputPath == "real-path/fake-path", the error
// returned below is not very clear (in particular the first case, "." and "fake-path").
// We do a check here to return a clearer error to the user.
//
// It should be noted that if the inputPath exists and contains many files, this may not
// be very performant, since storage.IsEmpty will do a full walk, however, it does
// not do any processing on ObjectInfo, so this should be reasonable.
empty, err := storage.IsEmpty(ctx, bucket, inputPath)
if err != nil {
return err
}
if empty {
return fmt.Errorf("input path %q does not exist", inputPath)
}
return fmt.Errorf("failed to build input %q because it is contained by module at path %q specified in your configuration, you must provide the workspace or module as the input, and filter to this path using --path", inputPath, moduleDirPath)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
version: v2
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
syntax = "proto3";

package buf;

message Buf {}
24 changes: 24 additions & 0 deletions private/buf/cmd/buf/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1134,6 +1134,30 @@ func TestWorkspaceInputOverlapFail(t *testing.T) {
testRunStdout(t, nil, 0, ``, "build", filepath.Join("testdata", "workspace", "success", "dir", "other"))
}

func TestWorkspaceInputOverlapNonExistentDirFail(t *testing.T) {
// The target input is a non-existent directory and the workspace contains a single
// module at the root, ".".
t.Parallel()
testRunStdoutStderrNoWarn(
t,
nil,
1,
``,
`Failure: input path "proto/fake-dir" does not exist`,
"build",
filepath.Join("testdata", "workspace", "fail", "overlap", "proto", "fake-dir"),
)
testRunStdoutStderrNoWarn(
t,
nil,
1,
``,
`Failure: input path "fake-dir" does not exist`,
"build",
filepath.Join("testdata", "workspace", "fail", "v2", "overlap", "fake-dir"),
)
}

func TestWorkspaceNoVersionFail(t *testing.T) {
// The buf.work.yaml must specify a version.
t.Parallel()
Expand Down

0 comments on commit e7606d7

Please sign in to comment.