Generally we don't want to have commands that have >1 arguments - it's confusing for a user to remember what argument goes where. In this case, it's probably preferable to use required flags, ie --module and --plugin.

Yeah I was wondering about this

I'm not sure that we ever concluded that the version will be in the same format across all possible language-specific registries, especially as num(languages) -> infinity. This may happen to be the case now, but I'm not sure we can guarantee this in the long-term. @mfridman thoughts on this?

This is already true today, the maven registry has a different format , e.g., 0.1.7.1.20230403200515.aed131420688

But we should be able to infer the versioning scheme based on the plugin. Every plugin is categorized, so if there is no client-side logic for building a version and it's returned from the server this should be okay?

Is it true that every plugin MUST have exactly one language (and associated registry), or is this just a property of the current plugins we manage?

It's a property of the plugins we manage, but I imagine if that wasn't true, we'd break up the plugin into logical units to ensure there is exactly one supported registry.

Similar to what we did with protoc-gen-validate -> validate-{go,cpp,java}

all of this resolving is server side at the moment, if there's a plugin that isn't supported in a registry it will return an error. I figured we can change this behaviour later if we want it to default to something else

Yea the client should print as-is, but if we can't think of a use case as to why they should be mixed, having separate commands seems more correct, and I'd argue actually provides better UX - I am declaring I know what type of version I want to get, and I know I only get that type of version back. We can also error if the plugin does not match the requested registry, ie a maven-version call for protoc-gen-go would error.

That's fair, the only case I can see for slamming them together is making it easier for us to maintain and in the future when we add another registry a single command that takes plugin + module that returns any version will "just work" for older clients.

If we intro new registries though, in theory no previously unsuccessful calls to this command will have been made that will now be newly successful. Even more to the point, I'd want protoc-gen-Haskell to error right now based on there being no valid registry for it, and it's weird for it to go from error to non-error just because we released a new feature (ie the Haskell registry). Tldr, I think separate commands is a less surprising overall experience for the user.

should the API return the type of the registry through the already existing registryv1alpha1.PluginRegistryType, or should it populate a different field depending on what type the plugin is?

message GetRemotePackageVersionResponse {
    string go_version = 1; 
    string npm_version = 2;
    string maven_version = 3;
    string swift_version = 4;
}

I'd imagine they'd be separate fields in a oneof, or likely even more correct is separate RPCs

Should all be private

Why is this not in bufcli? This is where we've put all the rest of this CLI-specific stuff. Even moreso, private/buf is meant for things only used by the CLI, wheres bufpkg is shared across organization repositories, but since this package should be deleted it doesn't matter.

Why does this still exist?

Was going to delete in another PR but I'll add it into this one

Does something like this not already exist as a generic type?

#2169 (comment) it does but it's got field parameters attached to it that I don't want to use. The decision we came to is to make this type that until this file is deleted (it's already deprecated) and the apis are removed that this will continue to exist

Ugh it's PluginReference that needs to change, but that ship has sailed for this version of the package.

I'd call this PluginVersion and make it generic.

That already exists

message PluginVersion {
  // The ID of the plugin version, which uniquely identifies the plugin version.
  // Mostly used for pagination.
  string id = 1;
  // The name of the version, i.e. "v1.4.0".
  string name = 2;
  // The name of the plugin to which this version relates.
  string plugin_name = 3;
  // The owner of the plugin to which this version relates.
  string plugin_owner = 4;
  // The full container image digest associated with this plugin version including
  // the algorithm.
  // Ref: https://github.com/opencontainers/image-spec/blob/main/descriptor.md#digests
  string container_image_digest = 5;
  // Optionally define the runtime libraries.
  repeated RuntimeLibrary runtime_libraries = 6;
}

This is all stuff we need to clean up in v1, but yea not as relevant to this PR

This is invalid - a PluginReference is expected to be validated, and part of that validation is ensuring a version is present and conforms to SemVer. All of bufbuild/buf assumes that the properties on an interface are fulfilled, and no such validation (ie for empty fields that are expected to be non-empty) needs to be performed by a user of an instance of such interface. What you are doing in this function is saying "if I have a PluginReference, use it, otherwise return a PluginIdentity", which you need to represent in the code. You can't skip validation for PluginReference however.

I think this will mean that this function should not exist.

I'm going to make the function return (remote, owner, plugin, version string, err error) because otherwise I need to replicate the logic everywhere of checking if the passed string is a valid reference, and if not then checking if it's a valid identity, and then setting remote, owner, plugin, version vars appropriately

Also yeah I don't think it's a great idea to make a new struct ReferenceOrIdentity

We do not want a shared function here that has 5 return values - there's other solutions, such as func PluginIdentityAndVersionForString(string) (PluginIdentity, string, error) that is documented to return empty for the version if no version is present.

Is it valid to have an empty version? @pkwarren. If so, need to document this, and what the behavior is.

If this is like the GetCuratedPlugin endpoint, then omitting a version will return the latest version/revision of the plugin.

Yeah, it passes the version info through to GetCuratedPlugin without any modification atm

Make sure this behavior is documented on the API