From d55f33d896f0340525396c277138b148741c7db3 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst Date: Thu, 4 Mar 2021 15:26:40 +0000 Subject: [PATCH] Update to v0.39.0 --- go.mod | 10 +- go.sum | 20 +- internal/buf/bufapimodule/module_reader.go | 11 +- internal/buf/bufcli/bufcli.go | 2 +- internal/buf/bufcli/errors.go | 23 +-- internal/buf/bufcore/bufcore.go | 12 -- .../bufcoreprotoparse/bufcoreprotoparse.go | 128 ------------- .../bufcoreprotoparse/path_resolver.go | 113 ----------- .../bufcore/bufcoretesting/bufcoretesting.go | 29 --- internal/buf/bufcore/bufimage/bufimage.go | 29 ++- .../bufcore/bufimage/bufimagebuild/builder.go | 15 +- .../bufimagetesting/bufimagetesting.go | 4 + .../bufimagetesting/bufimagetesting_test.go | 122 ++++++------ internal/buf/bufcore/bufimage/image_file.go | 10 +- internal/buf/bufcore/bufimage/util.go | 25 ++- internal/buf/bufcore/bufmodule/bufmodule.go | 111 ++++++++++- .../module_bucket_builder_test.go | 68 +++---- .../module_include_builder_test.go | 50 +++-- .../bufmodule/bufmodulecache/module_cacher.go | 11 +- internal/buf/bufcore/bufmodule/module.go | 42 +++- internal/buf/bufcore/bufmodule/module_file.go | 6 +- .../buf/bufcore/bufmodule/module_file_set.go | 48 +++-- .../buf/bufcore/bufmodule/targeting_module.go | 10 +- .../bufmodule/targeting_module_test.go | 80 ++++---- internal/buf/bufcore/bufmodule/validate.go | 4 +- internal/buf/bufwire/bufwire.go | 3 +- internal/buf/bufwire/file_lister.go | 6 +- .../buf/cmd/buf/command/beta/push/push.go | 13 ++ .../buf/cmd/protoc-gen-buf-lint/lint_test.go | 1 + .../registryv1alpha1api/organization.pb.go | 1 - .../registryv1alpha1api/repository.pb.go | 1 - .../repository_branch.pb.go | 1 - .../repository_commit.pb.go | 1 - .../registryv1alpha1apiclient.pb.go | 6 + .../registryv1alpha1apiclientgrpc.pb.go | 23 +++ .../registryv1alpha1apiclienttwirp.pb.go | 23 +++ .../proto/go/buf/alpha/image/v1/image.pb.go | 179 +++++++++++++++--- .../alpha/registry/v1alpha1/download.twirp.go | 1 + .../registry/v1alpha1/organization.twirp.go | 1 - .../registry/v1alpha1/repository.twirp.go | 1 - .../v1alpha1/repository_branch.twirp.go | 1 - .../v1alpha1/repository_commit.twirp.go | 1 - .../alpha/registry/v1alpha1/resolve.twirp.go | 4 +- .../buf/alpha/registry/v1alpha1/user.twirp.go | 4 +- .../appproto/appprotoexec/binary_handler.go | 6 +- internal/pkg/protogenutil/protogenutil.go | 5 +- make/buf/all.mk | 3 +- proto/buf/alpha/image/v1/image.proto | 24 +++ .../registry/v1alpha1/organization.proto | 1 - .../alpha/registry/v1alpha1/repository.proto | 1 - .../registry/v1alpha1/repository_branch.proto | 1 - .../registry/v1alpha1/repository_commit.proto | 1 - 52 files changed, 727 insertions(+), 569 deletions(-) delete mode 100644 internal/buf/bufcore/bufcoreprotoparse/bufcoreprotoparse.go delete mode 100644 internal/buf/bufcore/bufcoreprotoparse/path_resolver.go diff --git a/go.mod b/go.mod index ca882b2000..7895e354fe 100644 --- a/go.mod +++ b/go.mod @@ -8,8 +8,8 @@ require ( github.com/gofrs/uuid v4.0.0+incompatible github.com/golang/protobuf v1.4.3 github.com/google/go-cmp v0.5.4 - github.com/jhump/protoreflect v1.8.2 - github.com/klauspost/compress v1.11.8 + github.com/jhump/protoreflect v1.8.3-0.20210302193947-8255811fc3c0 + github.com/klauspost/compress v1.11.9 github.com/klauspost/pgzip v1.2.5 github.com/pkg/errors v0.9.1 // indirect github.com/pkg/profile v1.5.0 @@ -20,11 +20,11 @@ require ( go.opencensus.io v0.23.0 go.uber.org/multierr v1.6.0 go.uber.org/zap v1.16.0 - golang.org/x/net v0.0.0-20210224082022-3d97a244fca7 // indirect - golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073 // indirect + golang.org/x/net v0.0.0-20210226172049-e18ecbb05110 // indirect + golang.org/x/sys v0.0.0-20210304124612-50617c2ba197 // indirect golang.org/x/text v0.3.5 // indirect golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect - google.golang.org/genproto v0.0.0-20210224155714-063164c882e6 // indirect + google.golang.org/genproto v0.0.0-20210303154014-9728d6b83eeb // indirect google.golang.org/grpc v1.35.0-dev.0.20201218190559-666aea1fb34c google.golang.org/protobuf v1.25.1-0.20201208041424-160c7477e0e8 gopkg.in/yaml.v2 v2.4.0 // indirect diff --git a/go.sum b/go.sum index 21aee539a5..e9c269631d 100644 --- a/go.sum +++ b/go.sum @@ -125,8 +125,8 @@ github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2p github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= -github.com/jhump/protoreflect v1.8.2 h1:k2xE7wcUomeqwY0LDCYA16y4WWfyTcMx5mKhk0d4ua0= -github.com/jhump/protoreflect v1.8.2/go.mod h1:7GcYQDdMU/O/BBrl/cX6PNHpXh6cenjd8pneu5yW7Tg= +github.com/jhump/protoreflect v1.8.3-0.20210302193947-8255811fc3c0 h1:dBmhjIVYJbfDDPGBvq/WLfi0mAMg5e2GGYzv9nW/Jxg= +github.com/jhump/protoreflect v1.8.3-0.20210302193947-8255811fc3c0/go.mod h1:7GcYQDdMU/O/BBrl/cX6PNHpXh6cenjd8pneu5yW7Tg= github.com/jonboulle/clockwork v0.1.0/go.mod h1:Ii8DK3G1RaLaWxj9trq07+26W01tbo22gdxWY5EU2bo= github.com/json-iterator/go v1.1.6/go.mod h1:+SdeFBvtyEkXs7REEP0seUULqWtbJapLOCVDaaPEHmU= github.com/jstemmer/go-junit-report v0.0.0-20190106144839-af01ea7f8024/go.mod h1:6v2b51hI/fHJwM22ozAgKL4VKDeJcHhJFhtBdhmNjmU= @@ -134,8 +134,8 @@ github.com/jtolds/gls v4.20.0+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfV github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w= github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= -github.com/klauspost/compress v1.11.8 h1:difgzQsp5mdAz9v8lm3P/I+EpDKMU/6uTMw1y1FObuo= -github.com/klauspost/compress v1.11.8/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= +github.com/klauspost/compress v1.11.9 h1:5OCMOdde1TCT2sookEuVeEZzA8bmRSFV3AwPDZAG8AA= +github.com/klauspost/compress v1.11.9/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= github.com/klauspost/pgzip v1.2.5 h1:qnWYvvKqedOF2ulHpMG72XQol4ILEJ8k2wwRl/Km8oE= github.com/klauspost/pgzip v1.2.5/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= @@ -283,8 +283,8 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA= golang.org/x/net v0.0.0-20201110031124-69a78807bb2b/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.0.0-20210224082022-3d97a244fca7 h1:OgUuv8lsRpBibGNbSizVwKWlysjaNzmC9gYMhPVfqFM= -golang.org/x/net v0.0.0-20210224082022-3d97a244fca7/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20210226172049-e18ecbb05110 h1:qWPm9rbaAMKs8Bq/9LRpbMqxWRVUAQwMI9fVrssnTfw= +golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -311,8 +311,8 @@ golang.org/x/sys v0.0.0-20190624142023-c5567b49c5d0/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073 h1:8qxJSnu+7dRq6upnbntrmriWByIakBuct5OM/MdQC1M= -golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210304124612-50617c2ba197 h1:7+SpRyhoo46QjKkYInQXpcfxx3TYFEYkn131lwGE9/0= +golang.org/x/sys v0.0.0-20210304124612-50617c2ba197/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -371,8 +371,8 @@ google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98 google.golang.org/genproto v0.0.0-20190911173649-1774047e7e51/go.mod h1:IbNlFCBrqXvoKpeg0TB2l7cyZUmoaFKYIwrEpbDKLA8= google.golang.org/genproto v0.0.0-20191108220845-16a3f7862a1a/go.mod h1:n3cpQtvxv34hfy77yVDNjmbRyujviMdxYliBSkLhpCc= google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo= -google.golang.org/genproto v0.0.0-20210224155714-063164c882e6 h1:bXUwz2WkXXrXgiLxww3vWmoSHLOGv4ipdPdTvKymcKw= -google.golang.org/genproto v0.0.0-20210224155714-063164c882e6/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= +google.golang.org/genproto v0.0.0-20210303154014-9728d6b83eeb h1:hcskBH5qZCOa7WpTUFUFvoebnSFZBYpjykLtjIp9DVk= +google.golang.org/genproto v0.0.0-20210303154014-9728d6b83eeb/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no= google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c= google.golang.org/grpc v1.20.1/go.mod h1:10oTOabMzJvdu6/UiuZezV6QK5dSlG84ov/aaiqXj38= google.golang.org/grpc v1.21.1/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM= diff --git a/internal/buf/bufapimodule/module_reader.go b/internal/buf/bufapimodule/module_reader.go index aee874d38b..b0227a02d6 100644 --- a/internal/buf/bufapimodule/module_reader.go +++ b/internal/buf/bufapimodule/module_reader.go @@ -53,5 +53,14 @@ func (m *moduleReader) GetModule(ctx context.Context, modulePin bufmodule.Module } return nil, err } - return bufmodule.NewModuleForProto(ctx, module) + moduleReference, err := bufmodule.NewModuleReference( + modulePin.Remote(), + modulePin.Owner(), + modulePin.Repository(), + modulePin.Commit(), + ) + if err != nil { + return nil, err + } + return bufmodule.NewModuleForProto(ctx, module, bufmodule.ModuleWithModuleReference(moduleReference)) } diff --git a/internal/buf/bufcli/bufcli.go b/internal/buf/bufcli/bufcli.go index c2ca30d0c5..387a149757 100644 --- a/internal/buf/bufcli/bufcli.go +++ b/internal/buf/bufcli/bufcli.go @@ -52,7 +52,7 @@ import ( const ( // Version is the version of buf. - Version = "0.39.0-dev" + Version = "0.39.0" // FlagDeprecationMessageSuffix is the suffix for flag deprecation messages. FlagDeprecationMessageSuffix = ` diff --git a/internal/buf/bufcli/errors.go b/internal/buf/bufcli/errors.go index e6323f8a96..b001804307 100644 --- a/internal/buf/bufcli/errors.go +++ b/internal/buf/bufcli/errors.go @@ -133,31 +133,16 @@ func NewTokenNotFoundError(tokenID string) error { // Note that this function will wrap the error so that the underlying error // can be recovered via 'errors.Is'. func wrapError(action string, err error) error { - if err == nil || err.Error() == "" { - // If the error is nil or empty, we return - // it as-is. This is especially relevant - // for commands like lint and breaking. + if err == nil || (err.Error() == "" && rpc.GetErrorCode(err) == rpc.ErrorCodeUnknown) { + // If the error is nil or empty and not an rpc error, we return it as-is. + // This is especially relevant for commands like lint and breaking. return err } switch { - case rpc.GetErrorCode(err) == rpc.ErrorCodeUnauthenticated, isEmptyUnknownError(err): + case rpc.GetErrorCode(err) == rpc.ErrorCodeUnauthenticated: return fmt.Errorf(`Failed to %s; you are not authenticated. Create a new entry in your netrc, using a Buf API Key as the password. For details, visit https://beta.docs.buf.build/authentication`, action) case rpc.GetErrorCode(err) == rpc.ErrorCodeUnavailable: return fmt.Errorf(`Failed to %s: the server hosted at that remote is unavailable: %w.`, action, err) } return fmt.Errorf("Failed to %q: %w.", action, err) } - -// isEmptyUnknownError returns true if the given -// error is non-nil, but has an empty message -// and an unknown error code. -// -// This is relevant for errors returned by -// envoyauthd when the client does not provide -// an authentication header. -func isEmptyUnknownError(err error) bool { - if err == nil { - return false - } - return err.Error() == "" && rpc.GetErrorCode(err) == rpc.ErrorCodeUnknown -} diff --git a/internal/buf/bufcore/bufcore.go b/internal/buf/bufcore/bufcore.go index 161a89a844..4d1c7ca35c 100644 --- a/internal/buf/bufcore/bufcore.go +++ b/internal/buf/bufcore/bufcore.go @@ -16,8 +16,6 @@ package bufcore import ( - "sort" - "github.com/bufbuild/buf/internal/pkg/storage" ) @@ -61,13 +59,3 @@ func NewFileInfo(path string, externalPath string, isImport bool) (FileInfo, err func NewFileInfoForObjectInfo(objectInfo storage.ObjectInfo, isImport bool) FileInfo { return newFileInfoForObjectInfo(objectInfo, isImport) } - -// SortFileInfos sorts the FileInfos. -func SortFileInfos(fileInfos []FileInfo) { - sort.Slice( - fileInfos, - func(i int, j int) bool { - return fileInfos[i].Path() < fileInfos[j].Path() - }, - ) -} diff --git a/internal/buf/bufcore/bufcoreprotoparse/bufcoreprotoparse.go b/internal/buf/bufcore/bufcoreprotoparse/bufcoreprotoparse.go deleted file mode 100644 index 7703bc5914..0000000000 --- a/internal/buf/bufcore/bufcoreprotoparse/bufcoreprotoparse.go +++ /dev/null @@ -1,128 +0,0 @@ -// Copyright 2020-2021 Buf Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package bufcoreprotoparse - -import ( - "context" - "io" - - "github.com/bufbuild/buf/internal/buf/bufanalysis" - "github.com/bufbuild/buf/internal/buf/bufcore" - "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" - "github.com/bufbuild/buf/internal/pkg/normalpath" - "github.com/jhump/protoreflect/desc/protoparse" -) - -// ParserAccessorHandler handles ParserAccessor operations for protoparse.. -type ParserAccessorHandler interface { - // Open opens the given path, and tracks the external path and import status. - // - // This function can be used as a ParserAccessor for protoparse. - Open(path string) (io.ReadCloser, error) - // ExternalPath returns the external path for the input path. - // - // Returns the input path if the external path is not known. - ExternalPath(path string) string - // IsImport returns true if the path is an import. - IsImport(path string) bool -} - -// NewParserAccessorHandler returns a new ParserAccessorHandler. -// -// The given module should be a bufmodule.ModuleFileSet for image builds, as it needs -// access to not just the target files, but all dependency files as well. -// -// For AST building, this can just be a bufmodule.Module. -func NewParserAccessorHandler(ctx context.Context, module bufmodule.Module) ParserAccessorHandler { - return newParserAccessorHandler(ctx, module) -} - -// GetFileAnnotations gets the FileAnnotations for the ErrorWithPos errors. -func GetFileAnnotations( - ctx context.Context, - parserAccessorHandler ParserAccessorHandler, - errorsWithPos []protoparse.ErrorWithPos, -) ([]bufanalysis.FileAnnotation, error) { - fileAnnotations := make([]bufanalysis.FileAnnotation, 0, len(errorsWithPos)) - for _, errorWithPos := range errorsWithPos { - fileAnnotation, err := GetFileAnnotation( - ctx, - parserAccessorHandler, - errorWithPos, - ) - if err != nil { - return nil, err - } - fileAnnotations = append(fileAnnotations, fileAnnotation) - } - return fileAnnotations, nil -} - -// GetFileAnnotation gets the FileAnnotation for the ErrorWithPos error. -func GetFileAnnotation( - ctx context.Context, - parserAccessorHandler ParserAccessorHandler, - errorWithPos protoparse.ErrorWithPos, -) (bufanalysis.FileAnnotation, error) { - var fileInfo bufcore.FileInfo - var startLine int - var startColumn int - var endLine int - var endColumn int - typeString := "COMPILE" - message := "Compile error." - // this should never happen - // maybe we should error - if errorWithPos.Unwrap() != nil { - message = errorWithPos.Unwrap().Error() - } - sourcePos := protoparse.SourcePos{} - if errorWithSourcePos, ok := errorWithPos.(protoparse.ErrorWithSourcePos); ok { - if pos := errorWithSourcePos.Pos; pos != nil { - sourcePos = *pos - } - } - if sourcePos.Filename != "" { - path, err := normalpath.NormalizeAndValidate(sourcePos.Filename) - if err != nil { - return nil, err - } - fileInfo, err = bufcore.NewFileInfo( - path, - parserAccessorHandler.ExternalPath(path), - parserAccessorHandler.IsImport(path), - ) - if err != nil { - return nil, err - } - } - if sourcePos.Line > 0 { - startLine = sourcePos.Line - endLine = sourcePos.Line - } - if sourcePos.Col > 0 { - startColumn = sourcePos.Col - endColumn = sourcePos.Col - } - return bufanalysis.NewFileAnnotation( - fileInfo, - startLine, - startColumn, - endLine, - endColumn, - typeString, - message, - ), nil -} diff --git a/internal/buf/bufcore/bufcoreprotoparse/path_resolver.go b/internal/buf/bufcore/bufcoreprotoparse/path_resolver.go deleted file mode 100644 index ec020d5143..0000000000 --- a/internal/buf/bufcore/bufcoreprotoparse/path_resolver.go +++ /dev/null @@ -1,113 +0,0 @@ -// Copyright 2020-2021 Buf Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package bufcoreprotoparse - -import ( - "context" - "fmt" - "io" - "sync" - - "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" - "github.com/bufbuild/buf/internal/gen/data/datawkt" - "github.com/bufbuild/buf/internal/pkg/storage" - "go.uber.org/multierr" -) - -type parserAccessorHandler struct { - ctx context.Context - module bufmodule.Module - pathToExternalPath map[string]string - nonImportPaths map[string]struct{} - lock sync.RWMutex -} - -func newParserAccessorHandler( - ctx context.Context, - module bufmodule.Module, -) *parserAccessorHandler { - return &parserAccessorHandler{ - ctx: ctx, - module: module, - pathToExternalPath: make(map[string]string), - nonImportPaths: make(map[string]struct{}), - } -} - -func (p *parserAccessorHandler) Open(path string) (_ io.ReadCloser, retErr error) { - moduleFile, moduleErr := p.module.GetModuleFile(p.ctx, path) - if moduleErr != nil { - if !storage.IsNotExist(moduleErr) { - return nil, moduleErr - } - if wktModuleFile, wktErr := datawkt.ReadBucket.Get(p.ctx, path); wktErr == nil { - if wktModuleFile.Path() != path { - // this should never happen, but just in case - return nil, fmt.Errorf("parser accessor requested path %q but got %q", path, wktModuleFile.Path()) - } - if err := p.addPath(path, path, true); err != nil { - return nil, err - } - return wktModuleFile, nil - } - return nil, moduleErr - } - defer func() { - if retErr != nil { - retErr = multierr.Append(retErr, moduleFile.Close()) - } - }() - if moduleFile.Path() != path { - // this should never happen, but just in case - return nil, fmt.Errorf("parser accessor requested path %q but got %q", path, moduleFile.Path()) - } - if err := p.addPath(path, moduleFile.ExternalPath(), moduleFile.IsImport()); err != nil { - return nil, err - } - return moduleFile, nil -} - -func (p *parserAccessorHandler) ExternalPath(path string) string { - p.lock.RLock() - defer p.lock.RUnlock() - if externalPath := p.pathToExternalPath[path]; externalPath != "" { - return externalPath - } - return path -} - -func (p *parserAccessorHandler) IsImport(path string) bool { - p.lock.RLock() - defer p.lock.RUnlock() - _, isNotImport := p.nonImportPaths[path] - return !isNotImport -} - -func (p *parserAccessorHandler) addPath(path string, externalPath string, isImport bool) error { - p.lock.Lock() - defer p.lock.Unlock() - existingExternalPath, ok := p.pathToExternalPath[path] - if ok { - if existingExternalPath != externalPath { - return fmt.Errorf("parser accessor had external paths %q and %q for path %q", existingExternalPath, externalPath, path) - } - } else { - p.pathToExternalPath[path] = externalPath - } - if !isImport { - p.nonImportPaths[path] = struct{}{} - } - return nil -} diff --git a/internal/buf/bufcore/bufcoretesting/bufcoretesting.go b/internal/buf/bufcore/bufcoretesting/bufcoretesting.go index 37637948b3..8a133e466e 100644 --- a/internal/buf/bufcore/bufcoretesting/bufcoretesting.go +++ b/internal/buf/bufcore/bufcoretesting/bufcoretesting.go @@ -18,8 +18,6 @@ import ( "testing" "github.com/bufbuild/buf/internal/buf/bufcore" - "github.com/bufbuild/buf/internal/pkg/normalpath" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -38,30 +36,3 @@ func NewFileInfo( require.NoError(t, err) return fileInfo } - -// AssertFileInfosEqual asserts the expected FileInfos equal the actual FileInfos. -func AssertFileInfosEqual(t *testing.T, expected []bufcore.FileInfo, actual []bufcore.FileInfo) { - assert.Equal(t, expected, actual) -} - -// FileInfosToAbs converts the external paths to absolute. -func FileInfosToAbs(t *testing.T, fileInfos []bufcore.FileInfo) []bufcore.FileInfo { - newFileInfos := make([]bufcore.FileInfo, len(fileInfos)) - for i, fileInfo := range fileInfos { - newFileInfos[i] = FileInfoToAbs(t, fileInfo) - } - return newFileInfos -} - -// FileInfoToAbs converts the external path to absolute. -func FileInfoToAbs(t *testing.T, fileInfo bufcore.FileInfo) bufcore.FileInfo { - absExternalPath, err := normalpath.NormalizeAndAbsolute(fileInfo.ExternalPath()) - require.NoError(t, err) - newFileInfo, err := bufcore.NewFileInfo( - fileInfo.Path(), - absExternalPath, - fileInfo.IsImport(), - ) - require.NoError(t, err) - return newFileInfo -} diff --git a/internal/buf/bufcore/bufimage/bufimage.go b/internal/buf/bufcore/bufimage/bufimage.go index a9826af2eb..0915a634a1 100644 --- a/internal/buf/bufcore/bufimage/bufimage.go +++ b/internal/buf/bufcore/bufimage/bufimage.go @@ -15,7 +15,7 @@ package bufimage import ( - "github.com/bufbuild/buf/internal/buf/bufcore" + "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" imagev1 "github.com/bufbuild/buf/internal/gen/proto/go/buf/alpha/image/v1" "github.com/bufbuild/buf/internal/pkg/normalpath" "github.com/bufbuild/buf/internal/pkg/protodescriptor" @@ -26,7 +26,7 @@ import ( // ImageFile is a Protobuf file within an image. type ImageFile interface { - bufcore.FileInfo + bufmodule.FileInfo // Proto is the backing FileDescriptorProto for this File. // // This will never be nil. @@ -48,11 +48,13 @@ type ImageFile interface { // If externalPath is empty, path is used. func NewImageFile( fileDescriptorProto *descriptorpb.FileDescriptorProto, + moduleReference bufmodule.ModuleReference, externalPath string, isImport bool, ) (ImageFile, error) { return newImageFile( fileDescriptorProto, + moduleReference, externalPath, isImport, ) @@ -116,10 +118,19 @@ func NewImageForProto(protoImage *imagev1.Image) (Image, error) { if err != nil { return nil, err } + moduleReferenceRefs, err := getModuleReferenceRefs(protoImage) + if err != nil { + return nil, err + } imageFiles := make([]ImageFile, len(protoImage.File)) for i, fileDescriptorProto := range protoImage.File { _, isImport := importFileIndexes[i] - imageFile, err := NewImageFile(fileDescriptorProto, fileDescriptorProto.GetName(), isImport) + imageFile, err := NewImageFile( + fileDescriptorProto, + moduleReferenceRefs[i], // nil is a valid value. + fileDescriptorProto.GetName(), + isImport, + ) if err != nil { return nil, err } @@ -237,6 +248,18 @@ func ImageToProtoImage(image Image) *imagev1.Image { }, ) } + if moduleReference := imageFile.ModuleReference(); moduleReference != nil { + protoImage.BufbuildImageExtension.ModuleReferenceRefs = append( + protoImage.BufbuildImageExtension.ModuleReferenceRefs, + &imagev1.ModuleReferenceRef{ + FileIndex: proto.Uint32(uint32(i)), + Remote: proto.String(moduleReference.Remote()), + Owner: proto.String(moduleReference.Owner()), + Repository: proto.String(moduleReference.Repository()), + Reference: proto.String(moduleReference.Reference()), + }, + ) + } } return protoImage } diff --git a/internal/buf/bufcore/bufimage/bufimagebuild/builder.go b/internal/buf/bufcore/bufimage/bufimagebuild/builder.go index d34954924e..9a61a427cc 100644 --- a/internal/buf/bufcore/bufimage/bufimagebuild/builder.go +++ b/internal/buf/bufcore/bufimage/bufimagebuild/builder.go @@ -22,9 +22,9 @@ import ( "sync" "github.com/bufbuild/buf/internal/buf/bufanalysis" - "github.com/bufbuild/buf/internal/buf/bufcore/bufcoreprotoparse" "github.com/bufbuild/buf/internal/buf/bufcore/bufimage" "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" + "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule/bufmoduleprotoparse" "github.com/bufbuild/buf/internal/pkg/stringutil" "github.com/bufbuild/buf/internal/pkg/thread" "github.com/jhump/protoreflect/desc" @@ -68,7 +68,7 @@ func (b *builder) build( ctx, span := trace.StartSpan(ctx, "build") defer span.End() - parserAccessorHandler := bufcoreprotoparse.NewParserAccessorHandler(ctx, moduleFileSet) + parserAccessorHandler := bufmoduleprotoparse.NewParserAccessorHandler(ctx, moduleFileSet) targetFileInfos, err := moduleFileSet.TargetFileInfos(ctx) if err != nil { return nil, nil, err @@ -121,7 +121,7 @@ func (b *builder) build( func getBuildResults( ctx context.Context, - parserAccessorHandler bufcoreprotoparse.ParserAccessorHandler, + parserAccessorHandler bufmoduleprotoparse.ParserAccessorHandler, paths []string, excludeSourceCodeInfo bool, ) []*buildResult { @@ -177,7 +177,7 @@ func getBuildResults( func getBuildResult( ctx context.Context, - parserAccessorHandler bufcoreprotoparse.ParserAccessorHandler, + parserAccessorHandler bufmoduleprotoparse.ParserAccessorHandler, paths []string, excludeSourceCodeInfo bool, ) *buildResult { @@ -207,7 +207,7 @@ func getBuildResult( errors.New("got invalid source error from parse but no errors reported"), ) } - fileAnnotations, err := bufcoreprotoparse.GetFileAnnotations( + fileAnnotations, err := bufmoduleprotoparse.GetFileAnnotations( ctx, parserAccessorHandler, errorsWithPos, @@ -310,7 +310,7 @@ func getImage( ctx context.Context, excludeSourceCodeInfo bool, sortedFileDescriptors []*desc.FileDescriptor, - parserAccessorHandler bufcoreprotoparse.ParserAccessorHandler, + parserAccessorHandler bufmoduleprotoparse.ParserAccessorHandler, ) (bufimage.Image, error) { ctx, span := trace.StartSpan(ctx, "get_image") defer span.End() @@ -352,7 +352,7 @@ func getImageFilesRec( ctx context.Context, excludeSourceCodeInfo bool, descFileDescriptor *desc.FileDescriptor, - parserAccessorHandler bufcoreprotoparse.ParserAccessorHandler, + parserAccessorHandler bufmoduleprotoparse.ParserAccessorHandler, alreadySeen map[string]struct{}, nonImportFilenames map[string]struct{}, imageFiles []bufimage.ImageFile, @@ -393,6 +393,7 @@ func getImageFilesRec( _, isNotImport := nonImportFilenames[path] imageFile, err := bufimage.NewImageFile( fileDescriptorProto, + parserAccessorHandler.ModuleReference(path), // if empty, defaults to path parserAccessorHandler.ExternalPath(path), !isNotImport, diff --git a/internal/buf/bufcore/bufimage/bufimagetesting/bufimagetesting.go b/internal/buf/bufcore/bufimage/bufimagetesting/bufimagetesting.go index 9377a3dd42..e35e5e0940 100644 --- a/internal/buf/bufcore/bufimage/bufimagetesting/bufimagetesting.go +++ b/internal/buf/bufcore/bufimage/bufimagetesting/bufimagetesting.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/bufbuild/buf/internal/buf/bufcore/bufimage" + "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" @@ -28,11 +29,13 @@ import ( func NewImageFile( t testing.TB, fileDescriptorProto *descriptorpb.FileDescriptorProto, + moduleReference bufmodule.ModuleReference, externalPath string, isImport bool, ) bufimage.ImageFile { imageFile, err := bufimage.NewImageFile( fileDescriptorProto, + moduleReference, externalPath, isImport, ) @@ -69,6 +72,7 @@ func normalizeImageFiles(t testing.TB, imageFiles []bufimage.ImageFile) []bufima imageFile.Proto().GetName(), imageFile.Proto().GetDependency()..., ), + imageFile.ModuleReference(), imageFile.ExternalPath(), imageFile.IsImport(), ) diff --git a/internal/buf/bufcore/bufimage/bufimagetesting/bufimagetesting_test.go b/internal/buf/bufcore/bufimage/bufimagetesting/bufimagetesting_test.go index fd191f1313..a3382538fb 100644 --- a/internal/buf/bufcore/bufimage/bufimagetesting/bufimagetesting_test.go +++ b/internal/buf/bufcore/bufimage/bufimagetesting/bufimagetesting_test.go @@ -37,6 +37,7 @@ func BenchmarkNewImageWithOnlyPathsAllowNotExistFileOnly(b *testing.B) { b, fmt.Sprintf("a%d.proto/a%d.proto", i, i), ), + nil, fmt.Sprintf("foo/two/a%d.proto/a%d.proto", i, i), false, ), @@ -65,6 +66,7 @@ func BenchmarkNewImageWithOnlyPathsAllowNotExistDirOnly(b *testing.B) { b, fmt.Sprintf("a%d.proto/a%d.proto", i, i), ), + nil, fmt.Sprintf("foo/two/a%d.proto/a%d.proto", i, i), false, ), @@ -118,36 +120,42 @@ func TestBasic(t *testing.T) { fileImport := NewImageFile( t, fileDescriptorProtoImport, + nil, "some/import/import.proto", true, ) fileOneAA := NewImageFile( t, fileDescriptorProtoAA, + nil, "foo/one/a/a.proto", false, ) fileOneAB := NewImageFile( t, fileDescriptorProtoAB, + nil, "foo/one/a/b.proto", false, ) fileTwoBA := NewImageFile( t, fileDescriptorProtoBA, + nil, "foo/two/b/a.proto", false, ) fileTwoBB := NewImageFile( t, fileDescriptorProtoBB, + nil, "foo/two/b/b.proto", false, ) fileOutlandishDirectoryName := NewImageFile( t, fileDescriptorProtoOutlandishDirectoryName, + nil, "foo/three/d/d.proto/d.proto", false, ) @@ -166,12 +174,12 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "foo/one/a/a.proto", false), - NewImageFile(t, fileDescriptorProtoImport, "some/import/import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "foo/one/a/b.proto", false), - NewImageFile(t, fileDescriptorProtoBA, "foo/two/b/a.proto", false), - NewImageFile(t, fileDescriptorProtoBB, "foo/two/b/b.proto", false), - NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, "foo/three/d/d.proto/d.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "foo/one/a/a.proto", false), + NewImageFile(t, fileDescriptorProtoImport, nil, "some/import/import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "foo/one/a/b.proto", false), + NewImageFile(t, fileDescriptorProtoBA, nil, "foo/two/b/a.proto", false), + NewImageFile(t, fileDescriptorProtoBB, nil, "foo/two/b/b.proto", false), + NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, nil, "foo/three/d/d.proto/d.proto", false), }, image.Files(), ) @@ -180,11 +188,11 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "foo/one/a/a.proto", false), - NewImageFile(t, fileDescriptorProtoAB, "foo/one/a/b.proto", false), - NewImageFile(t, fileDescriptorProtoBA, "foo/two/b/a.proto", false), - NewImageFile(t, fileDescriptorProtoBB, "foo/two/b/b.proto", false), - NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, "foo/three/d/d.proto/d.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "foo/one/a/a.proto", false), + NewImageFile(t, fileDescriptorProtoAB, nil, "foo/one/a/b.proto", false), + NewImageFile(t, fileDescriptorProtoBA, nil, "foo/two/b/a.proto", false), + NewImageFile(t, fileDescriptorProtoBB, nil, "foo/two/b/b.proto", false), + NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, nil, "foo/three/d/d.proto/d.proto", false), }, bufimage.ImageWithoutImports(image).Files(), ) @@ -200,10 +208,10 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "foo/one/a/a.proto", true), - NewImageFile(t, fileDescriptorProtoImport, "some/import/import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "foo/one/a/b.proto", false), - NewImageFile(t, fileDescriptorProtoBA, "foo/two/b/a.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "foo/one/a/a.proto", true), + NewImageFile(t, fileDescriptorProtoImport, nil, "some/import/import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "foo/one/a/b.proto", false), + NewImageFile(t, fileDescriptorProtoBA, nil, "foo/two/b/a.proto", false), }, newImage.Files(), ) @@ -230,10 +238,10 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "foo/one/a/a.proto", true), - NewImageFile(t, fileDescriptorProtoImport, "some/import/import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "foo/one/a/b.proto", false), - NewImageFile(t, fileDescriptorProtoBA, "foo/two/b/a.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "foo/one/a/a.proto", true), + NewImageFile(t, fileDescriptorProtoImport, nil, "some/import/import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "foo/one/a/b.proto", false), + NewImageFile(t, fileDescriptorProtoBA, nil, "foo/two/b/a.proto", false), }, newImage.Files(), ) @@ -247,9 +255,9 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "foo/one/a/a.proto", false), - NewImageFile(t, fileDescriptorProtoImport, "some/import/import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "foo/one/a/b.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "foo/one/a/a.proto", false), + NewImageFile(t, fileDescriptorProtoImport, nil, "some/import/import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "foo/one/a/b.proto", false), }, newImage.Files(), ) @@ -263,11 +271,11 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "foo/one/a/a.proto", true), - NewImageFile(t, fileDescriptorProtoImport, "some/import/import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "foo/one/a/b.proto", true), - NewImageFile(t, fileDescriptorProtoBA, "foo/two/b/a.proto", false), - NewImageFile(t, fileDescriptorProtoBB, "foo/two/b/b.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "foo/one/a/a.proto", true), + NewImageFile(t, fileDescriptorProtoImport, nil, "some/import/import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "foo/one/a/b.proto", true), + NewImageFile(t, fileDescriptorProtoBA, nil, "foo/two/b/a.proto", false), + NewImageFile(t, fileDescriptorProtoBB, nil, "foo/two/b/b.proto", false), }, newImage.Files(), ) @@ -282,10 +290,10 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "foo/one/a/a.proto", false), - NewImageFile(t, fileDescriptorProtoImport, "some/import/import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "foo/one/a/b.proto", false), - NewImageFile(t, fileDescriptorProtoBA, "foo/two/b/a.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "foo/one/a/a.proto", false), + NewImageFile(t, fileDescriptorProtoImport, nil, "some/import/import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "foo/one/a/b.proto", false), + NewImageFile(t, fileDescriptorProtoBA, nil, "foo/two/b/a.proto", false), }, newImage.Files(), ) @@ -310,10 +318,10 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "foo/one/a/a.proto", false), - NewImageFile(t, fileDescriptorProtoImport, "some/import/import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "foo/one/a/b.proto", false), - NewImageFile(t, fileDescriptorProtoBA, "foo/two/b/a.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "foo/one/a/a.proto", false), + NewImageFile(t, fileDescriptorProtoImport, nil, "some/import/import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "foo/one/a/b.proto", false), + NewImageFile(t, fileDescriptorProtoBA, nil, "foo/two/b/a.proto", false), }, newImage.Files(), ) @@ -329,11 +337,11 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "foo/one/a/a.proto", false), - NewImageFile(t, fileDescriptorProtoImport, "some/import/import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "foo/one/a/b.proto", false), - NewImageFile(t, fileDescriptorProtoBA, "foo/two/b/a.proto", false), - NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, "foo/three/d/d.proto/d.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "foo/one/a/a.proto", false), + NewImageFile(t, fileDescriptorProtoImport, nil, "some/import/import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "foo/one/a/b.proto", false), + NewImageFile(t, fileDescriptorProtoBA, nil, "foo/two/b/a.proto", false), + NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, nil, "foo/three/d/d.proto/d.proto", false), }, newImage.Files(), ) @@ -349,11 +357,11 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "foo/one/a/a.proto", false), - NewImageFile(t, fileDescriptorProtoImport, "some/import/import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "foo/one/a/b.proto", false), - NewImageFile(t, fileDescriptorProtoBA, "foo/two/b/a.proto", false), - NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, "foo/three/d/d.proto/d.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "foo/one/a/a.proto", false), + NewImageFile(t, fileDescriptorProtoImport, nil, "some/import/import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "foo/one/a/b.proto", false), + NewImageFile(t, fileDescriptorProtoBA, nil, "foo/two/b/a.proto", false), + NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, nil, "foo/three/d/d.proto/d.proto", false), }, newImage.Files(), ) @@ -380,12 +388,12 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "a/a.proto", false), - NewImageFile(t, fileDescriptorProtoImport, "import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "a/b.proto", false), - NewImageFile(t, fileDescriptorProtoBA, "b/a.proto", false), - NewImageFile(t, fileDescriptorProtoBB, "b/b.proto", false), - NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, "d/d.proto/d.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "a/a.proto", false), + NewImageFile(t, fileDescriptorProtoImport, nil, "import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "a/b.proto", false), + NewImageFile(t, fileDescriptorProtoBA, nil, "b/a.proto", false), + NewImageFile(t, fileDescriptorProtoBB, nil, "b/b.proto", false), + NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, nil, "d/d.proto/d.proto", false), }, newImage.Files(), ) @@ -436,12 +444,12 @@ func TestBasic(t *testing.T) { AssertImageFilesEqual( t, []bufimage.ImageFile{ - NewImageFile(t, fileDescriptorProtoAA, "a/a.proto", false), - NewImageFile(t, fileDescriptorProtoImport, "import.proto", true), - NewImageFile(t, fileDescriptorProtoAB, "a/b.proto", false), - NewImageFile(t, fileDescriptorProtoBA, "b/a.proto", false), - NewImageFile(t, fileDescriptorProtoBB, "b/b.proto", false), - NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, "d/d.proto/d.proto", false), + NewImageFile(t, fileDescriptorProtoAA, nil, "a/a.proto", false), + NewImageFile(t, fileDescriptorProtoImport, nil, "import.proto", true), + NewImageFile(t, fileDescriptorProtoAB, nil, "a/b.proto", false), + NewImageFile(t, fileDescriptorProtoBA, nil, "b/a.proto", false), + NewImageFile(t, fileDescriptorProtoBB, nil, "b/b.proto", false), + NewImageFile(t, fileDescriptorProtoOutlandishDirectoryName, nil, "d/d.proto/d.proto", false), }, newImage.Files(), ) diff --git a/internal/buf/bufcore/bufimage/image_file.go b/internal/buf/bufcore/bufimage/image_file.go index 8d787c01bb..58fa04490b 100644 --- a/internal/buf/bufcore/bufimage/image_file.go +++ b/internal/buf/bufcore/bufimage/image_file.go @@ -16,6 +16,7 @@ package bufimage import ( "github.com/bufbuild/buf/internal/buf/bufcore" + "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" "github.com/bufbuild/buf/internal/pkg/protodescriptor" "google.golang.org/protobuf/types/descriptorpb" ) @@ -23,20 +24,21 @@ import ( var _ ImageFile = &imageFile{} type imageFile struct { - bufcore.FileInfo + bufmodule.FileInfo fileDescriptorProto *descriptorpb.FileDescriptorProto } func newImageFile( fileDescriptorProto *descriptorpb.FileDescriptorProto, + moduleReference bufmodule.ModuleReference, externalPath string, isImport bool, ) (*imageFile, error) { if err := protodescriptor.ValidateFileDescriptorProto(fileDescriptorProto); err != nil { return nil, err } - fileInfo, err := bufcore.NewFileInfo( + coreFileInfo, err := bufcore.NewFileInfo( fileDescriptorProto.GetName(), externalPath, isImport, @@ -45,7 +47,7 @@ func newImageFile( return nil, err } return &imageFile{ - FileInfo: fileInfo, + FileInfo: bufmodule.NewFileInfo(coreFileInfo, moduleReference), fileDescriptorProto: fileDescriptorProto, }, nil } @@ -60,7 +62,7 @@ func (f *imageFile) ImportPaths() []string { func (f *imageFile) withIsImport(isImport bool) ImageFile { return &imageFile{ - FileInfo: f.FileInfo.WithIsImport(isImport), + FileInfo: bufmodule.NewFileInfo(f.FileInfo.WithIsImport(isImport), f.FileInfo.ModuleReference()), fileDescriptorProto: f.fileDescriptorProto, } } diff --git a/internal/buf/bufcore/bufimage/util.go b/internal/buf/bufcore/bufimage/util.go index c6394d3bb1..1dd2735a49 100644 --- a/internal/buf/bufcore/bufimage/util.go +++ b/internal/buf/bufcore/bufimage/util.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" + "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" "github.com/bufbuild/buf/internal/buf/bufcore/internal/bufcorevalidate" imagev1 "github.com/bufbuild/buf/internal/gen/proto/go/buf/alpha/image/v1" "github.com/bufbuild/buf/internal/pkg/normalpath" @@ -30,13 +31,35 @@ func getImportFileIndexes(protoImage *imagev1.Image) (map[int]struct{}, error) { for _, imageImportRef := range imageImportRefs { if imageImportRef.FileIndex == nil { // this should have been caught in validation but just in case - return nil, errors.New("nil fileIndex") + return nil, errors.New("nil image import ref fileIndex") } importFileIndexes[int(imageImportRef.GetFileIndex())] = struct{}{} } return importFileIndexes, nil } +func getModuleReferenceRefs(protoImage *imagev1.Image) (map[int]bufmodule.ModuleReference, error) { + moduleReferenceRefs := protoImage.GetBufbuildImageExtension().GetModuleReferenceRefs() + parsedModuleReferenceRefs := make(map[int]bufmodule.ModuleReference, len(moduleReferenceRefs)) + for _, moduleReferenceRef := range moduleReferenceRefs { + if moduleReferenceRef.FileIndex == nil { + // this should have been caught in validation but just in case + return nil, errors.New("nil module reference ref fileIndex") + } + parsedModuleReferenceRef, err := bufmodule.NewModuleReference( + moduleReferenceRef.GetRemote(), + moduleReferenceRef.GetOwner(), + moduleReferenceRef.GetRepository(), + moduleReferenceRef.GetReference(), + ) + if err != nil { + return nil, err + } + parsedModuleReferenceRefs[int(moduleReferenceRef.GetFileIndex())] = parsedModuleReferenceRef + } + return parsedModuleReferenceRefs, nil +} + // paths can be either files (ending in .proto) or directories // paths must be normalized and validated, and not duplicated // if a directory, all .proto files underneath will be included diff --git a/internal/buf/bufcore/bufmodule/bufmodule.go b/internal/buf/bufcore/bufmodule/bufmodule.go index be1852b326..7761661e1a 100644 --- a/internal/buf/bufcore/bufmodule/bufmodule.go +++ b/internal/buf/bufcore/bufmodule/bufmodule.go @@ -40,9 +40,29 @@ const ( b1DigestPrefix = "b1" ) +// FileInfo contains module file info. +type FileInfo interface { + bufcore.FileInfo + + // Note this *can* be nil if we did not build from a named module. + // All code must assume this can be nil. + // nil checking should work since the backing type is always a pointer + ModuleReference() ModuleReference + + isFileInfo() +} + +// NewFileInfo returns a new FileInfo. +func NewFileInfo( + coreFileInfo bufcore.FileInfo, + moduleReference ModuleReference, +) FileInfo { + return newFileInfo(coreFileInfo, moduleReference) +} + // ModuleFile is a module file. type ModuleFile interface { - bufcore.FileInfo + FileInfo io.ReadCloser isModuleFile() @@ -309,13 +329,13 @@ type Module interface { // It does not include dependencies. // // The returned TargetFileInfos are sorted by path. - TargetFileInfos(ctx context.Context) ([]bufcore.FileInfo, error) + TargetFileInfos(ctx context.Context) ([]FileInfo, error) // SourceFileInfos gets all FileInfos belonging to the module. // // It does not include dependencies. // // The returned SourceFileInfos are sorted by path. - SourceFileInfos(ctx context.Context) ([]bufcore.FileInfo, error) + SourceFileInfos(ctx context.Context) ([]FileInfo, error) // GetModuleFile gets the source file for the given path. // // Returns storage.IsNotExist error if the file does not exist. @@ -329,16 +349,31 @@ type Module interface { DependencyModulePins() []ModulePin getSourceReadBucket() storage.ReadBucket + // Note this *can* be nil if we did not build from a named module. + // All code must assume this can be nil. + // nil checking should work since the backing type is always a pointer. + // + // TODO: We can remove the getModuleReference method on the if we fetch + // FileInfos from the Module and plumb in the ModuleReference here. + // + // This approach assumes that all of the FileInfos returned + // from SourceFileInfos will have their ModuleReference + // set to the same value, which can be validated. + getModuleReference() ModuleReference isModule() } +// ModuleOption is used to construct Modules. +type ModuleOption func(*moduleOptions) + // NewModuleForBucket returns a new Module. It attempts reads dependencies // from a lock file in the read bucket. func NewModuleForBucket( ctx context.Context, readBucket storage.ReadBucket, + options ...ModuleOption, ) (Module, error) { - return newModuleForBucket(ctx, readBucket) + return newModuleForBucket(ctx, readBucket, options...) } // NewModuleForBucketWithDependencyModulePins explicitly specifies the dependencies @@ -348,16 +383,18 @@ func NewModuleForBucketWithDependencyModulePins( ctx context.Context, readBucket storage.ReadBucket, dependencyModulePins []ModulePin, + options ...ModuleOption, ) (Module, error) { - return newModuleForBucketWithDependencyModulePins(ctx, readBucket, dependencyModulePins) + return newModuleForBucketWithDependencyModulePins(ctx, readBucket, dependencyModulePins, options...) } // NewModuleForProto returns a new Module for the given proto Module. func NewModuleForProto( ctx context.Context, protoModule *modulev1alpha1.Module, + options ...ModuleOption, ) (Module, error) { - return newModuleForProto(ctx, protoModule) + return newModuleForProto(ctx, protoModule, options...) } // ModuleWithTargetPaths returns a new Module that specifies specific file or directory paths to build. @@ -420,7 +457,7 @@ type ModuleFileSet interface { // AllFileInfos gets all FileInfos associated with the module, including dependencies. // // The returned FileInfos are sorted by path. - AllFileInfos(ctx context.Context) ([]bufcore.FileInfo, error) + AllFileInfos(ctx context.Context) ([]FileInfo, error) isModuleFileSet() } @@ -637,6 +674,66 @@ func SortModulePins(modulePins []ModulePin) { }) } +// ObjectInfo extends the storage.ObjectInfo interface with +// information specific to Modules. +// +// TODO: This type is currently not used outside of this package, +// so we could instead rely on just the concrete struct type. +// This includes an interface for consistency, and exports it to +// prevent name collisions. +type ObjectInfo interface { + storage.ObjectInfo + + // ModuleReference gets this object's ModuleReference, if any. + ModuleReference() ModuleReference +} + +// ReadBucket extends the storage.ReadBucket interface with +// information specific to Modules. +// +// TODO: This type is currently not used outside of this package, +// so we could instead rely on just the concrete struct type. +// This includes an interface for consistency, and exports it to +// prevent name collisions. +type ReadBucket interface { + storage.ReadBucket + + // StatModuleFile gets info in the object, including info + // specific to the file's module. + // + // Returns ErrNotExist if the path does not exist, other error + // if there is a system error. + StatModuleFile(ctx context.Context, path string) (ObjectInfo, error) + // WalkModuleFiles walks the bucket with the prefix, calling f on + // each path. If the prefix doesn't exist, this is a no-op. + // + // Note that foo/barbaz will not be called for foo/bar, but will + // be called for foo/bar/baz. + // + // All paths given to f are normalized and validated. + // If f returns error, Walk will stop short and return this error. + // Returns other error on system error. + WalkModuleFiles(ctx context.Context, prefix string, f func(ObjectInfo) error) error +} + +// NewReadBucket returns a new ReadBucket. +func NewReadBucket( + sourceReadBucket storage.ReadBucket, + moduleReference ModuleReference, +) ReadBucket { + return newReadBucket(sourceReadBucket, moduleReference) +} + +// sortFileInfos sorts the FileInfos. +func sortFileInfos(fileInfos []FileInfo) { + sort.Slice( + fileInfos, + func(i int, j int) bool { + return fileInfos[i].Path() < fileInfos[j].Path() + }, + ) +} + // parseModuleReferenceComponents parses and returns the remote, owner, repository, // and ref (branch or commit) from the given path. func parseModuleReferenceComponents(path string) (remote string, owner string, repository string, ref string, err error) { diff --git a/internal/buf/bufcore/bufmodule/bufmodulebuild/module_bucket_builder_test.go b/internal/buf/bufcore/bufmodule/bufmodulebuild/module_bucket_builder_test.go index 0f907e1546..783c26d1d5 100644 --- a/internal/buf/bufcore/bufmodule/bufmodulebuild/module_bucket_builder_test.go +++ b/internal/buf/bufcore/bufmodule/bufmodulebuild/module_bucket_builder_test.go @@ -19,8 +19,8 @@ import ( "path/filepath" "testing" - "github.com/bufbuild/buf/internal/buf/bufcore" "github.com/bufbuild/buf/internal/buf/bufcore/bufcoretesting" + "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule/internal" "github.com/bufbuild/buf/internal/pkg/normalpath" "github.com/bufbuild/buf/internal/pkg/storage/storageos" @@ -39,15 +39,15 @@ func TestBucketGetFileInfos1(t *testing.T) { []string{ "proto/b", }, - bufcoretesting.NewFileInfo(t, "a/1.proto", "testdata/1/proto/a/1.proto", false), - bufcoretesting.NewFileInfo(t, "a/2.proto", "testdata/1/proto/a/2.proto", false), - bufcoretesting.NewFileInfo(t, "a/3.proto", "testdata/1/proto/a/3.proto", false), - bufcoretesting.NewFileInfo(t, "a/c/1.proto", "testdata/1/proto/a/c/1.proto", false), - bufcoretesting.NewFileInfo(t, "a/c/2.proto", "testdata/1/proto/a/c/2.proto", false), - bufcoretesting.NewFileInfo(t, "a/c/3.proto", "testdata/1/proto/a/c/3.proto", false), - bufcoretesting.NewFileInfo(t, "d/1.proto", "testdata/1/proto/d/1.proto", false), - bufcoretesting.NewFileInfo(t, "d/2.proto", "testdata/1/proto/d/2.proto", false), - bufcoretesting.NewFileInfo(t, "d/3.proto", "testdata/1/proto/d/3.proto", false), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/1.proto", "testdata/1/proto/a/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/2.proto", "testdata/1/proto/a/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/3.proto", "testdata/1/proto/a/3.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/c/1.proto", "testdata/1/proto/a/c/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/c/2.proto", "testdata/1/proto/a/c/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/c/3.proto", "testdata/1/proto/a/c/3.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/1.proto", "testdata/1/proto/d/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/2.proto", "testdata/1/proto/d/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/3.proto", "testdata/1/proto/d/3.proto", false), nil), ) } @@ -61,12 +61,12 @@ func TestBucketGetFileInfos2(t *testing.T) { []string{ "proto/a", }, - bufcoretesting.NewFileInfo(t, "b/1.proto", "testdata/1/proto/b/1.proto", false), - bufcoretesting.NewFileInfo(t, "b/2.proto", "testdata/1/proto/b/2.proto", false), - bufcoretesting.NewFileInfo(t, "b/3.proto", "testdata/1/proto/b/3.proto", false), - bufcoretesting.NewFileInfo(t, "d/1.proto", "testdata/1/proto/d/1.proto", false), - bufcoretesting.NewFileInfo(t, "d/2.proto", "testdata/1/proto/d/2.proto", false), - bufcoretesting.NewFileInfo(t, "d/3.proto", "testdata/1/proto/d/3.proto", false), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/1.proto", "testdata/1/proto/b/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/2.proto", "testdata/1/proto/b/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/3.proto", "testdata/1/proto/b/3.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/1.proto", "testdata/1/proto/d/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/2.proto", "testdata/1/proto/d/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/3.proto", "testdata/1/proto/d/3.proto", false), nil), ) } @@ -80,15 +80,15 @@ func TestBucketGetFileInfo3(t *testing.T) { []string{ "proto/a/c", }, - bufcoretesting.NewFileInfo(t, "a/1.proto", "testdata/1/proto/a/1.proto", false), - bufcoretesting.NewFileInfo(t, "a/2.proto", "testdata/1/proto/a/2.proto", false), - bufcoretesting.NewFileInfo(t, "a/3.proto", "testdata/1/proto/a/3.proto", false), - bufcoretesting.NewFileInfo(t, "b/1.proto", "testdata/1/proto/b/1.proto", false), - bufcoretesting.NewFileInfo(t, "b/2.proto", "testdata/1/proto/b/2.proto", false), - bufcoretesting.NewFileInfo(t, "b/3.proto", "testdata/1/proto/b/3.proto", false), - bufcoretesting.NewFileInfo(t, "d/1.proto", "testdata/1/proto/d/1.proto", false), - bufcoretesting.NewFileInfo(t, "d/2.proto", "testdata/1/proto/d/2.proto", false), - bufcoretesting.NewFileInfo(t, "d/3.proto", "testdata/1/proto/d/3.proto", false), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/1.proto", "testdata/1/proto/a/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/2.proto", "testdata/1/proto/a/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/3.proto", "testdata/1/proto/a/3.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/1.proto", "testdata/1/proto/b/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/2.proto", "testdata/1/proto/b/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/3.proto", "testdata/1/proto/b/3.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/1.proto", "testdata/1/proto/d/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/2.proto", "testdata/1/proto/d/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/3.proto", "testdata/1/proto/d/3.proto", false), nil), ) } @@ -103,12 +103,12 @@ func TestBucketGetFileInfos4(t *testing.T) { "proto/a/c", "proto/d", }, - bufcoretesting.NewFileInfo(t, "a/1.proto", "testdata/1/proto/a/1.proto", false), - bufcoretesting.NewFileInfo(t, "a/2.proto", "testdata/1/proto/a/2.proto", false), - bufcoretesting.NewFileInfo(t, "a/3.proto", "testdata/1/proto/a/3.proto", false), - bufcoretesting.NewFileInfo(t, "b/1.proto", "testdata/1/proto/b/1.proto", false), - bufcoretesting.NewFileInfo(t, "b/2.proto", "testdata/1/proto/b/2.proto", false), - bufcoretesting.NewFileInfo(t, "b/3.proto", "testdata/1/proto/b/3.proto", false), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/1.proto", "testdata/1/proto/a/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/2.proto", "testdata/1/proto/a/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/3.proto", "testdata/1/proto/a/3.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/1.proto", "testdata/1/proto/b/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/2.proto", "testdata/1/proto/b/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/3.proto", "testdata/1/proto/b/3.proto", false), nil), ) } @@ -160,7 +160,7 @@ func testBucketGetFileInfos( relDir string, relRoots []string, relExcludes []string, - expectedFileInfos ...bufcore.FileInfo, + expectedFileInfos ...bufmodule.FileInfo, ) { t.Parallel() storageosProvider := storageos.NewProvider(storageos.ProviderWithSymlinks()) @@ -184,7 +184,7 @@ func testBucketGetFileInfos( require.NoError(t, err) fileInfos, err := module.SourceFileInfos(context.Background()) assert.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, expectedFileInfos, fileInfos, @@ -208,7 +208,7 @@ func testBucketGetFileInfos( require.NoError(t, err) fileInfos, err := module.TargetFileInfos(context.Background()) assert.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, expectedFileInfos, fileInfos, diff --git a/internal/buf/bufcore/bufmodule/bufmodulebuild/module_include_builder_test.go b/internal/buf/bufcore/bufmodule/bufmodulebuild/module_include_builder_test.go index a554bc1f28..a2a137bc30 100644 --- a/internal/buf/bufcore/bufmodule/bufmodulebuild/module_include_builder_test.go +++ b/internal/buf/bufcore/bufmodule/bufmodulebuild/module_include_builder_test.go @@ -22,6 +22,7 @@ import ( "github.com/bufbuild/buf/internal/buf/bufcore" "github.com/bufbuild/buf/internal/buf/bufcore/bufcoretesting" + "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule/internal" "github.com/bufbuild/buf/internal/pkg/normalpath" "github.com/bufbuild/buf/internal/pkg/storage/storageos" @@ -37,18 +38,18 @@ func TestIncludeGetFileInfos1(t *testing.T) { []string{ "proto", }, - bufcoretesting.NewFileInfo(t, "a/1.proto", "testdata/1/proto/a/1.proto", false), - bufcoretesting.NewFileInfo(t, "a/2.proto", "testdata/1/proto/a/2.proto", false), - bufcoretesting.NewFileInfo(t, "a/3.proto", "testdata/1/proto/a/3.proto", false), - bufcoretesting.NewFileInfo(t, "a/c/1.proto", "testdata/1/proto/a/c/1.proto", false), - bufcoretesting.NewFileInfo(t, "a/c/2.proto", "testdata/1/proto/a/c/2.proto", false), - bufcoretesting.NewFileInfo(t, "a/c/3.proto", "testdata/1/proto/a/c/3.proto", false), - bufcoretesting.NewFileInfo(t, "b/1.proto", "testdata/1/proto/b/1.proto", false), - bufcoretesting.NewFileInfo(t, "b/2.proto", "testdata/1/proto/b/2.proto", false), - bufcoretesting.NewFileInfo(t, "b/3.proto", "testdata/1/proto/b/3.proto", false), - bufcoretesting.NewFileInfo(t, "d/1.proto", "testdata/1/proto/d/1.proto", false), - bufcoretesting.NewFileInfo(t, "d/2.proto", "testdata/1/proto/d/2.proto", false), - bufcoretesting.NewFileInfo(t, "d/3.proto", "testdata/1/proto/d/3.proto", false), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/1.proto", "testdata/1/proto/a/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/2.proto", "testdata/1/proto/a/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/3.proto", "testdata/1/proto/a/3.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/c/1.proto", "testdata/1/proto/a/c/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/c/2.proto", "testdata/1/proto/a/c/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "a/c/3.proto", "testdata/1/proto/a/c/3.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/1.proto", "testdata/1/proto/b/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/2.proto", "testdata/1/proto/b/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "b/3.proto", "testdata/1/proto/b/3.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/1.proto", "testdata/1/proto/d/1.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/2.proto", "testdata/1/proto/d/2.proto", false), nil), + bufmodule.NewFileInfo(bufcoretesting.NewFileInfo(t, "d/3.proto", "testdata/1/proto/d/3.proto", false), nil), ) } @@ -85,14 +86,14 @@ func testIncludeGetFileInfos( t *testing.T, relDir string, relRoots []string, - expectedFileInfos ...bufcore.FileInfo, + expectedFileInfos ...bufmodule.FileInfo, ) { storageosProvider := storageos.NewProvider(storageos.ProviderWithSymlinks()) for _, isAbs := range []bool{true, false} { isAbs := isAbs expectedFileInfos := expectedFileInfos if isAbs { - expectedFileInfos = bufcoretesting.FileInfosToAbs(t, expectedFileInfos) + expectedFileInfos = fileInfosToAbs(t, expectedFileInfos) } t.Run(fmt.Sprintf("abs=%v", isAbs), func(t *testing.T) { t.Parallel() @@ -104,7 +105,7 @@ func testIncludeGetFileInfos( require.NoError(t, err) fileInfos, err := module.SourceFileInfos(context.Background()) assert.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, expectedFileInfos, fileInfos, @@ -123,7 +124,7 @@ func testIncludeGetFileInfos( require.NoError(t, err) fileInfos, err := module.TargetFileInfos(context.Background()) assert.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, expectedFileInfos, fileInfos, @@ -201,3 +202,20 @@ func testIncludeDirPaths( } return includeDirPaths } + +// fileInfosToAbs converts the external paths to absolute. +func fileInfosToAbs(t *testing.T, fileInfos []bufmodule.FileInfo) []bufmodule.FileInfo { + newFileInfos := make([]bufmodule.FileInfo, len(fileInfos)) + for i, fileInfo := range fileInfos { + absExternalPath, err := normalpath.NormalizeAndAbsolute(fileInfo.ExternalPath()) + require.NoError(t, err) + newCoreFileInfo, err := bufcore.NewFileInfo( + fileInfo.Path(), + absExternalPath, + fileInfo.IsImport(), + ) + require.NoError(t, err) + newFileInfos[i] = bufmodule.NewFileInfo(newCoreFileInfo, nil) + } + return newFileInfos +} diff --git a/internal/buf/bufcore/bufmodule/bufmodulecache/module_cacher.go b/internal/buf/bufcore/bufmodule/bufmodulecache/module_cacher.go index 3059891c1d..62365004b7 100644 --- a/internal/buf/bufcore/bufmodule/bufmodulecache/module_cacher.go +++ b/internal/buf/bufcore/bufmodule/bufmodulecache/module_cacher.go @@ -58,7 +58,16 @@ func (m *moduleCacher) GetModule( if !exists { return nil, multierr.Append(storage.NewErrNotExist(modulePath), unlocker.Unlock()) } - module, err := bufmodule.NewModuleForBucket(ctx, readWriteBucket) + moduleReference, err := bufmodule.NewModuleReference( + modulePin.Remote(), + modulePin.Owner(), + modulePin.Repository(), + modulePin.Commit(), // TODO: When a reference is resolved from a pin, we use the commit. A ModuleReferenceToModulePin mapper would be helpful. + ) + if err != nil { + return nil, err + } + module, err := bufmodule.NewModuleForBucket(ctx, readWriteBucket, bufmodule.ModuleWithModuleReference(moduleReference)) if err != nil { return nil, multierr.Append(err, unlocker.Unlock()) } diff --git a/internal/buf/bufcore/bufmodule/module.go b/internal/buf/bufcore/bufmodule/module.go index 65609b5744..3c7f969f42 100644 --- a/internal/buf/bufcore/bufmodule/module.go +++ b/internal/buf/bufcore/bufmodule/module.go @@ -28,9 +28,14 @@ import ( type module struct { sourceReadBucket storage.ReadBucket dependencyModulePins []ModulePin + moduleReference ModuleReference } -func newModuleForProto(ctx context.Context, protoModule *modulev1alpha1.Module) (*module, error) { +func newModuleForProto( + ctx context.Context, + protoModule *modulev1alpha1.Module, + options ...ModuleOption, +) (*module, error) { if err := ValidateProtoModule(protoModule); err != nil { return nil, err } @@ -53,48 +58,62 @@ func newModuleForProto(ctx context.Context, protoModule *modulev1alpha1.Module) ctx, sourceReadBucket, dependencyModulePins, + options..., ) } func newModuleForBucket( ctx context.Context, sourceReadBucket storage.ReadBucket, + options ...ModuleOption, ) (*module, error) { dependencyModulePins, err := getDependencyModulePinsForBucket(ctx, sourceReadBucket) if err != nil { return nil, err } - return newModuleForBucketWithDependencyModulePins(ctx, sourceReadBucket, dependencyModulePins) + return newModuleForBucketWithDependencyModulePins( + ctx, + sourceReadBucket, + dependencyModulePins, + options..., + ) } func newModuleForBucketWithDependencyModulePins( ctx context.Context, sourceReadBucket storage.ReadBucket, dependencyModulePins []ModulePin, + options ...ModuleOption, ) (*module, error) { if err := ValidateModulePinsUniqueByIdentity(dependencyModulePins); err != nil { return nil, err } + moduleOptions := &moduleOptions{} + for _, option := range options { + option(moduleOptions) + } // we rely on this being sorted here SortModulePins(dependencyModulePins) return &module{ sourceReadBucket: storage.MapReadBucket(sourceReadBucket, storage.MatchPathExt(".proto")), dependencyModulePins: dependencyModulePins, + moduleReference: moduleOptions.moduleReference, }, nil } -func (m *module) TargetFileInfos(ctx context.Context) ([]bufcore.FileInfo, error) { +func (m *module) TargetFileInfos(ctx context.Context) ([]FileInfo, error) { return m.SourceFileInfos(ctx) } -func (m *module) SourceFileInfos(ctx context.Context) ([]bufcore.FileInfo, error) { - var fileInfos []bufcore.FileInfo +func (m *module) SourceFileInfos(ctx context.Context) ([]FileInfo, error) { + var fileInfos []FileInfo if err := m.sourceReadBucket.Walk(ctx, "", func(objectInfo storage.ObjectInfo) error { // super overkill but ok if err := validateModuleFilePathWithoutNormalization(objectInfo.Path()); err != nil { return err } - fileInfos = append(fileInfos, bufcore.NewFileInfoForObjectInfo(objectInfo, false)) + coreFileInfo := bufcore.NewFileInfoForObjectInfo(objectInfo, false) + fileInfos = append(fileInfos, NewFileInfo(coreFileInfo, m.moduleReference)) return nil }); err != nil { return nil, fmt.Errorf("failed to enumerate module files: %w", err) @@ -102,20 +121,21 @@ func (m *module) SourceFileInfos(ctx context.Context) ([]bufcore.FileInfo, error if len(fileInfos) == 0 { return nil, internal.ErrNoTargetFiles } - bufcore.SortFileInfos(fileInfos) + sortFileInfos(fileInfos) return fileInfos, nil } func (m *module) GetModuleFile(ctx context.Context, path string) (ModuleFile, error) { // super overkill but ok - if err := validateModuleFilePath(path); err != nil { + if err := ValidateModuleFilePath(path); err != nil { return nil, err } readObjectCloser, err := m.sourceReadBucket.Get(ctx, path) if err != nil { return nil, err } - return newModuleFile(bufcore.NewFileInfoForObjectInfo(readObjectCloser, false), readObjectCloser), nil + coreFileInfo := bufcore.NewFileInfoForObjectInfo(readObjectCloser, false) + return newModuleFile(NewFileInfo(coreFileInfo, m.moduleReference), readObjectCloser), nil } func (m *module) DependencyModulePins() []ModulePin { @@ -127,4 +147,8 @@ func (m *module) getSourceReadBucket() storage.ReadBucket { return m.sourceReadBucket } +func (m *module) getModuleReference() ModuleReference { + return m.moduleReference +} + func (m *module) isModule() {} diff --git a/internal/buf/bufcore/bufmodule/module_file.go b/internal/buf/bufcore/bufmodule/module_file.go index b6eeb5f6ae..1b89077bba 100644 --- a/internal/buf/bufcore/bufmodule/module_file.go +++ b/internal/buf/bufcore/bufmodule/module_file.go @@ -16,18 +16,16 @@ package bufmodule import ( "io" - - "github.com/bufbuild/buf/internal/buf/bufcore" ) var _ ModuleFile = &moduleFile{} type moduleFile struct { - bufcore.FileInfo + FileInfo io.ReadCloser } -func newModuleFile(fileInfo bufcore.FileInfo, readCloser io.ReadCloser) moduleFile { +func newModuleFile(fileInfo FileInfo, readCloser io.ReadCloser) moduleFile { return moduleFile{ FileInfo: fileInfo, ReadCloser: readCloser, diff --git a/internal/buf/bufcore/bufmodule/module_file_set.go b/internal/buf/bufcore/bufmodule/module_file_set.go index ed3490acb1..229decfda1 100644 --- a/internal/buf/bufcore/bufmodule/module_file_set.go +++ b/internal/buf/bufcore/bufmodule/module_file_set.go @@ -26,27 +26,44 @@ var _ ModuleFileSet = &moduleFileSet{} type moduleFileSet struct { Module - allReadBucket storage.ReadBucket + allReadBucket ReadBucket } func newModuleFileSet( module Module, dependencies []Module, ) *moduleFileSet { - readBuckets := []storage.ReadBucket{module.getSourceReadBucket()} + // TODO: We can remove the getModuleReference method on the + // Module type if we fetch FileInfos from the Module + // and plumb in the ModuleReference here. + // + // This approach assumes that all of the FileInfos returned + // from SourceFileInfos will have their ModuleReference + // set to the same value. That can be enforced here. + moduleReadBuckets := []ReadBucket{ + NewReadBucket( + module.getSourceReadBucket(), + module.getModuleReference(), + ), + } for _, dependency := range dependencies { - readBuckets = append(readBuckets, dependency.getSourceReadBucket()) + moduleReadBuckets = append( + moduleReadBuckets, + NewReadBucket( + dependency.getSourceReadBucket(), + dependency.getModuleReference(), + ), + ) } return &moduleFileSet{ Module: module, - allReadBucket: storage.MultiReadBucket(readBuckets...), + allReadBucket: newMultiReadBucket(moduleReadBuckets...), } } -func (m *moduleFileSet) AllFileInfos(ctx context.Context) ([]bufcore.FileInfo, error) { - var fileInfos []bufcore.FileInfo - if err := m.allReadBucket.Walk(ctx, "", func(objectInfo storage.ObjectInfo) error { - // super overkill but ok +func (m *moduleFileSet) AllFileInfos(ctx context.Context) ([]FileInfo, error) { + var fileInfos []FileInfo + if err := m.allReadBucket.WalkModuleFiles(ctx, "", func(objectInfo ObjectInfo) error { if err := validateModuleFilePathWithoutNormalization(objectInfo.Path()); err != nil { return err } @@ -54,18 +71,18 @@ func (m *moduleFileSet) AllFileInfos(ctx context.Context) ([]bufcore.FileInfo, e if err != nil { return err } - fileInfos = append(fileInfos, bufcore.NewFileInfoForObjectInfo(objectInfo, !isNotImport)) + coreFileInfo := bufcore.NewFileInfoForObjectInfo(objectInfo, !isNotImport) + fileInfos = append(fileInfos, NewFileInfo(coreFileInfo, objectInfo.ModuleReference())) return nil }); err != nil { return nil, err } - bufcore.SortFileInfos(fileInfos) + sortFileInfos(fileInfos) return fileInfos, nil } func (m *moduleFileSet) GetModuleFile(ctx context.Context, path string) (ModuleFile, error) { - // super overkill but ok - if err := validateModuleFilePath(path); err != nil { + if err := ValidateModuleFilePath(path); err != nil { return nil, err } readObjectCloser, err := m.allReadBucket.Get(ctx, path) @@ -76,7 +93,12 @@ func (m *moduleFileSet) GetModuleFile(ctx context.Context, path string) (ModuleF if err != nil { return nil, err } - return newModuleFile(bufcore.NewFileInfoForObjectInfo(readObjectCloser, !isNotImport), readObjectCloser), nil + objectInfo, err := m.allReadBucket.StatModuleFile(ctx, path) + if err != nil { + return nil, err + } + coreFileInfo := bufcore.NewFileInfoForObjectInfo(readObjectCloser, !isNotImport) + return newModuleFile(NewFileInfo(coreFileInfo, objectInfo.ModuleReference()), readObjectCloser), nil } func (*moduleFileSet) isModuleFileSet() {} diff --git a/internal/buf/bufcore/bufmodule/targeting_module.go b/internal/buf/bufcore/bufmodule/targeting_module.go index 0de868e355..6c5fb8a190 100644 --- a/internal/buf/bufcore/bufmodule/targeting_module.go +++ b/internal/buf/bufcore/bufmodule/targeting_module.go @@ -51,13 +51,13 @@ func newTargetingModule( }, nil } -func (m *targetingModule) TargetFileInfos(ctx context.Context) (fileInfos []bufcore.FileInfo, retErr error) { +func (m *targetingModule) TargetFileInfos(ctx context.Context) (fileInfos []FileInfo, retErr error) { defer func() { if retErr == nil { if len(fileInfos) == 0 { retErr = internal.ErrNoTargetFiles } else { - bufcore.SortFileInfos(fileInfos) + sortFileInfos(fileInfos) } } }() @@ -87,7 +87,8 @@ func (m *targetingModule) TargetFileInfos(ctx context.Context) (fileInfos []bufc // add to the nonImportImageFiles if does not already exist if _, ok := fileInfoPaths[targetPath]; !ok { fileInfoPaths[targetPath] = struct{}{} - fileInfos = append(fileInfos, bufcore.NewFileInfoForObjectInfo(objectInfo, false)) + coreFileInfo := bufcore.NewFileInfoForObjectInfo(objectInfo, false) + fileInfos = append(fileInfos, NewFileInfo(coreFileInfo, m.Module.getModuleReference())) } } } @@ -127,7 +128,8 @@ func (m *targetingModule) TargetFileInfos(ctx context.Context) (fileInfos []bufc // then, add the file if it is not added if _, ok := fileInfoPaths[path]; !ok { fileInfoPaths[path] = struct{}{} - fileInfos = append(fileInfos, bufcore.NewFileInfoForObjectInfo(objectInfo, false)) + coreFileInfo := bufcore.NewFileInfoForObjectInfo(objectInfo, false) + fileInfos = append(fileInfos, NewFileInfo(coreFileInfo, m.Module.getModuleReference())) } } return nil diff --git a/internal/buf/bufcore/bufmodule/targeting_module_test.go b/internal/buf/bufcore/bufmodule/targeting_module_test.go index 402c0ae061..2826503993 100644 --- a/internal/buf/bufcore/bufmodule/targeting_module_test.go +++ b/internal/buf/bufcore/bufmodule/targeting_module_test.go @@ -18,9 +18,9 @@ import ( "context" "testing" - "github.com/bufbuild/buf/internal/buf/bufcore" "github.com/bufbuild/buf/internal/buf/bufcore/bufcoretesting" modulev1alpha1 "github.com/bufbuild/buf/internal/gen/proto/go/buf/alpha/module/v1alpha1" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -61,15 +61,15 @@ func TestTargetingModuleBasic(t *testing.T) { fileInfos, err := module.SourceFileInfos(ctx) require.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, - []bufcore.FileInfo{ - bufcoretesting.NewFileInfo(t, "a/a.proto", "a/a.proto", false), - bufcoretesting.NewFileInfo(t, "a/b.proto", "a/b.proto", false), - bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), - bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), - bufcoretesting.NewFileInfo(t, "c/c.proto/a.proto", "c/c.proto/a.proto", false), - bufcoretesting.NewFileInfo(t, "c/c.proto/b.proto", "c/c.proto/b.proto", false), + []FileInfo{ + NewFileInfo(bufcoretesting.NewFileInfo(t, "a/a.proto", "a/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "a/b.proto", "a/b.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "c/c.proto/a.proto", "c/c.proto/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "c/c.proto/b.proto", "c/c.proto/b.proto", false), nil), }, fileInfos, ) @@ -84,11 +84,11 @@ func TestTargetingModuleBasic(t *testing.T) { require.NoError(t, err) targetFileInfos, err := targetModule.TargetFileInfos(ctx) require.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, - []bufcore.FileInfo{ - bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), - bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), + []FileInfo{ + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), nil), }, targetFileInfos, ) @@ -102,11 +102,11 @@ func TestTargetingModuleBasic(t *testing.T) { require.NoError(t, err) targetFileInfos, err = targetModule.TargetFileInfos(ctx) require.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, - []bufcore.FileInfo{ - bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), - bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), + []FileInfo{ + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), nil), }, targetFileInfos, ) @@ -121,11 +121,11 @@ func TestTargetingModuleBasic(t *testing.T) { require.NoError(t, err) targetFileInfos, err = targetModule.TargetFileInfos(ctx) require.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, - []bufcore.FileInfo{ - bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), - bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), + []FileInfo{ + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), nil), }, targetFileInfos, ) @@ -140,13 +140,13 @@ func TestTargetingModuleBasic(t *testing.T) { require.NoError(t, err) targetFileInfos, err = targetModule.TargetFileInfos(ctx) require.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, - []bufcore.FileInfo{ - bufcoretesting.NewFileInfo(t, "a/a.proto", "a/a.proto", false), - bufcoretesting.NewFileInfo(t, "a/b.proto", "a/b.proto", false), - bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), - bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), + []FileInfo{ + NewFileInfo(bufcoretesting.NewFileInfo(t, "a/a.proto", "a/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "a/b.proto", "a/b.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), nil), }, targetFileInfos, ) @@ -174,11 +174,11 @@ func TestTargetingModuleBasic(t *testing.T) { require.NoError(t, err) targetFileInfos, err = targetModule.TargetFileInfos(ctx) require.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, - []bufcore.FileInfo{ - bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), - bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), + []FileInfo{ + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), nil), }, targetFileInfos, ) @@ -193,13 +193,13 @@ func TestTargetingModuleBasic(t *testing.T) { require.NoError(t, err) targetFileInfos, err = targetModule.TargetFileInfos(ctx) require.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, - []bufcore.FileInfo{ - bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), - bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), - bufcoretesting.NewFileInfo(t, "c/c.proto/a.proto", "c/c.proto/a.proto", false), - bufcoretesting.NewFileInfo(t, "c/c.proto/b.proto", "c/c.proto/b.proto", false), + []FileInfo{ + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/a.proto", "b/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "b/b.proto", "b/b.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "c/c.proto/a.proto", "c/c.proto/a.proto", false), nil), + NewFileInfo(bufcoretesting.NewFileInfo(t, "c/c.proto/b.proto", "c/c.proto/b.proto", false), nil), }, targetFileInfos, ) @@ -213,10 +213,10 @@ func TestTargetingModuleBasic(t *testing.T) { require.NoError(t, err) targetFileInfos, err = targetModule.TargetFileInfos(ctx) require.NoError(t, err) - bufcoretesting.AssertFileInfosEqual( + assert.Equal( t, - []bufcore.FileInfo{ - bufcoretesting.NewFileInfo(t, "c/c.proto/a.proto", "c/c.proto/a.proto", false), + []FileInfo{ + NewFileInfo(bufcoretesting.NewFileInfo(t, "c/c.proto/a.proto", "c/c.proto/a.proto", false), nil), }, targetFileInfos, ) diff --git a/internal/buf/bufcore/bufmodule/validate.go b/internal/buf/bufcore/bufmodule/validate.go index 8d89786424..58d7315781 100644 --- a/internal/buf/bufcore/bufmodule/validate.go +++ b/internal/buf/bufcore/bufmodule/validate.go @@ -55,7 +55,7 @@ func ValidateProtoModule(protoModule *modulev1alpha1.Module) error { totalContentLength := 0 filePathMap := make(map[string]struct{}, len(protoModule.Files)) for _, protoModuleFile := range protoModule.Files { - if err := validateModuleFilePath(protoModuleFile.Path); err != nil { + if err := ValidateModuleFilePath(protoModuleFile.Path); err != nil { return err } if _, ok := filePathMap[protoModuleFile.Path]; ok { @@ -287,7 +287,7 @@ func validateCreateTime(createTime *timestamppb.Timestamp) error { return createTime.CheckValid() } -func validateModuleFilePath(path string) error { +func ValidateModuleFilePath(path string) error { normalizedPath, err := normalpath.NormalizeAndValidate(path) if err != nil { return err diff --git a/internal/buf/bufwire/bufwire.go b/internal/buf/bufwire/bufwire.go index f3ba4c6d57..7d07e7398b 100644 --- a/internal/buf/bufwire/bufwire.go +++ b/internal/buf/bufwire/bufwire.go @@ -22,7 +22,6 @@ import ( "github.com/bufbuild/buf/internal/buf/bufanalysis" "github.com/bufbuild/buf/internal/buf/bufconfig" - "github.com/bufbuild/buf/internal/buf/bufcore" "github.com/bufbuild/buf/internal/buf/bufcore/bufimage" "github.com/bufbuild/buf/internal/buf/bufcore/bufimage/bufimagebuild" "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" @@ -135,7 +134,7 @@ type FileLister interface { container app.EnvStdinContainer, ref buffetch.Ref, configOverride string, - ) ([]bufcore.FileInfo, error) + ) ([]bufmodule.FileInfo, error) } // NewFileLister returns a new FileLister. diff --git a/internal/buf/bufwire/file_lister.go b/internal/buf/bufwire/file_lister.go index 87a3f60d7b..ccbb37c9cd 100644 --- a/internal/buf/bufwire/file_lister.go +++ b/internal/buf/bufwire/file_lister.go @@ -19,8 +19,8 @@ import ( "fmt" "github.com/bufbuild/buf/internal/buf/bufconfig" - "github.com/bufbuild/buf/internal/buf/bufcore" "github.com/bufbuild/buf/internal/buf/bufcore/bufimage/bufimagebuild" + "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule/bufmodulebuild" "github.com/bufbuild/buf/internal/buf/buffetch" "github.com/bufbuild/buf/internal/pkg/app" @@ -62,7 +62,7 @@ func (e *fileLister) ListFiles( container app.EnvStdinContainer, ref buffetch.Ref, configOverride string, -) (_ []bufcore.FileInfo, retErr error) { +) (_ []bufmodule.FileInfo, retErr error) { switch t := ref.(type) { case buffetch.ImageRef: // if we have an image, list the files in the image @@ -78,7 +78,7 @@ func (e *fileLister) ListFiles( return nil, err } files := image.Files() - fileInfos := make([]bufcore.FileInfo, len(files)) + fileInfos := make([]bufmodule.FileInfo, len(files)) for i, file := range files { fileInfos[i] = file } diff --git a/internal/buf/cmd/buf/command/beta/push/push.go b/internal/buf/cmd/buf/command/beta/push/push.go index 9cbeb28f4a..d4cd760f51 100644 --- a/internal/buf/cmd/buf/command/beta/push/push.go +++ b/internal/buf/cmd/buf/command/beta/push/push.go @@ -23,6 +23,7 @@ import ( "github.com/bufbuild/buf/internal/buf/bufcore/bufmodule" "github.com/bufbuild/buf/internal/pkg/app/appcmd" "github.com/bufbuild/buf/internal/pkg/app/appflag" + "github.com/bufbuild/buf/internal/pkg/rpc" "github.com/bufbuild/buf/internal/pkg/storage/storageos" "github.com/bufbuild/buf/internal/pkg/stringutil" "github.com/spf13/cobra" @@ -60,6 +61,7 @@ func NewCommand( type flags struct { Branch string ErrorFormat string + Force bool // special InputHashtag string } @@ -131,6 +133,17 @@ func run( protoModule, ) if err != nil { + if rpc.GetErrorCode(err) == rpc.ErrorCodeAlreadyExists && !flags.Force { + if _, err := container.Stderr().Write( + []byte(fmt.Sprintf( + "The latest commit on branch %q has the same content, not creating a new commit.\n", + flags.Branch, + )), + ); err != nil { + return err + } + return nil + } return err } if _, err := container.Stdout().Write([]byte(localModulePin.Commit + "\n")); err != nil { diff --git a/internal/buf/cmd/protoc-gen-buf-lint/lint_test.go b/internal/buf/cmd/protoc-gen-buf-lint/lint_test.go index 0c2c98b677..509a7305bc 100644 --- a/internal/buf/cmd/protoc-gen-buf-lint/lint_test.go +++ b/internal/buf/cmd/protoc-gen-buf-lint/lint_test.go @@ -236,6 +236,7 @@ func testBuildCodeGeneratorRequest( _, isNotImport := nonImportRootRelFilePaths[fileDescriptorProto.GetName()] imageFile, err := bufimage.NewImageFile( fileDescriptorProto, + nil, "", !isNotImport, ) diff --git a/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/organization.pb.go b/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/organization.pb.go index d21447003e..7899236d95 100644 --- a/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/organization.pb.go +++ b/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/organization.pb.go @@ -22,7 +22,6 @@ import ( ) // OrganizationService is the Organization service. -// All methods on the Organization service require authentication. type OrganizationService interface { // GetOrganization gets a organization by ID. GetOrganization(ctx context.Context, id string) (organization *v1alpha1.Organization, err error) diff --git a/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository.pb.go b/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository.pb.go index 446eba2f0e..7d0a764c5c 100644 --- a/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository.pb.go +++ b/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository.pb.go @@ -22,7 +22,6 @@ import ( ) // RepositoryService is the Repository service. -// All methods on the Repository service require authentication. type RepositoryService interface { // GetRepository gets a repository by ID. GetRepository(ctx context.Context, id string) (repository *v1alpha1.Repository, err error) diff --git a/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository_branch.pb.go b/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository_branch.pb.go index db7750300f..6c309471ac 100644 --- a/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository_branch.pb.go +++ b/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository_branch.pb.go @@ -22,7 +22,6 @@ import ( ) // RepositoryBranchService is the Repository branch service. -// All methods on the Repository branch service require authentication. type RepositoryBranchService interface { // CreateRepositoryBranch creates a new repository branch. CreateRepositoryBranch( diff --git a/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository_commit.pb.go b/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository_commit.pb.go index 2b26e86e29..482afbefa2 100644 --- a/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository_commit.pb.go +++ b/internal/gen/proto/api/buf/alpha/registry/v1alpha1/registryv1alpha1api/repository_commit.pb.go @@ -22,7 +22,6 @@ import ( ) // RepositoryCommitService is the Repository commit service. -// All methods on the Repository commit service require authentication. type RepositoryCommitService interface { // ListRepositoryCommits lists the repository commits associated with a repository branch. ListRepositoryCommits( diff --git a/internal/gen/proto/apiclient/buf/alpha/registry/v1alpha1/registryv1alpha1apiclient/registryv1alpha1apiclient.pb.go b/internal/gen/proto/apiclient/buf/alpha/registry/v1alpha1/registryv1alpha1apiclient/registryv1alpha1apiclient.pb.go index 1b60eec14f..cf4fdb8bcb 100644 --- a/internal/gen/proto/apiclient/buf/alpha/registry/v1alpha1/registryv1alpha1apiclient/registryv1alpha1apiclient.pb.go +++ b/internal/gen/proto/apiclient/buf/alpha/registry/v1alpha1/registryv1alpha1apiclient/registryv1alpha1apiclient.pb.go @@ -29,6 +29,7 @@ type Provider interface { RepositoryBranchServiceProvider RepositoryCommitServiceProvider RepositoryServiceProvider + RepositoryTagServiceProvider ResolveServiceProvider UserServiceProvider } @@ -63,6 +64,11 @@ type RepositoryServiceProvider interface { NewRepositoryService(ctx context.Context, address string) (registryv1alpha1api.RepositoryService, error) } +// RepositoryTagServiceProvider provides a client-side RepositoryTagService for an address. +type RepositoryTagServiceProvider interface { + NewRepositoryTagService(ctx context.Context, address string) (registryv1alpha1api.RepositoryTagService, error) +} + // ResolveServiceProvider provides a client-side ResolveService for an address. type ResolveServiceProvider interface { NewResolveService(ctx context.Context, address string) (registryv1alpha1api.ResolveService, error) diff --git a/internal/gen/proto/apiclientgrpc/buf/alpha/registry/v1alpha1/registryv1alpha1apiclientgrpc/registryv1alpha1apiclientgrpc.pb.go b/internal/gen/proto/apiclientgrpc/buf/alpha/registry/v1alpha1/registryv1alpha1apiclientgrpc/registryv1alpha1apiclientgrpc.pb.go index 1bfe090f4f..b228c027f4 100644 --- a/internal/gen/proto/apiclientgrpc/buf/alpha/registry/v1alpha1/registryv1alpha1apiclientgrpc/registryv1alpha1apiclientgrpc.pb.go +++ b/internal/gen/proto/apiclientgrpc/buf/alpha/registry/v1alpha1/registryv1alpha1apiclientgrpc/registryv1alpha1apiclientgrpc.pb.go @@ -204,6 +204,29 @@ func (p *provider) NewRepositoryService(ctx context.Context, address string) (re }, nil } +func (p *provider) NewRepositoryTagService(ctx context.Context, address string) (registryv1alpha1api.RepositoryTagService, error) { + var contextModifier func(context.Context) context.Context + var err error + if p.contextModifierProvider != nil { + contextModifier, err = p.contextModifierProvider(address) + if err != nil { + return nil, err + } + } + if p.addressMapper != nil { + address = p.addressMapper(address) + } + clientConn, err := p.clientConnProvider.NewClientConn(ctx, address) + if err != nil { + return nil, err + } + return &repositoryTagService{ + logger: p.logger, + client: v1alpha1.NewRepositoryTagServiceClient(clientConn), + contextModifier: contextModifier, + }, nil +} + func (p *provider) NewResolveService(ctx context.Context, address string) (registryv1alpha1api.ResolveService, error) { var contextModifier func(context.Context) context.Context var err error diff --git a/internal/gen/proto/apiclienttwirp/buf/alpha/registry/v1alpha1/registryv1alpha1apiclienttwirp/registryv1alpha1apiclienttwirp.pb.go b/internal/gen/proto/apiclienttwirp/buf/alpha/registry/v1alpha1/registryv1alpha1apiclienttwirp/registryv1alpha1apiclienttwirp.pb.go index 359f53b8c4..670fe8f0f9 100644 --- a/internal/gen/proto/apiclienttwirp/buf/alpha/registry/v1alpha1/registryv1alpha1apiclienttwirp/registryv1alpha1apiclienttwirp.pb.go +++ b/internal/gen/proto/apiclienttwirp/buf/alpha/registry/v1alpha1/registryv1alpha1apiclienttwirp/registryv1alpha1apiclienttwirp.pb.go @@ -205,6 +205,29 @@ func (p *provider) NewRepositoryService(ctx context.Context, address string) (re }, nil } +func (p *provider) NewRepositoryTagService(ctx context.Context, address string) (registryv1alpha1api.RepositoryTagService, error) { + var contextModifier func(context.Context) context.Context + var err error + if p.contextModifierProvider != nil { + contextModifier, err = p.contextModifierProvider(address) + if err != nil { + return nil, err + } + } + if p.addressMapper != nil { + address = p.addressMapper(address) + } + return &repositoryTagService{ + logger: p.logger, + client: v1alpha1.NewRepositoryTagServiceProtobufClient( + p.httpClient.ParseAddress(address), + p.httpClient, + twirpclient.NewClientOptions()..., + ), + contextModifier: contextModifier, + }, nil +} + func (p *provider) NewResolveService(ctx context.Context, address string) (registryv1alpha1api.ResolveService, error) { var contextModifier func(context.Context) context.Context var err error diff --git a/internal/gen/proto/go/buf/alpha/image/v1/image.pb.go b/internal/gen/proto/go/buf/alpha/image/v1/image.pb.go index 824e89048c..3420da7742 100644 --- a/internal/gen/proto/go/buf/alpha/image/v1/image.pb.go +++ b/internal/gen/proto/go/buf/alpha/image/v1/image.pb.go @@ -114,6 +114,11 @@ type ImageExtension struct { // A given FileDescriptorProto may or may not be an import depending on // the image context, so this information is not stored on each FileDescriptorProto. ImageImportRefs []*ImageImportRef `protobuf:"bytes,1,rep,name=image_import_refs,json=imageImportRefs" json:"image_import_refs,omitempty"` + // ModuleReferenceRefs are the module reference references for this specific Image. + // + // The lack of a ModuleReferenceRef for a given file means that the module + // reference information is not known, and must be treated as unknown. + ModuleReferenceRefs []*ModuleReferenceRef `protobuf:"bytes,2,rep,name=module_reference_refs,json=moduleReferenceRefs" json:"module_reference_refs,omitempty"` } func (x *ImageExtension) Reset() { @@ -155,6 +160,13 @@ func (x *ImageExtension) GetImageImportRefs() []*ImageImportRef { return nil } +func (x *ImageExtension) GetModuleReferenceRefs() []*ModuleReferenceRef { + if x != nil { + return x.ModuleReferenceRefs + } + return nil +} + // ImageImportRef is a reference to an image import. // // This is a message type instead of a scalar type so that we can add @@ -211,6 +223,95 @@ func (x *ImageImportRef) GetFileIndex() uint32 { return 0 } +// ModuleReferenceRef is a reference to a module reference. +type ModuleReferenceRef struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + // file_index is the index within the Image file array. + // + // This signifies that file[file_index] is the file that + // uses this module reference. + // + // This field must be set. + FileIndex *uint32 `protobuf:"varint,1,opt,name=file_index,json=fileIndex" json:"file_index,omitempty"` + Remote *string `protobuf:"bytes,2,opt,name=remote" json:"remote,omitempty"` + Owner *string `protobuf:"bytes,3,opt,name=owner" json:"owner,omitempty"` + Repository *string `protobuf:"bytes,4,opt,name=repository" json:"repository,omitempty"` + // This is always a commit for now, but we make this a reference instead of specifically a + // commit to give ourselves options in the future. We can easily distinguish between + // branches/tags and commits. + Reference *string `protobuf:"bytes,5,opt,name=reference" json:"reference,omitempty"` +} + +func (x *ModuleReferenceRef) Reset() { + *x = ModuleReferenceRef{} + if protoimpl.UnsafeEnabled { + mi := &file_buf_alpha_image_v1_image_proto_msgTypes[3] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *ModuleReferenceRef) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ModuleReferenceRef) ProtoMessage() {} + +func (x *ModuleReferenceRef) ProtoReflect() protoreflect.Message { + mi := &file_buf_alpha_image_v1_image_proto_msgTypes[3] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use ModuleReferenceRef.ProtoReflect.Descriptor instead. +func (*ModuleReferenceRef) Descriptor() ([]byte, []int) { + return file_buf_alpha_image_v1_image_proto_rawDescGZIP(), []int{3} +} + +func (x *ModuleReferenceRef) GetFileIndex() uint32 { + if x != nil && x.FileIndex != nil { + return *x.FileIndex + } + return 0 +} + +func (x *ModuleReferenceRef) GetRemote() string { + if x != nil && x.Remote != nil { + return *x.Remote + } + return "" +} + +func (x *ModuleReferenceRef) GetOwner() string { + if x != nil && x.Owner != nil { + return *x.Owner + } + return "" +} + +func (x *ModuleReferenceRef) GetRepository() string { + if x != nil && x.Repository != nil { + return *x.Repository + } + return "" +} + +func (x *ModuleReferenceRef) GetReference() string { + if x != nil && x.Reference != nil { + return *x.Reference + } + return "" +} + var File_buf_alpha_image_v1_image_proto protoreflect.FileDescriptor var file_buf_alpha_image_v1_image_proto_rawDesc = []byte{ @@ -229,21 +330,37 @@ var file_buf_alpha_image_v1_image_proto_rawDesc = []byte{ 0x62, 0x75, 0x66, 0x2e, 0x61, 0x6c, 0x70, 0x68, 0x61, 0x2e, 0x69, 0x6d, 0x61, 0x67, 0x65, 0x2e, 0x76, 0x31, 0x2e, 0x49, 0x6d, 0x61, 0x67, 0x65, 0x45, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x52, 0x16, 0x62, 0x75, 0x66, 0x62, 0x75, 0x69, 0x6c, 0x64, 0x49, 0x6d, 0x61, 0x67, 0x65, - 0x45, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x22, 0x60, 0x0a, 0x0e, 0x49, 0x6d, 0x61, - 0x67, 0x65, 0x45, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x12, 0x4e, 0x0a, 0x11, 0x69, - 0x6d, 0x61, 0x67, 0x65, 0x5f, 0x69, 0x6d, 0x70, 0x6f, 0x72, 0x74, 0x5f, 0x72, 0x65, 0x66, 0x73, - 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x22, 0x2e, 0x62, 0x75, 0x66, 0x2e, 0x61, 0x6c, 0x70, - 0x68, 0x61, 0x2e, 0x69, 0x6d, 0x61, 0x67, 0x65, 0x2e, 0x76, 0x31, 0x2e, 0x49, 0x6d, 0x61, 0x67, - 0x65, 0x49, 0x6d, 0x70, 0x6f, 0x72, 0x74, 0x52, 0x65, 0x66, 0x52, 0x0f, 0x69, 0x6d, 0x61, 0x67, - 0x65, 0x49, 0x6d, 0x70, 0x6f, 0x72, 0x74, 0x52, 0x65, 0x66, 0x73, 0x22, 0x2f, 0x0a, 0x0e, 0x49, - 0x6d, 0x61, 0x67, 0x65, 0x49, 0x6d, 0x70, 0x6f, 0x72, 0x74, 0x52, 0x65, 0x66, 0x12, 0x1d, 0x0a, - 0x0a, 0x66, 0x69, 0x6c, 0x65, 0x5f, 0x69, 0x6e, 0x64, 0x65, 0x78, 0x18, 0x01, 0x20, 0x01, 0x28, - 0x0d, 0x52, 0x09, 0x66, 0x69, 0x6c, 0x65, 0x49, 0x6e, 0x64, 0x65, 0x78, 0x42, 0x4f, 0x48, 0x01, - 0x5a, 0x48, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x62, 0x75, 0x66, - 0x62, 0x75, 0x69, 0x6c, 0x64, 0x2f, 0x62, 0x75, 0x66, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, - 0x61, 0x6c, 0x2f, 0x67, 0x65, 0x6e, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x67, 0x6f, 0x2f, - 0x62, 0x75, 0x66, 0x2f, 0x61, 0x6c, 0x70, 0x68, 0x61, 0x2f, 0x69, 0x6d, 0x61, 0x67, 0x65, 0x2f, - 0x76, 0x31, 0x3b, 0x69, 0x6d, 0x61, 0x67, 0x65, 0x76, 0x31, 0xf8, 0x01, 0x01, + 0x45, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x22, 0xbc, 0x01, 0x0a, 0x0e, 0x49, 0x6d, + 0x61, 0x67, 0x65, 0x45, 0x78, 0x74, 0x65, 0x6e, 0x73, 0x69, 0x6f, 0x6e, 0x12, 0x4e, 0x0a, 0x11, + 0x69, 0x6d, 0x61, 0x67, 0x65, 0x5f, 0x69, 0x6d, 0x70, 0x6f, 0x72, 0x74, 0x5f, 0x72, 0x65, 0x66, + 0x73, 0x18, 0x01, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x22, 0x2e, 0x62, 0x75, 0x66, 0x2e, 0x61, 0x6c, + 0x70, 0x68, 0x61, 0x2e, 0x69, 0x6d, 0x61, 0x67, 0x65, 0x2e, 0x76, 0x31, 0x2e, 0x49, 0x6d, 0x61, + 0x67, 0x65, 0x49, 0x6d, 0x70, 0x6f, 0x72, 0x74, 0x52, 0x65, 0x66, 0x52, 0x0f, 0x69, 0x6d, 0x61, + 0x67, 0x65, 0x49, 0x6d, 0x70, 0x6f, 0x72, 0x74, 0x52, 0x65, 0x66, 0x73, 0x12, 0x5a, 0x0a, 0x15, + 0x6d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x5f, 0x72, 0x65, 0x66, 0x65, 0x72, 0x65, 0x6e, 0x63, 0x65, + 0x5f, 0x72, 0x65, 0x66, 0x73, 0x18, 0x02, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x26, 0x2e, 0x62, 0x75, + 0x66, 0x2e, 0x61, 0x6c, 0x70, 0x68, 0x61, 0x2e, 0x69, 0x6d, 0x61, 0x67, 0x65, 0x2e, 0x76, 0x31, + 0x2e, 0x4d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x52, 0x65, 0x66, 0x65, 0x72, 0x65, 0x6e, 0x63, 0x65, + 0x52, 0x65, 0x66, 0x52, 0x13, 0x6d, 0x6f, 0x64, 0x75, 0x6c, 0x65, 0x52, 0x65, 0x66, 0x65, 0x72, + 0x65, 0x6e, 0x63, 0x65, 0x52, 0x65, 0x66, 0x73, 0x22, 0x2f, 0x0a, 0x0e, 0x49, 0x6d, 0x61, 0x67, + 0x65, 0x49, 0x6d, 0x70, 0x6f, 0x72, 0x74, 0x52, 0x65, 0x66, 0x12, 0x1d, 0x0a, 0x0a, 0x66, 0x69, + 0x6c, 0x65, 0x5f, 0x69, 0x6e, 0x64, 0x65, 0x78, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x09, + 0x66, 0x69, 0x6c, 0x65, 0x49, 0x6e, 0x64, 0x65, 0x78, 0x22, 0x9f, 0x01, 0x0a, 0x12, 0x4d, 0x6f, + 0x64, 0x75, 0x6c, 0x65, 0x52, 0x65, 0x66, 0x65, 0x72, 0x65, 0x6e, 0x63, 0x65, 0x52, 0x65, 0x66, + 0x12, 0x1d, 0x0a, 0x0a, 0x66, 0x69, 0x6c, 0x65, 0x5f, 0x69, 0x6e, 0x64, 0x65, 0x78, 0x18, 0x01, + 0x20, 0x01, 0x28, 0x0d, 0x52, 0x09, 0x66, 0x69, 0x6c, 0x65, 0x49, 0x6e, 0x64, 0x65, 0x78, 0x12, + 0x16, 0x0a, 0x06, 0x72, 0x65, 0x6d, 0x6f, 0x74, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, + 0x06, 0x72, 0x65, 0x6d, 0x6f, 0x74, 0x65, 0x12, 0x14, 0x0a, 0x05, 0x6f, 0x77, 0x6e, 0x65, 0x72, + 0x18, 0x03, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x6f, 0x77, 0x6e, 0x65, 0x72, 0x12, 0x1e, 0x0a, + 0x0a, 0x72, 0x65, 0x70, 0x6f, 0x73, 0x69, 0x74, 0x6f, 0x72, 0x79, 0x18, 0x04, 0x20, 0x01, 0x28, + 0x09, 0x52, 0x0a, 0x72, 0x65, 0x70, 0x6f, 0x73, 0x69, 0x74, 0x6f, 0x72, 0x79, 0x12, 0x1c, 0x0a, + 0x09, 0x72, 0x65, 0x66, 0x65, 0x72, 0x65, 0x6e, 0x63, 0x65, 0x18, 0x05, 0x20, 0x01, 0x28, 0x09, + 0x52, 0x09, 0x72, 0x65, 0x66, 0x65, 0x72, 0x65, 0x6e, 0x63, 0x65, 0x42, 0x4f, 0x48, 0x01, 0x5a, + 0x48, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x62, 0x75, 0x66, 0x62, + 0x75, 0x69, 0x6c, 0x64, 0x2f, 0x62, 0x75, 0x66, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, 0x61, + 0x6c, 0x2f, 0x67, 0x65, 0x6e, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x2f, 0x67, 0x6f, 0x2f, 0x62, + 0x75, 0x66, 0x2f, 0x61, 0x6c, 0x70, 0x68, 0x61, 0x2f, 0x69, 0x6d, 0x61, 0x67, 0x65, 0x2f, 0x76, + 0x31, 0x3b, 0x69, 0x6d, 0x61, 0x67, 0x65, 0x76, 0x31, 0xf8, 0x01, 0x01, } var ( @@ -258,22 +375,24 @@ func file_buf_alpha_image_v1_image_proto_rawDescGZIP() []byte { return file_buf_alpha_image_v1_image_proto_rawDescData } -var file_buf_alpha_image_v1_image_proto_msgTypes = make([]protoimpl.MessageInfo, 3) +var file_buf_alpha_image_v1_image_proto_msgTypes = make([]protoimpl.MessageInfo, 4) var file_buf_alpha_image_v1_image_proto_goTypes = []interface{}{ (*Image)(nil), // 0: buf.alpha.image.v1.Image (*ImageExtension)(nil), // 1: buf.alpha.image.v1.ImageExtension (*ImageImportRef)(nil), // 2: buf.alpha.image.v1.ImageImportRef - (*descriptorpb.FileDescriptorProto)(nil), // 3: google.protobuf.FileDescriptorProto + (*ModuleReferenceRef)(nil), // 3: buf.alpha.image.v1.ModuleReferenceRef + (*descriptorpb.FileDescriptorProto)(nil), // 4: google.protobuf.FileDescriptorProto } var file_buf_alpha_image_v1_image_proto_depIdxs = []int32{ - 3, // 0: buf.alpha.image.v1.Image.file:type_name -> google.protobuf.FileDescriptorProto + 4, // 0: buf.alpha.image.v1.Image.file:type_name -> google.protobuf.FileDescriptorProto 1, // 1: buf.alpha.image.v1.Image.bufbuild_image_extension:type_name -> buf.alpha.image.v1.ImageExtension 2, // 2: buf.alpha.image.v1.ImageExtension.image_import_refs:type_name -> buf.alpha.image.v1.ImageImportRef - 3, // [3:3] is the sub-list for method output_type - 3, // [3:3] is the sub-list for method input_type - 3, // [3:3] is the sub-list for extension type_name - 3, // [3:3] is the sub-list for extension extendee - 0, // [0:3] is the sub-list for field type_name + 3, // 3: buf.alpha.image.v1.ImageExtension.module_reference_refs:type_name -> buf.alpha.image.v1.ModuleReferenceRef + 4, // [4:4] is the sub-list for method output_type + 4, // [4:4] is the sub-list for method input_type + 4, // [4:4] is the sub-list for extension type_name + 4, // [4:4] is the sub-list for extension extendee + 0, // [0:4] is the sub-list for field type_name } func init() { file_buf_alpha_image_v1_image_proto_init() } @@ -318,6 +437,18 @@ func file_buf_alpha_image_v1_image_proto_init() { return nil } } + file_buf_alpha_image_v1_image_proto_msgTypes[3].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*ModuleReferenceRef); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } } type x struct{} out := protoimpl.TypeBuilder{ @@ -325,7 +456,7 @@ func file_buf_alpha_image_v1_image_proto_init() { GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: file_buf_alpha_image_v1_image_proto_rawDesc, NumEnums: 0, - NumMessages: 3, + NumMessages: 4, NumExtensions: 0, NumServices: 0, }, diff --git a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/download.twirp.go b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/download.twirp.go index 2db2858d6f..81697ad6db 100644 --- a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/download.twirp.go +++ b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/download.twirp.go @@ -28,6 +28,7 @@ It is generated from these files: buf/alpha/registry/v1alpha1/repository.proto buf/alpha/registry/v1alpha1/repository_branch.proto buf/alpha/registry/v1alpha1/repository_commit.proto + buf/alpha/registry/v1alpha1/repository_tag.proto buf/alpha/registry/v1alpha1/resolve.proto buf/alpha/registry/v1alpha1/user.proto */ diff --git a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/organization.twirp.go b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/organization.twirp.go index 74301471ba..51ad18821a 100644 --- a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/organization.twirp.go +++ b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/organization.twirp.go @@ -41,7 +41,6 @@ const _ = twirp.TwirpPackageIsVersion7 // ============================= // OrganizationService is the Organization service. -// All methods on the Organization service require authentication. type OrganizationService interface { // GetOrganization gets a organization by ID. GetOrganization(context.Context, *GetOrganizationRequest) (*GetOrganizationResponse, error) diff --git a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository.twirp.go b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository.twirp.go index 488a761e7a..12353fb0ef 100644 --- a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository.twirp.go +++ b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository.twirp.go @@ -41,7 +41,6 @@ const _ = twirp.TwirpPackageIsVersion7 // =========================== // RepositoryService is the Repository service. -// All methods on the Repository service require authentication. type RepositoryService interface { // GetRepository gets a repository by ID. GetRepository(context.Context, *GetRepositoryRequest) (*GetRepositoryResponse, error) diff --git a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository_branch.twirp.go b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository_branch.twirp.go index e773fdc147..3f55f390d8 100644 --- a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository_branch.twirp.go +++ b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository_branch.twirp.go @@ -41,7 +41,6 @@ const _ = twirp.TwirpPackageIsVersion7 // ================================= // RepositoryBranchService is the Repository branch service. -// All methods on the Repository branch service require authentication. type RepositoryBranchService interface { // CreateRepositoryBranch creates a new repository branch. CreateRepositoryBranch(context.Context, *CreateRepositoryBranchRequest) (*CreateRepositoryBranchResponse, error) diff --git a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository_commit.twirp.go b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository_commit.twirp.go index 4557b4b950..e0a011e16e 100644 --- a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository_commit.twirp.go +++ b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/repository_commit.twirp.go @@ -41,7 +41,6 @@ const _ = twirp.TwirpPackageIsVersion7 // ================================= // RepositoryCommitService is the Repository commit service. -// All methods on the Repository commit service require authentication. type RepositoryCommitService interface { // ListRepositoryCommits lists the repository commits associated with a repository branch. ListRepositoryCommits(context.Context, *ListRepositoryCommitsRequest) (*ListRepositoryCommitsResponse, error) diff --git a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/resolve.twirp.go b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/resolve.twirp.go index e35a4efdbe..0a8669761f 100644 --- a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/resolve.twirp.go +++ b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/resolve.twirp.go @@ -492,7 +492,7 @@ func (s *resolveServiceServer) serveGetModulePinsProtobuf(ctx context.Context, r } func (s *resolveServiceServer) ServiceDescriptor() ([]byte, int) { - return twirpFileDescriptor6, 0 + return twirpFileDescriptor7, 0 } func (s *resolveServiceServer) ProtocGenTwirpVersion() string { @@ -506,7 +506,7 @@ func (s *resolveServiceServer) PathPrefix() string { return baseServicePath(s.pathPrefix, "buf.alpha.registry.v1alpha1", "ResolveService") } -var twirpFileDescriptor6 = []byte{ +var twirpFileDescriptor7 = []byte{ // 303 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x91, 0x4f, 0x4b, 0xc4, 0x30, 0x10, 0xc5, 0x29, 0x8a, 0x87, 0x2c, 0x8a, 0x16, 0x05, 0xa9, 0x97, 0xa5, 0x88, 0xac, 0x1e, 0x12, diff --git a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/user.twirp.go b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/user.twirp.go index a9b2db4be3..724d2a62f0 100644 --- a/internal/gen/proto/go/buf/alpha/registry/v1alpha1/user.twirp.go +++ b/internal/gen/proto/go/buf/alpha/registry/v1alpha1/user.twirp.go @@ -4334,7 +4334,7 @@ func (s *userServiceServer) serveRemoveUserServerScopeByNameProtobuf(ctx context } func (s *userServiceServer) ServiceDescriptor() ([]byte, int) { - return twirpFileDescriptor7, 0 + return twirpFileDescriptor8, 0 } func (s *userServiceServer) ProtocGenTwirpVersion() string { @@ -4348,7 +4348,7 @@ func (s *userServiceServer) PathPrefix() string { return baseServicePath(s.pathPrefix, "buf.alpha.registry.v1alpha1", "UserService") } -var twirpFileDescriptor7 = []byte{ +var twirpFileDescriptor8 = []byte{ // 1062 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xd4, 0x58, 0xdf, 0x6f, 0xdb, 0x54, 0x14, 0xd6, 0x4d, 0x03, 0x6b, 0x4f, 0xb6, 0x76, 0xb9, 0x80, 0x94, 0xba, 0x2d, 0x4d, 0x8d, 0xd6, diff --git a/internal/pkg/app/appproto/appprotoexec/binary_handler.go b/internal/pkg/app/appproto/appprotoexec/binary_handler.go index bb655c7029..126acb9431 100644 --- a/internal/pkg/app/appproto/appprotoexec/binary_handler.go +++ b/internal/pkg/app/appproto/appprotoexec/binary_handler.go @@ -98,8 +98,10 @@ func (h *binaryHandler) Handle( return err } } - // call AddError even if this is empty - if response.Error != nil { + // plugin.proto specifies that only non-empty errors are considered errors. + // This is also consistent with protoc's behaviour. + // Ref: https://github.com/protocolbuffers/protobuf/blob/069f989b483e63005f87ab309de130677718bbec/src/google/protobuf/compiler/plugin.proto#L100-L108. + if response.GetError() != "" { responseWriter.AddError(response.GetError()) } return nil diff --git a/internal/pkg/protogenutil/protogenutil.go b/internal/pkg/protogenutil/protogenutil.go index 83d4f091ae..6d8f1811b0 100644 --- a/internal/pkg/protogenutil/protogenutil.go +++ b/internal/pkg/protogenutil/protogenutil.go @@ -58,7 +58,10 @@ func NewHandler(f func(*protogen.Plugin) error, options ...HandlerOption) apppro return err } } - if response.Error != nil { + // plugin.proto specifies that only non-empty errors are considered errors. + // This is also consistent with protoc's behaviour. + // Ref: https://github.com/protocolbuffers/protobuf/blob/069f989b483e63005f87ab309de130677718bbec/src/google/protobuf/compiler/plugin.proto#L100-L108. + if response.GetError() != "" { responseWriter.AddError(response.GetError()) } responseWriter.SetFeatureProto3Optional() diff --git a/make/buf/all.mk b/make/buf/all.mk index 96adde74a7..64ebbb261a 100644 --- a/make/buf/all.mk +++ b/make/buf/all.mk @@ -1,6 +1,7 @@ BUF_BIN ?= cmd/buf -PROTOREFLECT_VERSION := v1.8.2 +# Remove when protoreflect has a new release > 1.8.2 +PROTOREFLECT_VERSION := 8255811fc3c054aab548f7208e1471b668f4c5b3 # Remove when https://github.com/spf13/cobra/pull/1070 is released COBRA_VERSION := b97b5ead31f7d34f764ac8666e40c214bb8e06dc GO_GET_PKGS := $(GO_GET_PKGS) \ diff --git a/proto/buf/alpha/image/v1/image.proto b/proto/buf/alpha/image/v1/image.proto index 0691618d82..3af2a2ab2e 100644 --- a/proto/buf/alpha/image/v1/image.proto +++ b/proto/buf/alpha/image/v1/image.proto @@ -48,6 +48,12 @@ message ImageExtension { // A given FileDescriptorProto may or may not be an import depending on // the image context, so this information is not stored on each FileDescriptorProto. repeated ImageImportRef image_import_refs = 1; + + // ModuleReferenceRefs are the module reference references for this specific Image. + // + // The lack of a ModuleReferenceRef for a given file means that the module + // reference information is not known, and must be treated as unknown. + repeated ModuleReferenceRef module_reference_refs = 2; } // ImageImportRef is a reference to an image import. @@ -62,3 +68,21 @@ message ImageImportRef { // This field must be set. optional uint32 file_index = 1; } + +// ModuleReferenceRef is a reference to a module reference. +message ModuleReferenceRef { + // file_index is the index within the Image file array. + // + // This signifies that file[file_index] is the file that + // uses this module reference. + // + // This field must be set. + optional uint32 file_index = 1; + optional string remote = 2; + optional string owner = 3; + optional string repository = 4; + // This is always a commit for now, but we make this a reference instead of specifically a + // commit to give ourselves options in the future. We can easily distinguish between + // branches/tags and commits. + optional string reference = 5; +} diff --git a/proto/buf/alpha/registry/v1alpha1/organization.proto b/proto/buf/alpha/registry/v1alpha1/organization.proto index 8608c621a1..86a9baa6ea 100644 --- a/proto/buf/alpha/registry/v1alpha1/organization.proto +++ b/proto/buf/alpha/registry/v1alpha1/organization.proto @@ -34,7 +34,6 @@ message Organization { } // OrganizationService is the Organization service. -// All methods on the Organization service require authentication. service OrganizationService { // GetOrganization gets a organization by ID. rpc GetOrganization(GetOrganizationRequest) returns (GetOrganizationResponse) { diff --git a/proto/buf/alpha/registry/v1alpha1/repository.proto b/proto/buf/alpha/registry/v1alpha1/repository.proto index bb7e424f8d..ba02c0a367 100644 --- a/proto/buf/alpha/registry/v1alpha1/repository.proto +++ b/proto/buf/alpha/registry/v1alpha1/repository.proto @@ -46,7 +46,6 @@ message Repository { } // RepositoryService is the Repository service. -// All methods on the Repository service require authentication. service RepositoryService { // GetRepository gets a repository by ID. rpc GetRepository(GetRepositoryRequest) returns (GetRepositoryResponse) { diff --git a/proto/buf/alpha/registry/v1alpha1/repository_branch.proto b/proto/buf/alpha/registry/v1alpha1/repository_branch.proto index d7b4bf9f6d..ab3e537b15 100644 --- a/proto/buf/alpha/registry/v1alpha1/repository_branch.proto +++ b/proto/buf/alpha/registry/v1alpha1/repository_branch.proto @@ -35,7 +35,6 @@ message RepositoryBranch { } // RepositoryBranchService is the Repository branch service. -// All methods on the Repository branch service require authentication. service RepositoryBranchService { // CreateRepositoryBranch creates a new repository branch. rpc CreateRepositoryBranch(CreateRepositoryBranchRequest) returns (CreateRepositoryBranchResponse) { diff --git a/proto/buf/alpha/registry/v1alpha1/repository_commit.proto b/proto/buf/alpha/registry/v1alpha1/repository_commit.proto index c43c46a1fc..4aae1c4250 100644 --- a/proto/buf/alpha/registry/v1alpha1/repository_commit.proto +++ b/proto/buf/alpha/registry/v1alpha1/repository_commit.proto @@ -35,7 +35,6 @@ message RepositoryCommit { } // RepositoryCommitService is the Repository commit service. -// All methods on the Repository commit service require authentication. service RepositoryCommitService { // ListRepositoryCommits lists the repository commits associated with a repository branch. rpc ListRepositoryCommits(ListRepositoryCommitsRequest) returns (ListRepositoryCommitsResponse) {