Add .txtpb support to convert command (#2293)

CHANGELOG.md

@@ -2,7 +2,10 @@
 
 ## [Unreleased]
 
-- No changes yet.
+- Add `txtpb` format to handle the Protobuf text format. and automatically recognize
+  `.txtpb` files as Protobuf text files. The `txtpb` format can now be used with
+  all `buf` commands that take images as input or output, such as `build`, `convert`,
+  and `curl`.
 
 ## [v1.24.0] - 2023-07-13
 

private/buf/bufconvert/bufconvert.go

@@ -32,10 +32,14 @@ const (
 	MessageEncodingBinpb MessageEncoding = iota + 1
 	// MessageEncodingJSON is the JSON image encoding.
 	MessageEncodingJSON
+	// MessageEncodingTextpb is the protobuf text image encoding.
+	MessageEncodingTextpb
 	// formatBinpb is the binary format.
 	formatBinpb = "binpb"
 	// formatJSON is the JSON format.
 	formatJSON = "json"
+	// formatTxtpb is the protobuf text format.
+	formatTxtpb = "txtpb"
 
 	// formatBin is the binary format's old form, now deprecated.
 	formatBin = "bin"
@@ -50,6 +54,7 @@ var (
 	messageEncodingFormats = []string{
 		formatBinpb,
 		formatJSON,
+		formatTxtpb,
 	}
 )
 
@@ -112,6 +117,8 @@ func parseMessageEncodingExt(ext string, defaultEncoding MessageEncoding) Messag
 		return MessageEncodingBinpb
 	case formatJSON:
 		return MessageEncodingJSON
+	case formatTxtpb:
+		return MessageEncodingTextpb
 	default:
 		return defaultEncoding
 	}
@@ -123,6 +130,8 @@ func parseMessageEncodingFormat(format string) (MessageEncoding, error) {
 		return MessageEncodingBinpb, nil
 	case formatJSON:
 		return MessageEncodingJSON, nil
+	case formatTxtpb:
+		return MessageEncodingTextpb, nil
 	default:
 		return 0, fmt.Errorf("invalid format for message: %q", format)
 	}

private/buf/buffetch/buffetch.go

@@ -34,6 +34,8 @@ const (
 	ImageEncodingBin ImageEncoding = iota + 1
 	// ImageEncodingJSON is the JSON image encoding.
 	ImageEncodingJSON
+	// ImageEncodingTxtpb is the text protobuf image encoding.
+	ImageEncodingTxtpb
 )
 
 var (

private/buf/buffetch/format.go

@@ -15,8 +15,10 @@
 package buffetch
 
 const (
-	// formatBinpb is the binary format.
+	// formatBinpb is the protobuf binary format.
 	formatBinpb = "binpb"
+	// formatTxtpb is the protobuf text format.
+	formatTxtpb = "txtpb"
 	// formatDir is the directory format.
 	formatDir = "dir"
 	// formatGit is the git format.
@@ -50,11 +52,13 @@ var (
 		formatBingz,
 		formatJSON,
 		formatJSONGZ,
+		formatTxtpb,
 	}
 	// sorted
 	imageFormatsNotDeprecated = []string{
 		formatBinpb,
 		formatJSON,
+		formatTxtpb,
 	}
 	// sorted
 	sourceFormats = []string{
@@ -119,6 +123,7 @@ var (
 		formatProtoFile,
 		formatTar,
 		formatTargz,
+		formatTxtpb,
 		formatZip,
 	}
 	// sorted
@@ -130,6 +135,7 @@ var (
 		formatMod,
 		formatProtoFile,
 		formatTar,
+		formatTxtpb,
 		formatZip,
 	}
 

private/buf/buffetch/ref_parser.go

@@ -52,6 +52,7 @@ func newRefParser(logger *zap.Logger) *refParser {
 			internal.WithSingleFormat(formatBin),
 			internal.WithSingleFormat(formatBinpb),
 			internal.WithSingleFormat(formatJSON),
+			internal.WithSingleFormat(formatTxtpb),
 			internal.WithSingleFormat(
 				formatBingz,
 				internal.WithSingleDefaultCompressionType(
@@ -96,6 +97,7 @@ func newImageRefParser(logger *zap.Logger) *refParser {
 			internal.WithSingleFormat(formatBin),
 			internal.WithSingleFormat(formatBinpb),
 			internal.WithSingleFormat(formatJSON),
+			internal.WithSingleFormat(formatTxtpb),
 			internal.WithSingleFormat(
 				formatBingz,
 				internal.WithSingleDefaultCompressionType(
@@ -372,6 +374,8 @@ func newRawRefProcessor() func(*internal.RawRef) error {
 				format = formatJSON
 			case ".tar":
 				format = formatTar
+			case ".txtpb":
+				format = formatTxtpb
 			case ".zip":
 				format = formatZip
 			case ".gz":
@@ -383,6 +387,8 @@ func newRawRefProcessor() func(*internal.RawRef) error {
 					format = formatJSON
 				case ".tar":
 					format = formatTar
+				case ".txtpb":
+					format = formatTxtpb
 				default:
 					return fmt.Errorf("path %q had .gz extension with unknown format", rawRef.Path)
 				}
@@ -395,6 +401,8 @@ func newRawRefProcessor() func(*internal.RawRef) error {
 					format = formatJSON
 				case ".tar":
 					format = formatTar
+				case ".txtpb":
+					format = formatTxtpb
 				default:
 					return fmt.Errorf("path %q had .zst extension with unknown format", rawRef.Path)
 				}
@@ -520,13 +528,17 @@ func processRawRefImage(rawRef *internal.RawRef) error {
 			format = formatBinpb
 		case ".json":
 			format = formatJSON
+		case ".textpb":
+			format = formatTxtpb
 		case ".gz":
 			compressionType = internal.CompressionTypeGzip
 			switch filepath.Ext(strings.TrimSuffix(rawRef.Path, filepath.Ext(rawRef.Path))) {
 			case ".bin", ".binpb":
 				format = formatBinpb
 			case ".json":
 				format = formatJSON
+			case ".textpb":
+				format = formatTxtpb
 			default:
 				return fmt.Errorf("path %q had .gz extension with unknown format", rawRef.Path)
 			}
@@ -537,6 +549,8 @@ func processRawRefImage(rawRef *internal.RawRef) error {
 				format = formatBinpb
 			case ".json":
 				format = formatJSON
+			case ".textpb":
+				format = formatTxtpb
 			default:
 				return fmt.Errorf("path %q had .zst extension with unknown format", rawRef.Path)
 			}
@@ -560,6 +574,8 @@ func parseImageEncoding(format string) (ImageEncoding, error) {
 		return ImageEncodingBin, nil
 	case formatJSON, formatJSONGZ:
 		return ImageEncodingJSON, nil
+	case formatTxtpb:
+		return ImageEncodingTxtpb, nil
 	default:
 		return 0, fmt.Errorf("invalid format for image: %q", format)
 	}

private/buf/buffetch/ref_parser_test.go

@@ -548,6 +548,46 @@ func TestGetParsedRefSuccess(t *testing.T) {
 		),
 		"path/to/file.json.gz#compression=gzip",
 	)
+	testGetParsedRefSuccess(
+		t,
+		internal.NewDirectParsedSingleRef(
+			formatTxtpb,
+			"path/to/file.txtpb",
+			internal.FileSchemeLocal,
+			internal.CompressionTypeNone,
+		),
+		"path/to/file.txtpb",
+	)
+	testGetParsedRefSuccess(
+		t,
+		internal.NewDirectParsedSingleRef(
+			formatTxtpb,
+			"path/to/file.txtpb.gz",
+			internal.FileSchemeLocal,
+			internal.CompressionTypeGzip,
+		),
+		"path/to/file.txtpb.gz",
+	)
+	testGetParsedRefSuccess(
+		t,
+		internal.NewDirectParsedSingleRef(
+			formatTxtpb,
+			"path/to/file.txtpb.gz",
+			internal.FileSchemeLocal,
+			internal.CompressionTypeNone,
+		),
+		"path/to/file.txtpb.gz#compression=none",
+	)
+	testGetParsedRefSuccess(
+		t,
+		internal.NewDirectParsedSingleRef(
+			formatTxtpb,
+			"path/to/file.txtpb.gz",
+			internal.FileSchemeLocal,
+			internal.CompressionTypeGzip,
+		),
+		"path/to/file.txtpb.gz#compression=gzip",
+	)
 	testGetParsedRefSuccess(
 		t,
 		internal.NewDirectParsedSingleRef(
@@ -568,6 +608,16 @@ func TestGetParsedRefSuccess(t *testing.T) {
 		),
 		"-#format=json",
 	)
+	testGetParsedRefSuccess(
+		t,
+		internal.NewDirectParsedSingleRef(
+			formatTxtpb,
+			"",
+			internal.FileSchemeStdio,
+			internal.CompressionTypeNone,
+		),
+		"-#format=txtpb",
+	)
 	testGetParsedRefSuccess(
 		t,
 		internal.NewDirectParsedSingleRef(

private/buf/bufwire/image_reader.go

@@ -97,31 +97,11 @@ func (i *imageReader) GetImage(
 		}
 		span.End()
 	case buffetch.ImageEncodingJSON:
-		firstProtoImage := &imagev1.Image{}
-		_, span := i.tracer.Start(ctx, "first_json_unmarshal")
-		if err := protoencoding.NewJSONUnmarshaler(nil).Unmarshal(data, firstProtoImage); err != nil {
-			span.RecordError(err)
-			span.SetStatus(codes.Error, err.Error())
-			span.End()
-			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
-		span.End()
-		_, newResolverSpan := i.tracer.Start(ctx, "new_resolver")
-		resolver, err := protoencoding.NewResolver(
-			bufimage.ProtoImageToFileDescriptors(
-				firstProtoImage,
-			)...,
-		)
+		resolver, err := i.bootstrapResolver(ctx, protoencoding.NewJSONUnmarshaler(nil), data)
 		if err != nil {
-			newResolverSpan.RecordError(err)
-			newResolverSpan.SetStatus(codes.Error, err.Error())
-			newResolverSpan.End()
 			return nil, err
 		}
-		newResolverSpan.End()
-		_, jsonUnmarshalSpan := i.tracer.Start(ctx, "second_json_unmarshal")
+		_, jsonUnmarshalSpan := i.tracer.Start(ctx, "json_unmarshal")
 		if err := protoencoding.NewJSONUnmarshaler(resolver).Unmarshal(data, protoImage); err != nil {
 			jsonUnmarshalSpan.RecordError(err)
 			jsonUnmarshalSpan.SetStatus(codes.Error, err.Error())
@@ -131,6 +111,21 @@ func (i *imageReader) GetImage(
 		jsonUnmarshalSpan.End()
 		// we've already re-parsed, by unmarshalling 2x above
 		imageFromProtoOptions = append(imageFromProtoOptions, bufimage.WithNoReparse())
+	case buffetch.ImageEncodingTxtpb:
+		resolver, err := i.bootstrapResolver(ctx, protoencoding.NewTxtpbUnmarshaler(nil), data)
+		if err != nil {
+			return nil, err
+		}
+		_, txtpbUnmarshalSpan := i.tracer.Start(ctx, "txtpb_unmarshal")
+		if err := protoencoding.NewTxtpbUnmarshaler(resolver).Unmarshal(data, protoImage); err != nil {
+			txtpbUnmarshalSpan.RecordError(err)
+			txtpbUnmarshalSpan.SetStatus(codes.Error, err.Error())
+			txtpbUnmarshalSpan.End()
+			return nil, fmt.Errorf("could not unmarshal image: %v", err)
+		}
+		txtpbUnmarshalSpan.End()
+		// we've already re-parsed, by unmarshalling 2x above
+		imageFromProtoOptions = append(imageFromProtoOptions, bufimage.WithNoReparse())
 	default:
 		return nil, fmt.Errorf("unknown image encoding: %v", imageEncoding)
 	}
@@ -168,3 +163,33 @@ func (i *imageReader) GetImage(
 	}
 	return bufimage.ImageWithOnlyPaths(image, imagePaths, excludePaths)
 }
+
+func (i *imageReader) bootstrapResolver(
+	ctx context.Context,
+	unresolving protoencoding.Unmarshaler,
+	data []byte,
+) (protoencoding.Resolver, error) {
+	firstProtoImage := &imagev1.Image{}
+	_, span := i.tracer.Start(ctx, "bootstrap_unmarshal")
+	if err := unresolving.Unmarshal(data, firstProtoImage); err != nil {
+		span.RecordError(err)
+		span.SetStatus(codes.Error, err.Error())
+		span.End()
+		return nil, fmt.Errorf("could not unmarshal image: %v", err)
+	}
+	span.End()
+	_, newResolverSpan := i.tracer.Start(ctx, "new_resolver")
+	resolver, err := protoencoding.NewResolver(
+		bufimage.ProtoImageToFileDescriptors(
+			firstProtoImage,
+		)...,
+	)
+	if err != nil {
+		newResolverSpan.RecordError(err)
+		newResolverSpan.SetStatus(codes.Error, err.Error())
+		newResolverSpan.End()
+		return nil, err
+	}
+	newResolverSpan.End()
+	return resolver, nil
+}

private/buf/bufwire/image_writer.go

@@ -117,6 +117,17 @@ func (i *imageWriter) imageMarshal(
 			return nil, err
 		}
 		return protoencoding.NewJSONMarshaler(resolver).Marshal(message)
+	case buffetch.ImageEncodingTxtpb:
+		// TODO: verify that image is complete
+		resolver, err := protoencoding.NewResolver(
+			bufimage.ImageToFileDescriptors(
+				image,
+			)...,
+		)
+		if err != nil {
+			return nil, err
+		}
+		return protoencoding.NewTxtpbMarshaler(resolver).Marshal(message)
 	default:
 		return nil, fmt.Errorf("unknown image encoding: %v", imageEncoding)
 	}

private/buf/bufwire/proto_encoding_reader.go

@@ -77,6 +77,8 @@ func (p *protoEncodingReader) GetMessage(
 		unmarshaler = protoencoding.NewWireUnmarshaler(resolver)
 	case bufconvert.MessageEncodingJSON:
 		unmarshaler = protoencoding.NewJSONUnmarshaler(resolver)
+	case bufconvert.MessageEncodingTextpb:
+		unmarshaler = protoencoding.NewTxtpbUnmarshaler(resolver)
 	default:
 		return nil, errors.New("unknown message encoding type")
 	}

private/buf/bufwire/proto_encoding_writer.go

@@ -65,6 +65,8 @@ func (p *protoEncodingWriter) PutMessage(
 		marshaler = protoencoding.NewWireMarshaler()
 	case bufconvert.MessageEncodingJSON:
 		marshaler = protoencoding.NewJSONMarshaler(resolver)
+	case bufconvert.MessageEncodingTextpb:
+		marshaler = protoencoding.NewTxtpbMarshaler(resolver)
 	default:
 		return errors.New("unknown message encoding type")
 	}

private/buf/cmd/buf/buf_test.go

@@ -2034,6 +2034,22 @@ func TestConvert(t *testing.T) {
 			"-#format=binpb",
 		)
 	})
+	t.Run("stdin-image-txtpb-to-binpb", func(t *testing.T) {
+		t.Parallel()
+		file, err := os.Open(convertTestDataDir + "/bin_json/image.txtpb")
+		require.NoError(t, err)
+		testRunStdoutFile(t,
+			file,
+			0,
+			convertTestDataDir+"/bin_json/payload.binpb",
+			"convert",
+			"--type=buf.Foo",
+			"-#format=txtpb",
+			"--from="+convertTestDataDir+"/bin_json/payload.txtpb",
+			"--to",
+			"-#format=binpb",
+		)
+	})
 }
 
 func TestFormat(t *testing.T) {