adwinsky / goprotobuf

Automatically exported from code.google.com/p/goprotobuf
Other
0 stars 0 forks source link

Compiling two proto packages with dependencies #32

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
I'm having a problem compiling two proto files that I have. The two proto files 
are in different packages and one of them imports the other:

a.proto:
=====
package optimization.a;

message A {
  required uint32 id = 1;
}

b.proto:
=====
package optimization.b;

import "optimization/a.proto";

message B {
  required A a = 1;
}

I compile both files with goprotobuf and in b.pb.go I get an import 
"optimization/a.pb". The problem is that I'm not sure how I'm going to make 
this import work. I tried various things, but I guess I don't really understand 
exactly how go is going to try and import my a.pb.go file. Any suggestions?

Original issue reported on code.google.com by jesse...@gmail.com on 28 Aug 2012 at 1:50

GoogleCodeExporter commented 9 years ago
Sorry for the noise, I finally figured out what to do. I simply had to move the 
corresponding %.pb.go to %.pb directories so they could be correctly imported. 
It would be nice I guess to be able to specify alternate import paths (so that 
I could generate the files with a go gettable import path). I saw that there 
was some code that would allow this (option go_package), but couldn't figure 
out how to make it work.

Original comment by jesse...@gmail.com on 28 Aug 2012 at 2:58

GoogleCodeExporter commented 9 years ago

Original comment by dsymo...@golang.org on 30 Aug 2012 at 6:03

GoogleCodeExporter commented 9 years ago
I have the same issue, and FWIW, go_pacakge doesn't seem to work at all. I get:

foo.proto:3:8: Option "go_package" unknown.

I also looked at the test data, and found no test with go_package. And besides 
in comments, I can't find any reference of the string "go_package" in the code 
(unless it is some generated string somewhere, like in the protoc tool, as 
opposed to protoc-gen-go).

Thanks,

Original comment by yoh...@gmail.com on 1 Sep 2012 at 8:47

GoogleCodeExporter commented 9 years ago
The attached patch fixes the import problem for me. I am not 100% convinced it 
will work in every situation though, but it works in my case.

Original comment by yoh...@gmail.com on 10 Sep 2012 at 1:48

Attachments:

GoogleCodeExporter commented 9 years ago
This should do it.

Original comment by joe...@gmail.com on 22 Sep 2012 at 4:10

Attachments:

GoogleCodeExporter commented 9 years ago
This is just a side effect of the bug mentioned above, but the location of the 
generated descriptor.pb.go file makes it hard to import as well. Using the 
current naming convention (descriptor.pb/descriptor.pb.go) would make using 
{Field,Message}Options easier. (Even though options support is yet to be 
implemented.)

Original comment by ahochh...@samegoal.com on 30 Apr 2013 at 7:53

GoogleCodeExporter commented 9 years ago
i use another trick . i use the same package name with two proto files.

suppose there are two proto files. 

base.proto 
======
package protocol;
message kv {
    optional string k       = 1;
    optional string v       = 2;
}

protocol.proto 
====================
import "base.proto";
package protocol;
message request {
   optional kv a = 1;
}

====================
After i compiled these proto files. and found  "import protocol1 "base.pb" " in 
the file protocol.pb.go, After i write a script to remove "protocol1" , 
then i fixed the compilation error,  and it works.

proto.sh
==============

#! /bin/sh

protoc --go_out=. ./base.proto
protoc --go_out=. ./protocol.proto

sed_del_base_pb="/import\ protocol1\ \"base\.pb\"/d"
sed_del_protocol="s/protocol1\.//g"
mv ./protocol.pb.go temp && sed -e "$sed_del_base_pb" -e "$sed_del_protocol" 
./temp > ./protocol.pb.go

====================
the side effect is that all proto should be in the same packages. 

Any one have a better approach?

Original comment by LittleWh...@gmail.com on 28 Jun 2013 at 9:15

