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

blockchain_block_v1 is not defined #155

Open
VetchM opened this issue Aug 3, 2022 · 10 comments
Open

blockchain_block_v1 is not defined #155

VetchM opened this issue Aug 3, 2022 · 10 comments

Comments

@VetchM
Copy link

VetchM commented Aug 3, 2022

I'm tring to generate the pb file for my golang project, but every time i execute the protoc command i got some error.
the protoc command:

protoc -I=/c/User/Desktop/test/src/ --go_out=/c/User/Desktop/Helium/hnt/ /c/User/Desktop/test/src/blockchain_block.proto

output i got:

blockchain_block_v1.proto:21:12: "blockchain_txn" is not defined.
blockchain_block.proto:4:1: Import "blockchain_block_v1.proto" was not found or had errors.
blockchain_block.proto:7:17: "blockchain_block_v1" is not defined.

Can someone tell me how to fix this?

By the way i added option go_package = "./proto"; for every porot file, so it's probably not a go-generate problem

@madninja
Copy link
Member

madninja commented Aug 4, 2022

Looks like you may be missing some include paths for the protoc command

@VetchM
Copy link
Author

VetchM commented Aug 4, 2022

If i run the protoc cmd under the src directory, which i copied from this repo, no matter how i specify the --proto_path, it always reports the same error.

current path:

$ pwd
/c/User/Desktop/test/src

proto files in this path:

$ ls
blockchain_block.proto
blockchain_block_v1.proto 
blockchain_txn.proto
......

protoc cmd i tried:

$ protoc --go_out=/c/User/Desktop/Helium/hnt/ ./blockchain_block.proto
$ protoc --proto_path=. --go_out=/c/User/Desktop/Helium/hnt/ ./blockchain_block.proto
$ protoc --proto_path=/c/User/Desktop/test/src --go_out=/c/User/Desktop/Helium/hnt/ /c/User/Desktop/test/src/blockchain_block.proto

same error:

blockchain_block_v1.proto:21:12: "blockchain_txn" is not defined.
blockchain_block.proto:4:1: Import "blockchain_block_v1.proto" was not found or had errors.
blockchain_block.proto:7:17: "blockchain_block_v1" is not defined.

but the blockchain_txn.proto can be successful generated

$ protoc --go_out=/c/User/Desktop/Helium/hnt/ ./blockchain_txn.proto

Any suggestion? I don't what i can do to make it works

@madninja
Copy link
Member

madninja commented Aug 4, 2022

Does your protoc not take a -I option for include paths? if so you should use that

@VetchM
Copy link
Author

VetchM commented Aug 5, 2022

The -I or --proto_path works for most .proto file, except for blockchain_block.proto and blockchain_block_v1.proto. I compiled every .proto file other than these two.

@ederuiter
Copy link

Today I encountered the same problem; the issue is that the blockchain_block_v1.proto file does not contain a package helium so in order to import definitions that are part of the helium package you need to explicitly specify the package to import from.

So changing the definition to helium.blockchain_txn will fix the error

message blockchain_block_v1 {
  bytes prev_hash = 1;
  uint64 height = 2;
  uint64 time = 3;
  uint64 hbbft_round = 4;
  repeated helium.blockchain_txn transactions = 5; //<= change this line to fix it
  repeated blockchain_signature_v1 signatures = 6;
  uint64 election_epoch = 7;
  uint64 epoch_start = 8;
  bytes rescue_signature = 9;
  repeated blockchain_seen_vote_v1 seen_votes = 10;
  bytes bba_completion = 11;
  bytes snapshot_hash = 12;
  repeated bytes rescue_signatures = 13;
}

Not sure why this only seems to be a problem for the golang generation; it seems this is a bug in the proto file and all generator should have the same issue.

@madninja
Copy link
Member

Today I encountered the same problem; the issue is that the blockchain_block_v1.proto file does not contain a package helium so in order to import definitions that are part of the helium package you need to explicitly specify the package to import from.

So changing the definition to helium.blockchain_txn will fix the error

message blockchain_block_v1 {
  bytes prev_hash = 1;
  uint64 height = 2;
  uint64 time = 3;
  uint64 hbbft_round = 4;
  repeated helium.blockchain_txn transactions = 5; //<= change this line to fix it
  repeated blockchain_signature_v1 signatures = 6;
  uint64 election_epoch = 7;
  uint64 epoch_start = 8;
  bytes rescue_signature = 9;
  repeated blockchain_seen_vote_v1 seen_votes = 10;
  bytes bba_completion = 11;
  bytes snapshot_hash = 12;
  repeated bytes rescue_signatures = 13;
}

Not sure why this only seems to be a problem for the golang generation; it seems this is a bug in the proto file and all generator should have the same issue.

Have a PR? The CI will show errors if this works or is broken. It’s not a problem for JavaScript, rust, erlang and C/C++

@ederuiter
Copy link

@madninja If you want I can create a PR, but for the golang generation to work I also needed 1 other change

I have applied both changes in the project I am currently working on:

I also found a commit in the python PR that had similar changes

