change go package

[pfi79]

No description provided.

[pfi79]

@denyeart Help me figure out what's wrong.
I think I've definitely fixed the error.

Here are examples of correctly filled go_package:

• https://github.com/hyperledger/fabric-protos/blob/main/orderer/ab.proto#L7
• https://github.com/hyperledger/fabric-protos/blob/main/orderer/blockattestation.proto#L7
• https://github.com/hyperledger/fabric-protos/blob/main/orderer/cluster.proto#L7
• https://github.com/hyperledger/fabric-protos/blob/main/orderer/clusterserver.proto#L7
• https://github.com/hyperledger/fabric-protos/blob/main/orderer/configuration.proto#L7
• https://github.com/hyperledger/fabric-protos/blob/main/orderer/etcdraft/configuration.proto#L7
• https://github.com/hyperledger/fabric-protos/blob/main/orderer/etcdraft/metadata.proto#L7

I just changed the file https://github.com/hyperledger/fabric-protos/blob/main/orderer/smartbft/configuration.proto to the same form

[jt-nti]

Hi @pfi79, I'm not sure what you're trying to fix but code generation is managed by Buf, including the default go package...

https://github.com/hyperledger/fabric-protos/blob/main/buf.gen.yaml#L5

[pfi79]

https://github.com/hyperledger/fabric-protos/blob/main/buf.gen.yaml#L5

So I should change it to fabric-protos-go-apiv2 everywhere?

[pfi79]

Hi @pfi79, I'm not sure what you're trying to fix but code generation is managed by Buf, including the default go package...

https://github.com/hyperledger/fabric-protos/blob/main/buf.gen.yaml#L5

Let me try to tell you what we do.
For our application we created our proto-files, where we connect proto-files from fabric-protos.
When we try to compile the go-modules, we get an error.
Buf fails on option go_package = "github.com/hyperledger/fabric/protos/orderer/smartbft";, saying that no such package exists. So it really doesn't exist.
All other proto-files in fabric-protos have the following line: option go_package = "github.com/hyperledger/fabric-protos-go/...........

Therefore, this line is set incorrectly in the orderer/smartbft/configuration.proto file.

[pfi79]

Basically, I create my proto file:

syntax = "proto3";

package myproto;
option go_package = "gihub.com/some-org/some-project/proto";

import "orderer/smartbft/configuration.proto";

message MyMessage {
  string arg1 = 1;
  string arg2 = 2;
  string orderer.smartbft.Options opts = 3;
}

buf will generate a .pb.go file with this import section.

// Code generated by protoc-gen-go. DO NOT EDIT.
............

package proto

import (
	smartbft "github.com/hyperledger/fabric/protos/orderer/smartbft"
	.............

But there is no such module github.com/hyperledger/fabric/protos/orderer/smartbft

[denyeart]

For Fabric we still use apiv1.
See the buf config for apiv1:
https://github.com/hyperledger/fabric-protos/blob/main/buf.gen-apiv1.yaml

I believe paths=source_relative overrides the go_package and puts everything in the correct package regardless of go_package. So I think this change will be a noop for Fabric. But I agree with the change and think we should merge regardless of the Lint failure.

@jt-nti Do you agree? How do we suppress the Lint failure so that it builds?

[jt-nti]

@denyeart I don't think we want to supress the lint warning since in general we probably want to think about changing packages. (Talking of which, is it too soon to update the go packages to github.com/hyperledger/fabric-protos-go-apiv2?)

I would keep the lint warning and if you're happy that it's ok for this PR, then you should be able to force merge it.

(I tried to approve the PR but I've been removed from the Hyperledger org so it wouldn't let me 🙁)

[C0rWin]

@denyeart I don't think we want to supress the lint warning since in general we probably want to think about changing packages. (Talking of which, is it too soon to update the go packages to github.com/hyperledger/fabric-protos-go-apiv2?)

I would keep the lint warning and if you're happy that it's ok for this PR, then you should be able to force merge it.

(I tried to approve the PR but I've been removed from the Hyperledger org so it wouldn't let me 🙁)

given you are ok, I've approved this PR.

[denyeart]

@jt-nti There have been some challenges updating core Fabric to apiv2, see discussion in hyperledger/fabric#3650