Add protocolbuffers/gofeatures module

[jhump]

No description provided.

[pkwarren]

Should this have a dependency on protocolbuffers/wellknowntypes? If so does it need to pin a version of the WKTs in the buf.yaml file?

[pkwarren]

I think this should depend on WKT - without it I got the following error running the fetch script locally (with buf v1.31.0):

google/protobuf/go_features.proto:17:28:extension pb.go: tag 1002 is not in valid range for extended type google.protobuf.FeatureSet

[jhump]

Hmm, unclear. The wellknowntypes never require an explicit dep since buf bundles them. They are only needed as a dependency to explicitly pin a particular version.

In this case, this version (1.34.0) requires v25.0 or higher of the wellknowntypes. And the next version will require v27.0 or higher. But will that be weird to pin it? Would that mean that all modules that use the gofeatures end up getting "stuck" on whatever version it pins? (I.e. they wouldn't end up using newer wellknowntypes, even if using a newer version of the CLI that bundles a newer version of them.)

[bufdev]

I agree with Philip here, let's make it a dependency

[jhump]

What's our stance on the version to pin: should it be the minimum version required (v25.0)? Or should we keep it recent (e.g. v26.1)?

I'm going to guess the minimum one since it doesn't look like we have anything automated to update it as newer versions of deps are released.

[pkwarren]

I think we can leave it unpinned if we want - it will just point at the latest WKT version when we sync on the user's system. That also means that we don't have to remember to bump the pinned version in the future.

[jhump]

it will just point at the latest WKT version when we sync on the user's system.

But if the user has already pinned the WKT module, but an older version, it will fail to build, right? I'm also wondering if we should do something different than leaving it unpinned in the next version (which will require a new v27+ of the wellknowntypes).

In fact, that makes me wonder if maybe we should hold off on trying to merge this until we have a v27 of wellknowntypes and a v1.35 or v1.34.1 (whatever they number it) for protobuf-go and not bother trying to sync the current v1.34.0...

[jhump]

Now that v27.0 of protocolbuffers/wellknowntypes is available, I've updated this so that it will start with v1.34.1 (which requires v27 of descriptor.proto). Updated the config in this PR.

I'll now start going through the checklist for new modules.

[jhump]

I ran the fetch action for this branch and it produced #567. The files there LGTM. @pkwarren, want to take one last look at this and #567 before I merge this?

[bufdev]

What are these stars for?

[pkwarren]

I believe it is to recurse in subdirectories but not positive. I can try removing it to see if it still works.

[pkwarren]

Removing that causes the fetch script to fail with Failure: "buf.build/protocolbuffers/gofeatures" had no .proto files.

[bufdev]

What does the rsync manual say?

[pkwarren]

       Note that, when using the --recursive (-r) option (which is implied by -a), every subcomponent of every path is visited from the top down, so include/exclude patterns get applied recursively to each subcomponent's
       full name (e.g. to include "/foo/bar/baz" the subcomponents "/foo" and "/foo/bar" must not be excluded).  The exclude patterns actually short-circuit the directory traversal stage when rsync finds the files to send.
       If a pattern excludes a particular parent directory, it can render a deeper include pattern ineffectual because rsync did not descend through that excluded section of the hierarchy.  This is particularly important
       when using a trailing '*' rule.  For instance, this won't work:

              + /some/path/this-file-will-not-be-found
              + /file-is-included
              - *

       This fails because the parent directory "some" is excluded by the '*' rule, so rsync never visits any of the files in the "some" or "some/path" directories.  One solution is to ask for all directories in the
       hierarchy to be included by using a single rule: "+ */" (put it somewhere before the "- *" rule), and perhaps use the --prune-empty-dirs option.  Another solution is to add specific include rules for all the parent
       dirs that need to be visited.  For instance, this set of rules works fine:

              + /some/
              + /some/path/
              + /some/path/this-file-is-found
              + /file-also-included
              - *

Specifically:

One solution is to ask for all directories in the hierarchy to be included by using a single rule: "+ */" (put it somewhere before the "- *" rule)

[bufdev]

Can you translate that - is there a tighter way to do this, or why or why not does this guarantee we are just limited to the two files we want?

[pkwarren]

If we wanted to be the most strict, then we could add these lines:

google/
google/protobuf/

However, using */ is a shortcut to not have to include those, since we're using the --prune-empty-dirs rsync option.