With these changes I can build all definitions for golang .. but it is still a bit complicated: you'll still need to map all files to a golang package and all services need to be in a separate package, otherwise things like the GatewayClient in iot_config service and the one in gateway service have a name clash.

package="github.com/thisisdevelopment/helium-route-updater/pkg"
dir="api/helium"
go_opts="--go_opt=module=${package}"
grpc_opts="--go-grpc_opt=module=${package}"
for file in $(find ${dir} -not -path "${dir}/service/*" -name "*.proto"); do
  file=${file##$dir/}
  files="${files} ${file}"
  subdir=$(dirname $file)
  file_package=${package}/${dir}
  if [ "${subdir}" != "." ]; then
    file_package="${file_package}/${subdir}"
  fi
  go_opts="${go_opts} --go_opt=M${file}=${file_package}"
  grpc_opts="${grpc_opts} --go-grpc_opt=M${file}=${file_package}"
done
protoc -I=$dir --go_out=pkg --go-grpc_out=pkg ${go_opts} ${grpc_opts} $files

for file in ${dir}/service/*.proto; do
  service=${file##$dir/service/}
  service_name=${service%%.proto}
  protoc -I=$dir --go_out=pkg --go-grpc_out=pkg ${go_opts} ${grpc_opts} --go_opt=Mservice/${service}=${package}/${dir}/service/${service_name} --go-grpc_opt=Mservice/${service}=${package}/${dir}/service/${service_name} service/${service}
done

^ You can find this here

@madninja
Copy link
Member

Ugh yeah. I remember this. It’d break a number tot downstream applications

@ederuiter
Copy link

It seems the protoc compiler behaves differently depending on the order of it encountering dupliclate symbol names: https://github.com/protocolbuffers/protobuf/blob/364328858e2694f4f85613cf168a96053eb434d6/src/google/protobuf/descriptor.cc#L4696
If the existing symbol is a package no error is generated, if the existing symbol is not a package an error is generated.

It seems the intent is to prevent it, but the current implementation allows it under some circumstances

@ederuiter
Copy link

After some more testing I found that the error messages generated by protoc are order dependant; but it does generate errors in both cases; just different ones

test.proto

syntax = "proto3";
package test;
option go_package = "github.com/thisisdevelopment/helium-route-updater/test";

enum origin {
  hello = 0;
  world = 1;
}

world.proto

syntax = "proto3";
package test.world;

option go_package = "github.com/thisisdevelopment/helium-route-updater/test/world";

enum World {
  Europe = 0;
  Asia = 1;
}

protoc -I=api/test --go_out=pkg --go-grpc_out=pkg base.proto world.proto

world.proto:2:1: "test.world" is already defined (as something other than a package) in file "base.proto".

protoc -I=api/test --cpp_out=pkg world.proto base.proto

base.proto:7:3: "test.world" is already defined in file "world.proto".
base.proto:7:3: Note that enum values use C++ scoping rules, meaning that enum values are siblings of their type, not children of it.  Therefore, "world" must be unique within "test", not just within "origin".

The real reason the other builds are not giving errors is that they simply don't build all proto files:

build.cpp

SERVICES=$(echo src/service/{iot_config,downlink,follower,gateway,local,packet_router,poc_lora,poc_mobile,router,state_channel,transaction}.proto)
MESSAGES=$(echo src/{blockchain_txn,entropy,data_rate,region,mapper}.proto)
SRC_FILES="$SERVICES $MESSAGES"

build.rs

const SERVICES: &[&str] = &[
    "src/service/router.proto",
    "src/service/state_channel.proto",
    "src/service/local.proto",
    "src/service/gateway.proto",
    "src/service/transaction.proto",
    "src/service/follower.proto",
    "src/service/poc_mobile.proto",
    "src/service/poc_lora.proto",
    "src/service/poc_entropy.proto",
    "src/service/packet_router.proto",
    "src/service/iot_config.proto",
    "src/service/downlink.proto",
    "src/service/packet_verifier.proto",
];

const MESSAGES: &[&str] = &[
    "src/blockchain_txn.proto",
    "src/entropy.proto",
    "src/data_rate.proto",
    "src/region.proto",
    "src/mapper.proto",
    "src/reward_manifest.proto",
    "src/blockchain_region_param_v1.proto",
];

Note that both builds have different proto files that they compile!
At least for the cpp build it seems the generated code is not complete as for example the service/gateway.pb.h tries to include files which are not generated (all blockchain_*.h files)

And if I try to include all protofiles in the cpp build it will give the same errors as with the golang output:

protoc -I=api/helium --cpp_out=pkg api/helium/*.proto api/helium/service/*.proto

blockchain_block_v1.proto:20:12: "blockchain_txn" is not defined.
blockchain_block.proto:3:1: Import "blockchain_block_v1.proto" was not found or had errors.
blockchain_block.proto:6:17: "blockchain_block_v1" is not defined.

After fixing that error, the error caused by the duplicate symbols pops up:

radio.proto:3:1: "helium.radio" is already defined (as something other than a package) in file "blockchain_poc_core_v1.proto".

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