From 9a7026ad44cd0945059d38702dbbde25702ba4c7 Mon Sep 17 00:00:00 2001 From: bufdev Date: Wed, 14 Feb 2024 18:33:31 -0500 Subject: [PATCH 1/2] change --- private/buf/bufworkspace/workspace_provider.go | 8 ++++++-- private/bufpkg/bufconfig/object_data.go | 9 ++++++--- .../bufmodule/bufmoduletesting/bufmoduletesting.go | 8 ++++++-- .../bufmoduletesting/cmd/buf-digest/digest.go | 10 ++++++++-- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/private/buf/bufworkspace/workspace_provider.go b/private/buf/bufworkspace/workspace_provider.go index 9d2b12e93c..0d0a319a69 100644 --- a/private/buf/bufworkspace/workspace_provider.go +++ b/private/buf/bufworkspace/workspace_provider.go @@ -375,11 +375,15 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1( } v1BufYAMLObjectData, err := bufconfig.GetBufYAMLV1Beta1OrV1ObjectDataForPrefix(ctx, bucket, moduleDirPath) if err != nil { - return nil, err + if !errors.Is(err, fs.ErrNotExist) { + return nil, err + } } v1BufLockObjectData, err := bufconfig.GetBufLockV1Beta1OrV1ObjectDataForPrefix(ctx, bucket, moduleDirPath) if err != nil { - return nil, err + if !errors.Is(err, fs.ErrNotExist) { + return nil, err + } } moduleSetBuilder.AddLocalModule( mappedModuleBucket, diff --git a/private/bufpkg/bufconfig/object_data.go b/private/bufpkg/bufconfig/object_data.go index 1d480201db..fbd96f4da7 100644 --- a/private/bufpkg/bufconfig/object_data.go +++ b/private/bufpkg/bufconfig/object_data.go @@ -44,7 +44,7 @@ type ObjectData interface { // the given bucket prefix, if the buf.yaml file was v1beta1 or v1. // // The file is only parsed for its file version. No additional validation is performed. -// If the file does not exist, nil is returned. This is as opposed to other functions that return fs.ErrNotExist. +// If the file does not exist, an error that satisfies fs.ErrNotExist is returned. // // This function is used to help optionally get ObjectData where it is needed for digest calculations. func GetBufYAMLV1Beta1OrV1ObjectDataForPrefix( @@ -59,7 +59,7 @@ func GetBufYAMLV1Beta1OrV1ObjectDataForPrefix( // the given bucket prefix, if the buf.lock file was v1beta1 or v1. // // The file is only parsed for its file version. No additional validation is performed. -// If the file does not exist, nil is returned. This is as opposed to other functions that return fs.ErrNotExist. +// If the file does not exist, an error that satisfies fs.ErrNotExist is returned. // // This function is used to help optionally get ObjectData where it is needed for digest calculations. func GetBufLockV1Beta1OrV1ObjectDataForPrefix( @@ -101,6 +101,9 @@ func getV1Beta1OrV1ObjectDataForPrefix( fileNames []string, fileNameToSupportedFileVersions map[string]map[FileVersion]struct{}, ) (ObjectData, error) { + if len(fileNames) == 0 { + return nil, syserror.New("expected at least one file name for getV1Beta1OrV1ObjectDataForPrefix") + } for _, fileName := range fileNames { path := normalpath.Join(prefix, fileName) data, err := storage.ReadPath(ctx, bucket, path) @@ -137,5 +140,5 @@ func getV1Beta1OrV1ObjectDataForPrefix( return nil, syserror.Newf("unknown FileVersion: %v", fileVersion) } } - return nil, nil + return nil, &fs.PathError{Op: "read", Path: normalpath.Join(prefix, fileNames[0]), Err: fs.ErrNotExist} } diff --git a/private/bufpkg/bufmodule/bufmoduletesting/bufmoduletesting.go b/private/bufpkg/bufmodule/bufmoduletesting/bufmoduletesting.go index 137ecaa200..db1792e783 100644 --- a/private/bufpkg/bufmodule/bufmoduletesting/bufmoduletesting.go +++ b/private/bufpkg/bufmodule/bufmoduletesting/bufmoduletesting.go @@ -444,11 +444,15 @@ func addModuleDataToModuleSetBuilder( ctx := context.Background() bufYAMLObjectData, err := bufconfig.GetBufYAMLV1Beta1OrV1ObjectDataForPrefix(ctx, bucket, ".") if err != nil { - return err + if !errors.Is(err, fs.ErrNotExist) { + return err + } } bufLockObjectData, err := bufconfig.GetBufLockV1Beta1OrV1ObjectDataForPrefix(ctx, bucket, ".") if err != nil { - return err + if !errors.Is(err, fs.ErrNotExist) { + return err + } } localModuleOptions = append( localModuleOptions, diff --git a/private/bufpkg/bufmodule/bufmoduletesting/cmd/buf-digest/digest.go b/private/bufpkg/bufmodule/bufmoduletesting/cmd/buf-digest/digest.go index 7e5a577b42..c940d78141 100644 --- a/private/bufpkg/bufmodule/bufmoduletesting/cmd/buf-digest/digest.go +++ b/private/bufpkg/bufmodule/bufmoduletesting/cmd/buf-digest/digest.go @@ -16,7 +16,9 @@ package main import ( "context" + "errors" "fmt" + "io/fs" "time" "github.com/bufbuild/buf/private/bufpkg/bufconfig" @@ -109,11 +111,15 @@ func run( } v1BufYAMLObjectData, err := bufconfig.GetBufYAMLV1Beta1OrV1ObjectDataForPrefix(ctx, bucket, ".") if err != nil { - return err + if !errors.Is(err, fs.ErrNotExist) { + return err + } } v1BufLockObjectData, err := bufconfig.GetBufLockV1Beta1OrV1ObjectDataForPrefix(ctx, bucket, ".") if err != nil { - return err + if !errors.Is(err, fs.ErrNotExist) { + return err + } } moduleSetBuilder.AddLocalModule( bucket, From 339e0230fdd7e6df3fbf1c70fb8d1d9ff75b1a92 Mon Sep 17 00:00:00 2001 From: bufdev Date: Wed, 14 Feb 2024 18:40:41 -0500 Subject: [PATCH 2/2] cleanup comments --- private/buf/bufworkspace/workspace_provider.go | 10 +++------- private/bufpkg/bufmodule/module_data.go | 4 ++-- private/bufpkg/bufmodule/module_set_builder.go | 10 ++++++---- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/private/buf/bufworkspace/workspace_provider.go b/private/buf/bufworkspace/workspace_provider.go index 0d0a319a69..a63ba91f73 100644 --- a/private/buf/bufworkspace/workspace_provider.go +++ b/private/buf/bufworkspace/workspace_provider.go @@ -436,19 +436,15 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2( var err error if overrideBufYAMLFile != nil { bufYAMLFile = overrideBufYAMLFile - // We don't want to have ObjectData for a --config override. - // TODO: What happened when you specified a --config pre-refactor with tamper-proofing? We might - // have actually still used the buf.yaml for tamper-proofing, if so, we need to attempt to read it - // regardless of whether override was specified. } else { bufYAMLFile, err = bufconfig.GetBufYAMLFileForPrefix(ctx, bucket, ".") if err != nil { // This should be apparent from above functions. return nil, syserror.Newf("error getting v2 buf.yaml: %w", err) } - if bufYAMLFile.FileVersion() != bufconfig.FileVersionV2 { - return nil, syserror.Newf("expected v2 buf.yaml but got %v", bufYAMLFile.FileVersion()) - } + } + if bufYAMLFile.FileVersion() != bufconfig.FileVersionV2 { + return nil, syserror.Newf("expected v2 buf.yaml but got %v", bufYAMLFile.FileVersion()) } // config.targetSubDirPath is the input targetSubDirPath. We only want to target modules that are inside diff --git a/private/bufpkg/bufmodule/module_data.go b/private/bufpkg/bufmodule/module_data.go index fcd9f9c80c..509df6688e 100644 --- a/private/bufpkg/bufmodule/module_data.go +++ b/private/bufpkg/bufmodule/module_data.go @@ -47,14 +47,14 @@ type ModuleData interface { // BufYAMLObjectData gets the v1beta1 or v1 buf.yaml ObjectData. // // This is always present, even if the Module was created from a v2 buf.yaml file. The BSR will - // synthesize a value. + // synthesize a value for v2 buf.yamls. // // This is used for digest calcuations. It is not used otherwise. V1Beta1OrV1BufYAMLObjectData() (ObjectData, error) // BufYAMLObjectData gets the v1beta1 or v1 buf.lock ObjectData. // // This is always present, even if the Module was created from a v2 buf.yaml file. The BSR will - // synthesize a value. + // synthesize a value for v2 buf.yamls. // // This is used for digest calcuations. It is not used otherwise. V1Beta1OrV1BufLockObjectData() (ObjectData, error) diff --git a/private/bufpkg/bufmodule/module_set_builder.go b/private/bufpkg/bufmodule/module_set_builder.go index 2388c1a06a..5db2785c88 100644 --- a/private/bufpkg/bufmodule/module_set_builder.go +++ b/private/bufpkg/bufmodule/module_set_builder.go @@ -210,7 +210,9 @@ func LocalModuleWithProtoFileTargetPath( // LocalModuleWithV1Beta1OrV1BufYAMLObjectData returns a new LocalModuleOption that attaches the original // source buf.yaml file associated with this module for v1 or v1beta1 buf.yaml-backed Modules. // -// This is required for Modules backed with a v1beta1 or v1 buf.yaml. +// If a buf.yaml exists on disk, should be set for Modules backed with a v1beta1 or v1 buf.yaml. It is possible +// that a Module has no buf.yaml (if it was built from defaults), in which case this will not be set. +// // For Modules backed with v2 buf.yamls, this should not be set. // // This file content is just used for dependency calculations. It is not parsed. @@ -223,13 +225,13 @@ func LocalModuleWithV1Beta1OrV1BufYAMLObjectData(v1BufYAMLObjectData ObjectData) // LocalModuleWithV1Beta1OrV1BufLockObjectData returns a new LocalModuleOption that attaches the original // source buf.local file associated with this Module for v1 or v1beta1 buf.lock-backed Modules. // -// This is required for Modules backed with a v1beta1 or v1 buf.lock. -// For Modules backed with v2 buf.locks, this should not be set. -// +// If a buf.lock exists on disk, should be set for Modules backed with a v1beta1 or v1 buf.lock. // Note that a buf.lock may not exist for a v1 Module, if there are no dependencies, and in this // case, this is not set. However, if there is a buf.lock file that was generated, even if it // had no dependencies, this is set. // +// For Modules backed with v2 buf.locks, this should not be set. +// // This file content is just used for dependency calculations. It is not parsed. func LocalModuleWithV1Beta1OrV1BufLockObjectData(v1BufLockObjectData ObjectData) LocalModuleOption { return func(localModuleOptions *localModuleOptions) {