Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --exclude-source-retention-options flag to 'buf build' #2807

Merged
merged 1 commit into from
Mar 7, 2024

Conversation

jhump
Copy link
Member

@jhump jhump commented Mar 7, 2024

This adds a flag that controls whether source-retention options are included in the output image or descriptor set from buf build.

I compared the output to protoc and did notice one small discrepancy: the code I wrote in protocompile leaves an empty options field if it happens to remove all options (i.e. if they were all source-retention). But protoc clears that field if it ends up empty. I can go make a small PR to protocompile, but I don't think the difference really matters or is worth holding any of this up.

(Never mind the unrecognized field 8042: I didn't use --as-file-descriptor-set with buf build when creating these files. So the right-hand column is actually an image, so it has that extra ImageFile field which is interpreted as an unrecognized field of FileDescriptorProto.)

> diff -W 110 -y test-w-protoc-minimal.txt test-w-buf-minimal.txt
file {						       file {
  name: "test.proto"				         name: "test.proto"
  dependency: "google/protobuf/descriptor.proto"         dependency: "google/protobuf/descriptor.proto"
  message_type {				         message_type {
    name: "Foo"					           name: "Foo"
    extension_range {				           extension_range {
      start: 1					             start: 1
      end: 1001					             end: 1001
						     >       options {
						     >       }
    }						           }
  }						         }
  extension {					         extension {
    name: "foo"					           name: "foo"
    extendee: ".google.protobuf.FileOptions"	           extendee: ".google.protobuf.FileOptions"
    number: 10101				           number: 10101
    label: LABEL_OPTIONAL			           label: LABEL_OPTIONAL
    type: TYPE_STRING				           type: TYPE_STRING
    options {					           options {
      retention: RETENTION_SOURCE		             retention: RETENTION_SOURCE
    }						           }
    json_name: "foo"				           json_name: "foo"
						     >   }
						     >   options {
						     >   }
						     >   8042 {
						     >     1: 0
						     >     3: 0
  }						         }
}						       }

The file I was testing with included both standard options that are source-only (the extension range declaration stuff, which is why option retention was introduced in the first place) and a custom option with source-only retention:

syntax = "proto2";

import "google/protobuf/descriptor.proto";

extend google.protobuf.FileOptions {
  optional string foo = 10101 [retention = RETENTION_SOURCE];
}

message Foo {
  extensions 1 to 1000 [
    declaration = {
      number: 10,
      full_name: ".foo.bar.baz";
      type: "int32"
    },
    declaration = {
      number: 20,
      full_name: ".bob.lob.law",
      type: ".bob.lob.Lawdy"
      repeated: true
    },
    declaration = {
      number: 30
      reserved: true
    },
    verification = DECLARATION
  ];
}

option (foo) = "abcdefghijklmnopqrstuvwxz0123456789";

@jhump jhump requested a review from bufdev March 7, 2024 21:51
@bufdev bufdev merged commit fa6f9de into main Mar 7, 2024
13 checks passed
@bufdev bufdev deleted the jh/buf-build-exclude-source-options branch March 7, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants