Skip to content

Commit

Permalink
Rename InputDirPath to SubDirPath for clarity
Browse files Browse the repository at this point in the history
  • Loading branch information
doriable committed Mar 6, 2024
1 parent de6a9df commit 21c5088
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 47 deletions.
2 changes: 1 addition & 1 deletion private/buf/buffetch/internal/read_bucket_closer.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func newReadBucketCloser(
bucketPath string,
bucketTargeting buftarget.BucketTargeting,
) *readBucketCloser {
normalizedSubDirPath := normalpath.Normalize(bucketTargeting.InputDirPath())
normalizedSubDirPath := normalpath.Normalize(bucketTargeting.SubDirPath())
return &readBucketCloser{
ReadBucketCloser: storageReadBucketCloser,
subDirPath: normalizedSubDirPath,
Expand Down
2 changes: 1 addition & 1 deletion private/buf/buffetch/internal/read_write_bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func newReadWriteBucket(
bucketPath string,
bucketTargeting buftarget.BucketTargeting,
) *readWriteBucket {
normalizedSubDirPath := normalpath.Normalize(bucketTargeting.InputDirPath())
normalizedSubDirPath := normalpath.Normalize(bucketTargeting.SubDirPath())
return &readWriteBucket{
ReadWriteBucket: storageReadWriteBucket,
subDirPath: normalizedSubDirPath,
Expand Down
6 changes: 3 additions & 3 deletions private/buf/buffetch/internal/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ func getReadBucketCloserForBucket(
if bucketTargeting.ControllingWorkspace() != nil {
bucketPath = bucketTargeting.ControllingWorkspace().Path()
} else {
bucketPath = bucketTargeting.InputDirPath()
bucketPath = bucketTargeting.SubDirPath()
}
if bucketPath != "." {
inputBucket = storage.MapReadBucketCloser(
Expand Down Expand Up @@ -720,7 +720,7 @@ func getReadWriteBucketForOS(
return nil, nil, err
}
}
inputDir = osRootBucketTargeting.InputDirPath()
inputDir = osRootBucketTargeting.SubDirPath()
bucketTargetPaths = osRootBucketTargeting.TargetPaths()
bucketTargetExcludePaths = osRootBucketTargeting.TargetExcludePaths()
} else {
Expand Down Expand Up @@ -749,7 +749,7 @@ func getReadWriteBucketForOS(
if err != nil {
return nil, nil, err
}
inputDir, err = normalpath.Rel(bucketPathFSRelPath, osRootBucketTargeting.InputDirPath())
inputDir, err = normalpath.Rel(bucketPathFSRelPath, osRootBucketTargeting.SubDirPath())
if err != nil {
return nil, nil, err
}
Expand Down
31 changes: 17 additions & 14 deletions private/buf/buftarget/bucket_targeting.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,14 @@ type BucketTargeting interface {
// ControllingWorkpsace returns the information for the controlling workspace, if one was
// found. If not found, then this will be nil.
ControllingWorkspace() ControllingWorkspace
// InputDirPath returns the input directory relative to the root of the bucket
InputDirPath() string
// TargetPaths returns the target paths relative to the root of the bucket.
// SubDirPath returns the input directory relative to the controlling workspace, if one
// was found, otherwise it is relative to the root of the bucket
SubDirPath() string
// TargetPaths returns the target paths relative to the controlling workpsace, if one was
// found, otherwise it is relative to the root of the bucket.
TargetPaths() []string
// TargetExcludePaths returns the target exclude paths relative to the root of the bucket.
// TargetExcludePaths returns the target exclude paths relative to the controlling
// workspace, if one is found, otherwise it is relative to the root of the bucket.
TargetExcludePaths() []string

isBucketTargeting()
Expand All @@ -41,7 +44,7 @@ type BucketTargeting interface {
// NewBucketTargeting returns new targeting information for the given bucket, input dir,
// target paths, and target exclude paths.
//
// The inputDir, targetPaths, and targetExcludePaths are expected to be relative to the
// The subDirPath, targetPaths, and targetExcludePaths are expected to be relative to the
// root of the bucket.
//
// If a controlling workspace is found, the input dir, target paths, and target exclude
Expand All @@ -51,7 +54,7 @@ func NewBucketTargeting(
ctx context.Context,
logger *zap.Logger,
bucket storage.ReadBucket,
inputDir string,
subDirPath string,
targetPaths []string,
targetExcludePaths []string,
terminateFunc TerminateFunc,
Expand All @@ -60,7 +63,7 @@ func NewBucketTargeting(
ctx,
logger,
bucket,
inputDir,
subDirPath,
targetPaths,
targetExcludePaths,
terminateFunc,
Expand All @@ -73,7 +76,7 @@ var _ BucketTargeting = &bucketTargeting{}

type bucketTargeting struct {
controllingWorkspace ControllingWorkspace
inputDir string
subDirPath string
targetPaths []string
targetExcludePaths []string
}
Expand All @@ -82,17 +85,17 @@ func newBucketTargeting(
ctx context.Context,
logger *zap.Logger,
bucket storage.ReadBucket,
inputDir string,
subDirPath string,
targetPaths []string,
targetExcludePaths []string,
terminateFunc TerminateFunc,
) (*bucketTargeting, error) {
// First we map the controlling workspace for the inputDir.
// First we map the controlling workspace for the subDirpath.
controllingWorkspace, mappedInputDir, err := mapControllingWorkspaceAndPath(
ctx,
logger,
bucket,
inputDir,
subDirPath,
terminateFunc,
)
if err != nil {
Expand Down Expand Up @@ -130,7 +133,7 @@ func newBucketTargeting(
}
return &bucketTargeting{
controllingWorkspace: controllingWorkspace,
inputDir: mappedInputDir,
subDirPath: mappedInputDir,
targetPaths: mappedTargetPaths,
targetExcludePaths: mappedTargetExcludePaths,
}, nil
Expand All @@ -140,8 +143,8 @@ func (b *bucketTargeting) ControllingWorkspace() ControllingWorkspace {
return b.controllingWorkspace
}

func (b *bucketTargeting) InputDirPath() string {
return b.inputDir
func (b *bucketTargeting) SubDirPath() string {
return b.subDirPath
}

func (b *bucketTargeting) TargetPaths() []string {
Expand Down
2 changes: 1 addition & 1 deletion private/buf/bufworkspace/workspace_dep_manager_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,5 @@ func (w *workspaceDepManagerProvider) GetWorkspaceDepManager(
}
// Otherwise we simply ignore any buf.work.yaml that was found and attempt to build
// a v1 module at the SubDirPath
return newWorkspaceDepManager(bucket, bucketTargeting.InputDirPath(), false), nil
return newWorkspaceDepManager(bucket, bucketTargeting.SubDirPath(), false), nil
}
44 changes: 22 additions & 22 deletions private/buf/bufworkspace/workspace_targeting.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func newWorkspaceTargeting(
if overrideBufYAMLFile != nil {
logger.Debug(
"targeting workspace with config override",
zap.String("input dir", bucketTargeting.InputDirPath()),
zap.String("subDirPath", bucketTargeting.SubDirPath()),
)
switch fileVersion := overrideBufYAMLFile.FileVersion(); fileVersion {
case bufconfig.FileVersionV1Beta1, bufconfig.FileVersionV1:
Expand All @@ -91,7 +91,7 @@ func newWorkspaceTargeting(
config,
bucket,
bucketTargeting,
[]string{bucketTargeting.InputDirPath()},
[]string{bucketTargeting.SubDirPath()},
overrideBufYAMLFile,
)
case bufconfig.FileVersionV2:
Expand All @@ -105,7 +105,7 @@ func newWorkspaceTargeting(
if controllingWorkspace.BufYAMLFile() != nil {
logger.Debug(
"targeting workspace based on v2 buf.yaml",
zap.String("input dir", bucketTargeting.InputDirPath()),
zap.String("subDirPath", bucketTargeting.SubDirPath()),
)
return v2WorkspaceTargeting(ctx, config, bucket, bucketTargeting, controllingWorkspace.BufYAMLFile())
}
Expand All @@ -115,7 +115,7 @@ func newWorkspaceTargeting(
// This means that we attempted to target a v1 workspace at the buf.work.yaml, not
// an individual module within the v1 workspace defined in buf.work.yaml.
// This is disallowed.
if bucketTargeting.InputDirPath() == "." {
if bucketTargeting.SubDirPath() == "." {
return nil, errors.New(`Workspaces defined with buf.work.yaml cannot be updated or pushed, only
the individual modules within a workspace can be updated or pushed. Workspaces
defined with a v2 buf.yaml can be updated, see the migration documentation for more details.`)
Expand All @@ -124,14 +124,14 @@ defined with a v2 buf.yaml can be updated, see the migration documentation for m
// the workspace entirely, and just act as if the buf.work.yaml did not exist.
logger.Debug(
"targeting workspace, ignoring v1 buf.work.yaml, just building on module at target",
zap.String("input dir", bucketTargeting.InputDirPath()),
zap.String("subDirPath", bucketTargeting.SubDirPath()),
)
return v1WorkspaceTargeting(
ctx,
config,
bucket,
bucketTargeting,
[]string{bucketTargeting.InputDirPath()}, // Assume we are targeting only the module at the input dir
[]string{bucketTargeting.SubDirPath()}, // Assume we are targeting only the module at the input dir
nil,
)
}
Expand All @@ -147,7 +147,7 @@ defined with a v2 buf.yaml can be updated, see the migration documentation for m
}
logger.Debug(
"targeting workspace with no found buf.work.yaml or buf.yaml",
zap.String("input dir", bucketTargeting.InputDirPath()),
zap.String("subDirPath", bucketTargeting.SubDirPath()),
)
// We did not find any buf.work.yaml or buf.yaml, we invoke fallback logic.
return fallbackWorkspaceTargeting(
Expand All @@ -171,7 +171,7 @@ func v2WorkspaceTargeting(
// --exclude-path flags resulted in no targeted modules. This condition is represented
// by hadIsTentativelyTargetModule == true && hadIsTargetModule = false
//
// If hadIsTentativelyTargetModule is false, this means that our input bucketTargeting.InputDirPath() was not
// If hadIsTentativelyTargetModule is false, this means that our input bucketTargeting.SubDirPath() was not
// actually representative of any module that we detected in buf.work.yaml or v2 buf.yaml
// directories, and this is a system error - this should be verified before we reach this function.
var hadIsTentativelyTargetModule bool
Expand All @@ -183,14 +183,14 @@ func v2WorkspaceTargeting(
moduleDirPath := moduleConfig.DirPath()
moduleDirPaths = append(moduleDirPaths, moduleDirPath)
bucketIDToModuleConfig[moduleDirPath] = moduleConfig
// bucketTargeting.InputDirPath() is the input targetSubDirPath. We only want to target modules that are inside
// bucketTargeting.SubDirPath() is the input targetSubDirPath. We only want to target modules that are inside
// this targetSubDirPath. Example: bufWorkYAMLDirPath is "foo", targetSubDirPath is "foo/bar",
// listed directories are "bar/baz", "bar/bat", "other". We want to include "foo/bar/baz"
// and "foo/bar/bat".
//
// This is new behavior - before, we required that you input an exact match for the module directory path,
// but now, we will take all the modules underneath this workspace.
isTentativelyTargetModule := normalpath.EqualsOrContainsPath(bucketTargeting.InputDirPath(), moduleDirPath, normalpath.Relative)
isTentativelyTargetModule := normalpath.EqualsOrContainsPath(bucketTargeting.SubDirPath(), moduleDirPath, normalpath.Relative)
// We ignore this check for proto file refs, since the input is considered the directory
// of the proto file reference, which is unlikely to contain a module in its entirety.
// In the future, it would be nice to handle this more elegently.
Expand Down 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.InputDirPath(), moduleDirPaths)
return nil, checkForOverlap(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 @@ -252,7 +252,7 @@ func v1WorkspaceTargeting(
// --exclude-path flags resulted in no targeted modules. This condition is represented
// by hadIsTentativelyTargetModule == true && hadIsTargetModule = false
//
// If hadIsTentativelyTargetModule is false, this means that our input bucketTargeting.InputDirPath() was not
// If hadIsTentativelyTargetModule is false, this means that our input bucketTargeting.SubDirPath() was not
// actually representative of any module that we detected in buf.work.yaml or v2 buf.yaml
// directories, and this is a system error - this should be verified before we reach this function.
var hadIsTentativelyTargetModule bool
Expand Down Expand Up @@ -286,14 +286,14 @@ func v1WorkspaceTargeting(
}
}
bucketIDToModuleConfig[moduleDirPath] = moduleConfig
// We only want to target modules that are inside the bucketTargeting.InputDirPath().
// Example: bufWorkYAMLDirPath is "foo", bucketTargeting.InputDirPath() is "foo/bar",
// We only want to target modules that are inside the bucketTargeting.SubDirPath().
// Example: bufWorkYAMLDirPath is "foo", bucketTargeting.SubDirPath() is "foo/bar",
// listed directories are "bar/baz", "bar/bat", "other". We want to include "foo/bar/baz"
// and "foo/bar/bat".
//
// This is new behavior - before, we required that you input an exact match for the module directory path,
// but now, we will take all the modules underneath this workspace.
isTentativelyTargetModule := normalpath.EqualsOrContainsPath(bucketTargeting.InputDirPath(), moduleDirPath, normalpath.Relative)
isTentativelyTargetModule := normalpath.EqualsOrContainsPath(bucketTargeting.SubDirPath(), moduleDirPath, normalpath.Relative)
// We ignore this check for proto file refs, since the input is considered the directory
// of the proto file reference, which is unlikely to contain a module in its entirety.
// In the future, it would be nice to handle this more elegently.
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.InputDirPath(), moduleDirPaths)
return nil, checkForOverlap(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 @@ -366,7 +366,7 @@ func fallbackWorkspaceTargeting(
ctx,
logger,
bucket,
bucketTargeting.InputDirPath(),
bucketTargeting.SubDirPath(),
true,
)
if err != nil {
Expand Down Expand Up @@ -443,7 +443,7 @@ func fallbackWorkspaceTargeting(
// If we still have no v1 module paths, then we go to the final fallback and set a v1
// module at the input dir.
if len(v1ModulePaths) == 0 {
v1ModulePaths = append(v1ModulePaths, bucketTargeting.InputDirPath())
v1ModulePaths = append(v1ModulePaths, bucketTargeting.SubDirPath())
}
return v1WorkspaceTargeting(
ctx,
Expand Down Expand Up @@ -474,8 +474,8 @@ func validateBucketTargeting(
// We don't use --path, --exclude-path here because these paths have had ExternalPathToPath
// applied to them. Which is another argument to do this at a higher level.
for _, targetPath := range bucketTargeting.TargetPaths() {
if targetPath == bucketTargeting.InputDirPath() {
// The targetPath/InputDirPath may not equal something on the command line as we have done
if targetPath == bucketTargeting.SubDirPath() {
// The targetPath/SubDirPath may not equal something on the command line as we have done
// targeting via workspaces by now, so do not print them.
return errors.New("given input is equal to a value of --path, this has no effect and is disallowed")
}
Expand All @@ -500,8 +500,8 @@ func validateBucketTargeting(
}
}
for _, targetExcludePath := range bucketTargeting.TargetExcludePaths() {
if targetExcludePath == bucketTargeting.InputDirPath() {
unnormalizedTargetSubDirPath := filepath.Clean(normalpath.Unnormalize(bucketTargeting.InputDirPath()))
if targetExcludePath == bucketTargeting.SubDirPath() {
unnormalizedTargetSubDirPath := filepath.Clean(normalpath.Unnormalize(bucketTargeting.SubDirPath()))
unnormalizedTargetExcludePath := filepath.Clean(normalpath.Unnormalize(targetExcludePath))
return fmt.Errorf(`given input "%s" is equal to a value of --exclude-path "%s", this would exclude everything`, unnormalizedTargetSubDirPath, unnormalizedTargetExcludePath)
}
Expand Down
6 changes: 3 additions & 3 deletions private/buf/bufworkspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func testBasic(t *testing.T, subDirPath string) {
)
require.NotNil(t, bucketTargeting.ControllingWorkspace())
require.Equal(t, ".", bucketTargeting.ControllingWorkspace().Path())
require.Equal(t, "finance/portfolio/proto", bucketTargeting.InputDirPath())
require.Equal(t, "finance/portfolio/proto", bucketTargeting.SubDirPath())
require.NoError(t, err)

workspace, err := workspaceProvider.GetWorkspaceForBucket(
Expand Down Expand Up @@ -153,7 +153,7 @@ func testBasic(t *testing.T, subDirPath string) {
require.NoError(t, err)
require.NotNil(t, bucketTargeting.ControllingWorkspace())
require.Equal(t, ".", bucketTargeting.ControllingWorkspace().Path())
require.Equal(t, "common/money/proto", bucketTargeting.InputDirPath())
require.Equal(t, "common/money/proto", bucketTargeting.SubDirPath())
require.Equal(
t,
[]string{"common/money/proto/acme/money/v1/currency_code.proto"},
Expand Down Expand Up @@ -209,7 +209,7 @@ func TestUnusedDep(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, bucketTargeting.ControllingWorkspace())
require.Equal(t, ".", bucketTargeting.ControllingWorkspace().Path())
require.Equal(t, ".", bucketTargeting.InputDirPath())
require.Equal(t, ".", bucketTargeting.SubDirPath())

workspace, err := workspaceProvider.GetWorkspaceForBucket(
ctx,
Expand Down
2 changes: 0 additions & 2 deletions private/buf/cmd/buf/buf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1183,8 +1183,6 @@ func TestModInitBasic(t *testing.T) {
testModInit(
t,
`version: v2
modules:
- path: .
lint:
use:
- DEFAULT
Expand Down

0 comments on commit 21c5088

Please sign in to comment.