GoogleCodeExporter commented 9 years ago
Yeah, I use comment#4 's patch and then everything runs smoothly.

I just put .proto files along with my .go files, resulting in .pb.go files

FYI here is the implicit rule I used for make

#implicit rule to generate .pb.go
src/%.pb.go: src/%.proto
    @echo "protoc $<"
    @protoc -Isrc $(PROTOPATH) --go_out=src/ $<

where PROTOPATH is derived from GOPATH like this
PROTOPATH     :=$(patsubst %,-I%/src,$(subst :, ,$(GOPATH)) )

(note that the subst : is platform dependent, use ; instead of : on windowses )

On of the benefits is that you can share messages between go projects

BTW, any chance that this patch to be reintegrated into the code ?

Original comment by eric.ati...@mydoceapower.com on 28 Jun 2013 at 9:55

GoogleCodeExporter commented 9 years ago
In my production project, my workaround is add a option go_import_path. The 
generated code use the go_import_path to import other files or skip import when 
two file with same go_import_path.

Original comment by liuyuan...@gmail.com on 2 Aug 2013 at 2:47

Attachments:

GoogleCodeExporter commented 9 years ago
It'd be nice if we can merge these patches. 

Original comment by john.bel...@gmail.com on 9 Aug 2013 at 9:29

GoogleCodeExporter commented 9 years ago
I too would love seeing this merge, as I just ran into the same thing.

Original comment by m...@mihasya.com on 12 Sep 2013 at 9:06

GoogleCodeExporter commented 9 years ago
The goprotobuf.patch in comment #9 is causing panic errors.  Here is a patch 
for a fix on top of that.  You still need to apply the protobuf.patch.

Original comment by davidwu...@gmail.com on 23 Nov 2013 at 3:41

Attachments:

GoogleCodeExporter commented 9 years ago
I just suffer the same issue.

Original comment by jimenezr...@gmail.com on 18 Dec 2013 at 8:27

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
I had trouble importing a .proto from the same package, protoc-gen-go fails to 
notice they belong to the same package as it imposes a unique package name for 
every file.  The result is of course "import cycle not allowed".

a.proto:
package example;

message Foo {
  optional int32 f = 1;
}

b.proto:
package example;

import "example/a.proto"

message Bar {
  optional Foo foo = 1;
}

The patch in comment #5 has solved the issue, so I would love to see this 
merged in.

Original comment by clar...@gmail.com on 15 Apr 2014 at 11:00

GoogleCodeExporter commented 9 years ago
#5 has lapsed.

Original comment by laf...@gmail.com on 23 Apr 2014 at 2:48

GoogleCodeExporter commented 9 years ago
I found a "workaround", without the need for patch:

reusing the example in #7

base.proto 
====================
package protocol;
message kv {
    optional string k       = 1;
    optional string v       = 2;
}
====================

protocol.proto 
====================
import "base.proto";
package protocol;
message request {
   optional kv a = 1;
}
====================

then on the command line:

$ protoc --go_out=.  *.proto

The single most important thing is the *.proto here, apparently:

$ protoc a.proto ; protoc b.proto 

is not equivalent to :

$ protoc a.proto b.proto

Enjoy,

Original comment by e...@ericaro.net on 29 Jun 2014 at 7:33

GoogleCodeExporter commented 9 years ago
The output of this plugin is not even consistent with itself.  

- When importing a proto from the same proto package, it generates no import.  
This means that the .pb.go files must be in the same go package.
- When importing a proto from a different proto package, it generates an import 
that requires the specific proto file to be in its own package (e.g. import 
"foo/foo1.pb") 

Between those two properties, I'm not sure how to use this tool without post 
processing the files.

Example 

Using protoc version 2.5 and the latest goprotobuf

===========

### ./foo/foo1.proto
package foo;
message Foo1 {
    required int64 id = 1;
}

### ./foo/foo2.proto
package foo;
import "foo/foo1.proto";
message FooRequest {
    required Foo1 foo = 1;
}

