Skip to content

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

Merged
bufdev merged 1 commit intomainfrom
jh/buf-build-exclude-source-options
Mar 7, 2024
Merged

Add --exclude-source-retention-options flag to 'buf build'#2807
bufdev merged 1 commit intomainfrom
jh/buf-build-exclude-source-options

Conversation

@jhump
Copy link
Copy Markdown
Member

@jhump jhump commented Mar 7, 2024 •
edited
Loading

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
@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

Reviewers

@bufdev bufdev bufdev approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jhump @bufdev