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

Update go package names to hyperledger/fabric-protos-go-apiv2 #225

Open
sandeepnRES opened this issue May 16, 2024 · 7 comments
Open

Update go package names to hyperledger/fabric-protos-go-apiv2 #225

sandeepnRES opened this issue May 16, 2024 · 7 comments

Comments

@sandeepnRES
Copy link
Member

sandeepnRES commented May 16, 2024

I believe https://github.com/hyperledger/fabric-protos-go is deprecated, and https://github.com/hyperledger/fabric-protos-go-apiv2 is the latest fabric-protos-go package.
Hence this needs to be updated in all .proto definitions as:

option go_package = "github.com/hyperledger/fabric-protos-go-apiv2";

This is so that if some other's project's .proto depends upon fabric-protos, then on compiling it created dependency to deprecated version i.e. github.com/hyperledger/fabric-protos-go, which then later on create conflicts with go applications depending on this proto and fabric-protos.
I'm a maintainer/co-owner of Hyperledger/Cacti, where we are facing this issue.

I've a bash script to do this (which is what I'm using for workaround), if you are okay with it, I can create PR for this.

script:

Script

#!/bin/bash

ROOT_DIR=${1:-'..'}

pushd $ROOT_DIR
files=$(find ./fabric-protos -name *.proto)

for filename in $files
do
sed -i'.scriptbak' -e ' s#^option go_package = "github.com/hyperledger/fabric-protos-go/#option go_package = "github.com/hyperledger/fabric-protos-go-apiv2/#' "$filename"
rm -rf $filename.scriptbak
cat $filename | grep "option go_package"
done
popd $ROOT_DIR

@jt-nti
Copy link
Member

jt-nti commented May 16, 2024

I've also been wondering if it's time to update the go_packages to apiv2. It might make sense to wait until Fabric switches to apiv2 though. It's great to have a script for those that want to switch sooner, and I think Buf is another option until the .proto files are updated, i.e. using the managed option.

@sandeepnRES
Copy link
Member Author

sandeepnRES commented May 16, 2024

@jt-nti Thanks for the quick reply, so its officially confirmed that going ahead apiv2 will be used, and github.com/hyperledger/fabric-protos-go will be deprecated.
If I update it in our project, using apiv2 in fabric chaincodes would cause any issue? Considering the fabric peers would be using the old fabric-protos-go

@jt-nti
Copy link
Member

jt-nti commented May 16, 2024

@sandeepnRES I'm not sure how official it is but I think @denyeart wants to go ahead with apiv2. It would certainly be nice to deprecate github.com/hyperledger/fabric-protos-go. As far as I know, you shouldn't have any issues updating to apiv2 in your project- fabric-gateway is already using apiv2 for example.

@bestbeforetoday
Copy link
Member

The wire format is unchanged between the two API versions so client and server ends of a connection can use different API versions completely seamlessly. As @jt-nti mentions, the fabric-gateway client API uses the newer API, and also switched API versions between releases with no Fabric compatibility issues. The problem is when both fabric-protos versions are mixed in the same Go module as you have two Go bindings for the same protobuf namespace:

https://protobuf.dev/reference/go/faq/#namespace-conflict

Go chaincode runs in its own chaincode container/process separate from the peer so it seems like it could be migrated easily to fabric-protos-go-apiv2. The problem looks to be that Fabric itself has a dependency on both fabric-chaincode-go and fabric-protos.

Changing Fabric core means also changing other libraries that it depends on, and is a bigger job. I think there is a willingness to make this change for the v3 release. The issue is a lack of time/resource to make that change in time for the v3 release.

@denyeart likely has a better informed opinion than me. 😉

@sandeepnRES
Copy link
Member Author

@bestbeforetoday @jt-nti Thanks for the confirmation. But I just now noticed that, these two packages:

	github.com/hyperledger/fabric-chaincode-go
	github.com/hyperledger/fabric-contract-api-go 

still uses v1 fabric-protos-go, so this is creating conflict if I try to update in our chaincode.

@jt-nti
Copy link
Member

jt-nti commented May 16, 2024

@sandeepnRES It sounds like you'll need to stay with the old api until Fabric updates, unless there's some way to untangle the dependency Fabric has on fabric-chaincode-go sooner.

@bestbeforetoday
Copy link
Member

bestbeforetoday commented May 16, 2024

As I mentioned (probably not clearly enough) above, fabric-chaincode-go still uses the fabric-protos-go, but can probably be very easily updated to fabric-protos-go-apiv2, which would make chaincode developers happy. It would still work with Fabric (on the old fabric-protos-go) fine.

The issue is that fabric has a dependency (perhaps only for testing?) on fabric-chaincode-go. As soon as fabric picks up a version of fabric-chaincode-go that uses fabric-protos-go-apiv2, there will be a potential conflict in the fabric codebase.

One thing we could do is to create a fabric-chaincode-go/v2 package that uses fabric-protos-go-apiv2. Fabric could continue to depend on fabric-chaincode-go until it is able to move to fabric-protos-apiv2 and fabric-chaincode-go/v2. Creating a v2 release of the chaincode (which is currently not versioned at all) is a little more heavy-weight, but it does mean that nobody can accidentally update to a version where the protos have changed and potentially cause a breakage. It's just a case of copying all the code to a sub-directory and making some changes to the PR build. There could be an accompanying fabric-contract-api-go/v2 package.

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

No branches or pull requests

3 participants