Marshal/unmarshal custom options in JSON and add image convert command (

#87)

go.mod

@@ -15,7 +15,7 @@ require (
 	github.com/stretchr/testify v1.6.1
 	go.uber.org/multierr v1.5.0
 	go.uber.org/zap v1.15.0
-	google.golang.org/genproto v0.0.0-20200617032506-f1bdc9086088 // indirect
+	google.golang.org/genproto v0.0.0-20200622133129-d0ee0c36e670 // indirect
 	google.golang.org/protobuf v1.24.0
 	gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776
 )

go.sum

@@ -211,8 +211,8 @@ google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8/go.mod h1:JiN7NxoA
 google.golang.org/genproto v0.0.0-20190819201941-24fa4b261c55/go.mod h1:DMBHOl98Agz4BDEuKkezgsaosCRResVns1a3J2ZsMNc=
 google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013 h1:+kGHl1aib/qcwaRi1CbqBZ1rk19r85MNUf8HaBghugY=
 google.golang.org/genproto v0.0.0-20200526211855-cb27e3aa2013/go.mod h1:NbSheEEYHJ7i3ixzK3sjbqSGDJWnxyFXZblF3eUsNvo=
-google.golang.org/genproto v0.0.0-20200617032506-f1bdc9086088 h1:XXo4PvhJkaWYIkwn7bX7mcdB8RdcOvn12HbaUUAwX3E=
-google.golang.org/genproto v0.0.0-20200617032506-f1bdc9086088/go.mod h1:jDfRM7FcilCzHH/e9qn6dsT145K34l5v+OpcnNgKAAA=
+google.golang.org/genproto v0.0.0-20200622133129-d0ee0c36e670 h1:v/N9fZIfu6jopNImrZwgx48fql5yT3k82CJvpIyGtPA=
+google.golang.org/genproto v0.0.0-20200622133129-d0ee0c36e670/go.mod h1:FWY/as6DDZQgahTzZj3fqbO1CbirC29ZNUFHwi0/+no=
 google.golang.org/grpc v1.8.0/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw=
 google.golang.org/grpc v1.19.0/go.mod h1:mqu4LbDTu4XGKhr4mRzUsmM4RtVoemTSY81AxZiDr8c=
 google.golang.org/grpc v1.21.0/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ijfRaM=

internal/buf/bufcli/bufcli.go

@@ -44,7 +44,7 @@ type EnvReader interface {
 		configOverride string,
 		externalFilePaths []string,
 		externalFileFilePathsAllowNotExist bool,
-		excludeSourceInfo bool,
+		excludeSourceCodeInfo bool,
 	) (Env, []bufanalysis.FileAnnotation, error)
 	// GetImageEnv is the same as GetEnv but only allows image values and never builds.
 	GetImageEnv(
@@ -54,6 +54,7 @@ type EnvReader interface {
 		configOverride string,
 		externalFilePaths []string,
 		externalFileFilePathsAllowNotExist bool,
+		excludeSourceCodeInfo bool,
 	) (Env, error)
 	// GetSourceEnv is the same as GetEnv but only allows source values and always builds.
 	GetSourceEnv(
@@ -63,8 +64,17 @@ type EnvReader interface {
 		configOverride string,
 		externalFilePaths []string,
 		externalFileFilePathsAllowNotExist bool,
-		excludeSourceInfo bool,
+		excludeSourceCodeInfo bool,
 	) (Env, []bufanalysis.FileAnnotation, error)
+	// GetImage is the same as GetImageEnv but does not get a configuration.
+	GetImage(
+		ctx context.Context,
+		container app.EnvStdinContainer,
+		value string,
+		externalFilePaths []string,
+		externalFileFilePathsAllowNotExist bool,
+		excludeSourceCodeInfo bool,
+	) (bufimage.Image, error)
 	// ListFiles lists the files.
 	ListFiles(
 		ctx context.Context,

internal/buf/bufcli/env_reader.go

@@ -98,6 +98,7 @@ func (e *envReader) GetEnv(
 			configOverride,
 			externalFilePaths,
 			externalFilePathsAllowNotExist,
+			excludeSourceCodeInfo,
 			t,
 		)
 		return env, nil, err
@@ -123,6 +124,7 @@ func (e *envReader) GetImageEnv(
 	configOverride string,
 	externalFilePaths []string,
 	externalFilePathsAllowNotExist bool,
+	excludeSourceCodeInfo bool,
 ) (_ Env, retErr error) {
 	defer instrument.Start(e.logger, "get_image_env").End()
 	defer func() {
@@ -141,6 +143,7 @@ func (e *envReader) GetImageEnv(
 		configOverride,
 		externalFilePaths,
 		externalFilePathsAllowNotExist,
+		excludeSourceCodeInfo,
 		imageRef,
 	)
 }
@@ -176,6 +179,35 @@ func (e *envReader) GetSourceEnv(
 	)
 }
 
+func (e *envReader) GetImage(
+	ctx context.Context,
+	container app.EnvStdinContainer,
+	value string,
+	externalFilePaths []string,
+	externalFilePathsAllowNotExist bool,
+	excludeSourceCodeInfo bool,
+) (_ bufimage.Image, retErr error) {
+	defer instrument.Start(e.logger, "get_image").End()
+	defer func() {
+		if retErr != nil {
+			retErr = fmt.Errorf("%v: %w", e.valueFlagName, retErr)
+		}
+	}()
+
+	imageRef, err := e.fetchRefParser.GetImageRef(ctx, value)
+	if err != nil {
+		return nil, err
+	}
+	return e.getImage(
+		ctx,
+		container,
+		externalFilePaths,
+		externalFilePathsAllowNotExist,
+		excludeSourceCodeInfo,
+		imageRef,
+	)
+}
+
 func (e *envReader) ListFiles(
 	ctx context.Context,
 	container app.EnvStdinContainer,
@@ -194,7 +226,14 @@ func (e *envReader) ListFiles(
 	switch t := ref.(type) {
 	case buffetch.ImageRef:
 		// if we have an image, list the files in the image
-		image, err := e.getImage(ctx, container, t)
+		image, err := e.getImage(
+			ctx,
+			container,
+			nil,
+			false,
+			true,
+			t,
+		)
 		if err != nil {
 			return nil, err
 		}
@@ -250,36 +289,24 @@ func (e *envReader) getEnvFromImage(
 	configOverride string,
 	externalFilePaths []string,
 	externalFilePathsAllowNotExist bool,
+	excludeSourceCodeInfo bool,
 	imageRef buffetch.ImageRef,
 ) (_ Env, retErr error) {
-	image, err := e.getImage(ctx, container, imageRef)
+	image, err := e.getImage(
+		ctx,
+		container,
+		externalFilePaths,
+		externalFilePathsAllowNotExist,
+		excludeSourceCodeInfo,
+		imageRef,
+	)
 	if err != nil {
 		return nil, err
 	}
 	config, err := e.GetConfig(ctx, configOverride)
 	if err != nil {
 		return nil, err
 	}
-	if len(externalFilePaths) > 0 {
-		// this is usually a full rel file path, but because this is a read image,
-		// the root is always ".", so this is OK, although awkward
-		rootRelFilePaths := make([]string, len(externalFilePaths))
-		for i, externalFilePath := range externalFilePaths {
-			rootRelFilePath, err := imageRef.PathResolver().ExternalPathToRelPath(externalFilePath)
-			if err != nil {
-				return nil, err
-			}
-			rootRelFilePaths[i] = rootRelFilePath
-		}
-		if externalFilePathsAllowNotExist {
-			image, err = bufimage.ImageWithOnlyRootRelFilePathsAllowNotExist(image, rootRelFilePaths)
-		} else {
-			image, err = bufimage.ImageWithOnlyRootRelFilePaths(image, rootRelFilePaths)
-		}
-		if err != nil {
-			return nil, err
-		}
-	}
 	return newEnv(image, config), nil
 }
 
@@ -353,6 +380,9 @@ func (e *envReader) getEnvFromSource(
 func (e *envReader) getImage(
 	ctx context.Context,
 	container app.EnvStdinContainer,
+	externalFilePaths []string,
+	externalFilePathsAllowNotExist bool,
+	excludeSourceCodeInfo bool,
 	imageRef buffetch.ImageRef,
 ) (_ bufimage.Image, retErr error) {
 	readCloser, err := e.fetchReader.GetImageFile(ctx, container, imageRef)
@@ -366,21 +396,84 @@ func (e *envReader) getImage(
 	if err != nil {
 		return nil, err
 	}
-	// we cannot determine fileDescriptorProtos ahead of time so we cannot handle extensions
-	// TODO: we do not happen to need them for our use case with linting, but we need to dicuss this
 	protoImage := &imagev1.Image{}
 	switch imageEncoding := imageRef.ImageEncoding(); imageEncoding {
+	// we have to double parse due to custom options
+	// See https://github.com/golang/protobuf/issues/1123
+	// TODO: revisit
 	case buffetch.ImageEncodingBin:
-		err = protoencoding.NewWireUnmarshaler(nil).Unmarshal(data, protoImage)
+		firstProtoImage := &imagev1.Image{}
+		timer := instrument.Start(e.logger, "first_wire_unmarshal")
+		if err := protoencoding.NewWireUnmarshaler(nil).Unmarshal(data, firstProtoImage); err != nil {
+			return nil, fmt.Errorf("could not unmarshal Image: %v", err)
+		}
+		timer.End()
+		timer = instrument.Start(e.logger, "new_resolver")
+		// TODO right now, NewResolver sets AllowUnresolvable to true all the time
+		// we want to make this into a check, and we verify if we need this for the individual command
+		resolver, err := protoencoding.NewResolver(
+			firstProtoImage.File...,
+		)
+		if err != nil {
+			return nil, err
+		}
+		timer.End()
+		timer = instrument.Start(e.logger, "second_wire_unmarshal")
+		if err := protoencoding.NewWireUnmarshaler(resolver).Unmarshal(data, protoImage); err != nil {
+			return nil, fmt.Errorf("could not unmarshal Image: %v", err)
+		}
+		timer.End()
 	case buffetch.ImageEncodingJSON:
-		err = protoencoding.NewJSONUnmarshaler(nil).Unmarshal(data, protoImage)
+		firstProtoImage := &imagev1.Image{}
+		timer := instrument.Start(e.logger, "first_json_unmarshal")
+		if err := protoencoding.NewJSONUnmarshaler(nil).Unmarshal(data, firstProtoImage); err != nil {
+			return nil, fmt.Errorf("could not unmarshal Image: %v", err)
+		}
+		// TODO right now, NewResolver sets AllowUnresolvable to true all the time
+		// we want to make this into a check, and we verify if we need this for the individual command
+		timer.End()
+		timer = instrument.Start(e.logger, "new_resolver")
+		resolver, err := protoencoding.NewResolver(
+			firstProtoImage.File...,
+		)
+		if err != nil {
+			return nil, err
+		}
+		timer.End()
+		timer = instrument.Start(e.logger, "second_json_unmarshal")
+		if err := protoencoding.NewJSONUnmarshaler(resolver).Unmarshal(data, protoImage); err != nil {
+			return nil, fmt.Errorf("could not unmarshal Image: %v", err)
+		}
+		timer.End()
 	default:
 		return nil, fmt.Errorf("unknown image encoding: %v", imageEncoding)
 	}
+	if excludeSourceCodeInfo {
+		for _, fileDescriptorProto := range protoImage.File {
+			fileDescriptorProto.SourceCodeInfo = nil
+		}
+	}
+	image, err := bufimage.NewImageForProto(protoImage)
 	if err != nil {
-		return nil, fmt.Errorf("could not unmarshal Image: %v", err)
+		return nil, err
+	}
+	if len(externalFilePaths) == 0 {
+		return image, nil
+	}
+	// this is usually a full rel file path, but because this is a read image,
+	// the root is always ".", so this is OK, although awkward
+	rootRelFilePaths := make([]string, len(externalFilePaths))
+	for i, externalFilePath := range externalFilePaths {
+		rootRelFilePath, err := imageRef.PathResolver().ExternalPathToRelPath(externalFilePath)
+		if err != nil {
+			return nil, err
+		}
+		rootRelFilePaths[i] = rootRelFilePath
+	}
+	if externalFilePathsAllowNotExist {
+		return bufimage.ImageWithOnlyRootRelFilePathsAllowNotExist(image, rootRelFilePaths)
 	}
-	return bufimage.NewImageForProto(protoImage)
+	return bufimage.ImageWithOnlyRootRelFilePaths(image, rootRelFilePaths)
 }
 
 func (e *envReader) getSourceBucketAndConfig(

internal/buf/cmd/buf/buf.go

@@ -21,7 +21,7 @@ import (
 	"github.com/bufbuild/buf/internal/pkg/app/appflag"
 )
 
-const version = "0.17.0"
+const version = "0.18.0-dev"
 
 // Main is the main.
 func Main(use string, options ...RootCommandOption) {