### ./bar/bar.proto
package bar;
import "foo/foo2.proto";
message BarService {
    required foo.FooRequest req = 1;
}

===========

Compile it:
protoc --proto_path . --go_out=. foo/*
protoc --proto_path . --go_out=. bar/*

It produces these files:
./bar/bar.pb.go
./foo/foo1.pb.go
./foo/foo2.pb.go

- foo2.pb.go uses the type defined in foo1.pb.go without importing anything.
- bar.pb.go has the line: import foo1 "foo/foo2.pb"

The result does not build (foo builds, bar does not due to missing import)

This plugin must be used widely, so I'm not sure what I could be doing 
differently.  As a workaround, I use gogoprotobuf which is consistent on 
generating code to use the package-centric imports.  
https://code.google.com/p/gogoprotobuf/

Original comment by rob...@gmail.com on 5 Jul 2014 at 3:30

GoogleCodeExporter commented 9 years ago
Followup to comment #18: Your findings match mine and I was equally surprised. 
I found some undocumented flags that you can pass via the --go_out parameter 
which allow you to fix package and path names, and another poorly documented 
requirement is that each invocation of protoc requires that it be passed *all* 
the .proto files for that package. 

Here's an excerpt of a Makefile that invokes protoc allows for multiple 
cross-package dependencies and multiple .proto files in each package:

{{{
# assumes that make is invoked from the directory above src/.

# path to protoc-gen-go.
PROTOC_GEN_GO=$(abspath bin/protoc-gen-go)
# PROTOMAP creates a list of kv pairs of file path to package name. Example:
# 
Mexample.com/proto/package1/t.proto=example.com/proto/package1,Mexample.com/prot
o/package2/two.proto=example.com/proto/package2
PROTOMAP=$(shell (cd src && \
        for dotproto in `find . -iname *.proto`; do \
                echo M$$dotproto=`dirname $$dotproto`; \
        done) | paste -s -d,)
# PROTODIRS is a list of all directories containing .proto files. 
# protoc needs to be invoked per-package and be given all the filenames of 
protos in that package.
PROTODIRS=$(wildcard src/example.com/proto/*)
PROTODIRS+=src/example.com/some/other/directory/containing/protos
# Translates PROTODIRS into Go package names by stripping the src/ prefix
PROTOPKGS=$(PROTODIRS:src/%=%)
# for each directory that conatins protos, invoke protoc-gen-go and pass it all 
the .proto files
# in that package. Note that --go_out= includes $(PROTOMAP). 
$(PROTODIRS): bin/protoc-gen-go
        cd $@ && protoc --plugin=$(PROTOC_GEN_GO) \
                -I . -I ../../../ --go_out=$(PROTOMAP):. *.proto
.PHONY: $(PROTODIRS)
protodirs: $(PROTODIRS)
}}}

The end result is that you can use relative include paths when depending on 
other protos in the same directory, and top-level paths when depending across 
packages. Ex:

### example.com/proto/foo/foo1.proto
{{{
package foo;
message Foo1 {
    required int64 id = 1;
}
}}}

### example.com/proto/foo/foo2.proto
{{{
package foo;
import "foo1.proto";
message FooRequest {
    required Foo1 foo = 1;
}
}}}

### ./bar/bar.proto
{{{
package bar;
import "example.com/proto/foo/foo2.proto";
message BarService {
    required foo.FooRequest req = 1;
}
}}}

Original comment by expa...@gmail.com on 16 Jul 2014 at 7:13

GoogleCodeExporter commented 9 years ago
#19 good hints for package mapping things. Code generated for same package 
import looks good, but cross-package import still need post processing.
bar.proto from your example generate this
  import foo1 "example.com/proto/foo/foo2.pb"

but what I expected is 
  import foo1 "example.com/proto/foo"

Original comment by liuyuan...@gmail.com on 25 Oct 2014 at 8:43