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

Compile google/protobuf/*.proto files into golang structs and add to a new package #50

Closed
peter-edge opened this Issue Jul 16, 2015 · 24 comments

Comments

Projects
None yet
8 participants
@peter-edge

peter-edge commented Jul 16, 2015

I did this https://github.com/peter-edge/go-google-protobuf which is fine, but I would love to not have to maintain this, and it makes a lot of sense to have it somewhere in here. I would be happy to do the work and commit it over, but before doing any work on this, wanted to see how open the maintainers were to it. Thoughts?

Also addresses #49

@grepory

This comment has been minimized.

Show comment
Hide comment
@grepory

grepory Sep 15, 2015

@peter-edge Hi. Sorry for this, but I just started doing:

sed -i 's/import google_protobuf "google\/protobuf"/import google_protobuf "github.com\/peter-edge\/go-google-protobuf"/' ${proto_dir}/checker.pb.go

Because works. Let me know if I can do anything to help maintain. I'm not sure of how it might be integrated into this project, exactly. But yeah... I have this problem, too.

grepory commented Sep 15, 2015

@peter-edge Hi. Sorry for this, but I just started doing:

sed -i 's/import google_protobuf "google\/protobuf"/import google_protobuf "github.com\/peter-edge\/go-google-protobuf"/' ${proto_dir}/checker.pb.go

Because works. Let me know if I can do anything to help maintain. I'm not sure of how it might be integrated into this project, exactly. But yeah... I have this problem, too.

@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Sep 15, 2015

@grepory note that you'll want to update this to go.pedge.io/google-protobuf instead (started using custom import urls for my active golang projects)

I wrote a tool that takes care of everything for you called protoc-all: https://github.com/peter-edge/go-tools/tree/master/protoc-all

It fixes all the import paths etc too. Sample usage: https://github.com/pachyderm/pachyderm/blob/master/Makefile#L85

You shouldn't need to set PROTOC_INCLUDE_PATH in general. Ping me at my email if you need help.

peter-edge commented Sep 15, 2015

@grepory note that you'll want to update this to go.pedge.io/google-protobuf instead (started using custom import urls for my active golang projects)

I wrote a tool that takes care of everything for you called protoc-all: https://github.com/peter-edge/go-tools/tree/master/protoc-all

It fixes all the import paths etc too. Sample usage: https://github.com/pachyderm/pachyderm/blob/master/Makefile#L85

You shouldn't need to set PROTOC_INCLUDE_PATH in general. Ping me at my email if you need help.

@grepory

This comment has been minimized.

Show comment
Hide comment
@grepory

grepory Sep 15, 2015

Why are you so awesome? Thanks. ❤️

grepory commented Sep 15, 2015

Why are you so awesome? Thanks. ❤️

@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Sep 15, 2015

Theoretically I'm paid to be amazing, but hey, I think the joke is on Pachyderm :)

peter-edge commented Sep 15, 2015

Theoretically I'm paid to be amazing, but hey, I think the joke is on Pachyderm :)

@chancez

This comment has been minimized.

Show comment
Hide comment
@chancez

chancez Sep 29, 2015

This would be awesome. It would make using protobuf with Go significantly easier

chancez commented Sep 29, 2015

This would be awesome. It would make using protobuf with Go significantly easier

@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Oct 24, 2015

Hey, ping on this, I'm seeing a bunch of people starting to import go.pedge.io/google-protobuf and obviously that's sub-optimal :) I also just wrote go.pedge.io/googleapis for the same purpose, which references go.pedge.io/google-protobuf, I'd love if Google just took over both of them heh.

peter-edge commented Oct 24, 2015

Hey, ping on this, I'm seeing a bunch of people starting to import go.pedge.io/google-protobuf and obviously that's sub-optimal :) I also just wrote go.pedge.io/googleapis for the same purpose, which references go.pedge.io/google-protobuf, I'd love if Google just took over both of them heh.

@chai2010

This comment has been minimized.

Show comment
Hide comment
@chai2010

chai2010 Oct 29, 2015

build type.proto failed:

protoc version:

protoc --version
libprotoc 3.0.0

error message:

protoc --go_out=. any.proto source_context.proto type.proto
google/protobuf/any.proto:94:10: "google.protobuf.Any.type_url" is already defined in file "any.proto".
google/protobuf/any.proto:97:9: "google.protobuf.Any.value" is already defined in file "any.proto".
google/protobuf/any.proto:73:9: "google.protobuf.Any" is already defined in file "any.proto".
google/protobuf/source_context.proto:46:10: "google.protobuf.SourceContext.file_name" is already defined in file "source_context.proto".
google/protobuf/source_context.proto:43:9: "google.protobuf.SourceContext" is already defined in file "source_context.proto".
type.proto: Import "google/protobuf/any.proto" was not found or had errors.
type.proto: Import "google/protobuf/source_context.proto" was not found or had errors.
type.proto:55:3: "google.protobuf.SourceContext" seems to be defined in "source_context.proto", which is not imported by "type.proto".  To use it here, please add the necessary import.
type.proto:147:3: "google.protobuf.SourceContext" seems to be defined in "source_context.proto", which is not imported by "type.proto".  To use it here, please add the necessary import.
type.proto:167:3: "google.protobuf.Any" seems to be defined in "any.proto", which is not imported by "type.proto".  To use it here, please add the necessary import.

chai2010 commented Oct 29, 2015

build type.proto failed:

protoc version:

protoc --version
libprotoc 3.0.0

error message:

protoc --go_out=. any.proto source_context.proto type.proto
google/protobuf/any.proto:94:10: "google.protobuf.Any.type_url" is already defined in file "any.proto".
google/protobuf/any.proto:97:9: "google.protobuf.Any.value" is already defined in file "any.proto".
google/protobuf/any.proto:73:9: "google.protobuf.Any" is already defined in file "any.proto".
google/protobuf/source_context.proto:46:10: "google.protobuf.SourceContext.file_name" is already defined in file "source_context.proto".
google/protobuf/source_context.proto:43:9: "google.protobuf.SourceContext" is already defined in file "source_context.proto".
type.proto: Import "google/protobuf/any.proto" was not found or had errors.
type.proto: Import "google/protobuf/source_context.proto" was not found or had errors.
type.proto:55:3: "google.protobuf.SourceContext" seems to be defined in "source_context.proto", which is not imported by "type.proto".  To use it here, please add the necessary import.
type.proto:147:3: "google.protobuf.SourceContext" seems to be defined in "source_context.proto", which is not imported by "type.proto".  To use it here, please add the necessary import.
type.proto:167:3: "google.protobuf.Any" seems to be defined in "any.proto", which is not imported by "type.proto".  To use it here, please add the necessary import.
@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Oct 29, 2015

I think that is a separate issue, not relevant to this one :)

(also in your compile there, you have to have -I to the base protobuf src directory, which is usually /usr/include on linux machines, /usr/local/include on macs if you installed protobuf with homebrew, or just /path/to/github.com/google/protobuf/src if you clone https://github.com/google/protobuf)

DIR=/usr/include
protoc -I $DIR $DIR/google/protobuf/any.proto $DIR/google/protobuf/source_context.proto $DIR/google/protobuf/type.proto

(or just use go.pedge.io/google-protobuf for now, which is what this issue is trying to resolve :) )

peter-edge commented Oct 29, 2015

I think that is a separate issue, not relevant to this one :)

(also in your compile there, you have to have -I to the base protobuf src directory, which is usually /usr/include on linux machines, /usr/local/include on macs if you installed protobuf with homebrew, or just /path/to/github.com/google/protobuf/src if you clone https://github.com/google/protobuf)

DIR=/usr/include
protoc -I $DIR $DIR/google/protobuf/any.proto $DIR/google/protobuf/source_context.proto $DIR/google/protobuf/type.proto

(or just use go.pedge.io/google-protobuf for now, which is what this issue is trying to resolve :) )

@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Dec 1, 2015

@grepory I did this https://go.pedge.io/protoeasy mostly this weekend, you might find it interesting regarding this specifically :)

peter-edge commented Dec 1, 2015

@grepory I did this https://go.pedge.io/protoeasy mostly this weekend, you might find it interesting regarding this specifically :)

@grepory

This comment has been minimized.

Show comment
Hide comment
@grepory

grepory commented Dec 1, 2015

😽

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Dec 10, 2015

It would be really nice to have those packages compiled

LK4D4 commented Dec 10, 2015

It would be really nice to have those packages compiled

@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Jan 17, 2016

@grepory @LK4D4 I did some more fun stuff since this has still been an issue: https://go.pedge.io/pb

  • Has everything in go.pedge.io/google-protobuf
  • Has everything in go.pedge.io/googleapis
  • Also has my personal stdlib for protobuf, under the "pb" packages
  • Also has a lot of fun around countries/currencies/money, including all iso3166 / iso4217 codes generated as protobuf enums, a money package with math operations
  • Also works for both golang/protobuf and gogo/protobuf

Thought you would be interested.

peter-edge commented Jan 17, 2016

@grepory @LK4D4 I did some more fun stuff since this has still been an issue: https://go.pedge.io/pb

  • Has everything in go.pedge.io/google-protobuf
  • Has everything in go.pedge.io/googleapis
  • Also has my personal stdlib for protobuf, under the "pb" packages
  • Also has a lot of fun around countries/currencies/money, including all iso3166 / iso4217 codes generated as protobuf enums, a money package with math operations
  • Also works for both golang/protobuf and gogo/protobuf

Thought you would be interested.

@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Feb 5, 2016

Hey, any update on this? There's a lot of things where this would be really nice, and I'm starting to have to do custom hacks to jsonpb where if the well-known types were just a part of this package, it would be much better.

https://github.com/libopenstorage/openstorage/tree/master/pkg/jsonpb

Again, have it all set up at https://go.pedge.io/pb, and am more than happy to send over a PR that moves this over here.

peter-edge commented Feb 5, 2016

Hey, any update on this? There's a lot of things where this would be really nice, and I'm starting to have to do custom hacks to jsonpb where if the well-known types were just a part of this package, it would be much better.

https://github.com/libopenstorage/openstorage/tree/master/pkg/jsonpb

Again, have it all set up at https://go.pedge.io/pb, and am more than happy to send over a PR that moves this over here.

dsymonds added a commit that referenced this issue Feb 10, 2016

Add well-known types.
This introduces supported Go packages for each well-known type,
and a placeholder support package for interacting with them.

This commit adds Any, Duration and Empty; more types will follow.

Updates #50.
@dsymonds

This comment has been minimized.

Show comment
Hide comment
@dsymonds

dsymonds Feb 10, 2016

Member

8ea33d2 adds some initial types. I'll add more shortly.

Member

dsymonds commented Feb 10, 2016

8ea33d2 adds some initial types. I'll add more shortly.

@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Feb 10, 2016

Why are they in different packages/is the go package there the same as in
Google/protobuf?

On Wednesday, February 10, 2016, David Symonds notifications@github.com
wrote:

8ea33d2
8ea33d2
adds some initial types. I'll add more shortly.


Reply to this email directly or view it on GitHub
#50 (comment).

peter-edge commented Feb 10, 2016

Why are they in different packages/is the go package there the same as in
Google/protobuf?

On Wednesday, February 10, 2016, David Symonds notifications@github.com
wrote:

8ea33d2
8ea33d2
adds some initial types. I'll add more shortly.


Reply to this email directly or view it on GitHub
#50 (comment).

@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Feb 10, 2016

I would argue for there to be zero diff from Google/protobuf, especially as
this is what other languages use, and names like any.Any, empty.Empty, etc,
are repeating.

On Wednesday, February 10, 2016, Peter Edge peter.edge@gmail.com wrote:

Why are they in different packages/is the go package there the same as in
Google/protobuf?

On Wednesday, February 10, 2016, David Symonds <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

8ea33d2
8ea33d2
adds some initial types. I'll add more shortly.


Reply to this email directly or view it on GitHub
#50 (comment).

peter-edge commented Feb 10, 2016

I would argue for there to be zero diff from Google/protobuf, especially as
this is what other languages use, and names like any.Any, empty.Empty, etc,
are repeating.

On Wednesday, February 10, 2016, Peter Edge peter.edge@gmail.com wrote:

Why are they in different packages/is the go package there the same as in
Google/protobuf?

On Wednesday, February 10, 2016, David Symonds <notifications@github.com
javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

8ea33d2
8ea33d2
adds some initial types. I'll add more shortly.


Reply to this email directly or view it on GitHub
#50 (comment).

@dsymonds

This comment has been minimized.

Show comment
Hide comment
@dsymonds

dsymonds Feb 10, 2016

Member

There's zero diff from the google/protobuf .proto files (except for adding a go_package option to make protoc-gen-go generate a good package name, which I hope to upstream).

I put them in separate Go packages since that seemed to make more sense to me, and couldn't think of compelling reasons for them to all be in the same package (their existing package declarations notwithstanding). But I'm happy to hear arguments for them all going together.

Member

dsymonds commented Feb 10, 2016

There's zero diff from the google/protobuf .proto files (except for adding a go_package option to make protoc-gen-go generate a good package name, which I hope to upstream).

I put them in separate Go packages since that seemed to make more sense to me, and couldn't think of compelling reasons for them to all be in the same package (their existing package declarations notwithstanding). But I'm happy to hear arguments for them all going together.

@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Feb 10, 2016

Let me think about it overnight? I'll try to come up with an argument,
otherwise ok :)

On Wednesday, February 10, 2016, David Symonds notifications@github.com
wrote:

There's zero diff from the google/protobuf .proto files (except for adding
a go_package option to make protoc-gen-go generate a good package name,
which I hope to upstream).

I put them in separate Go packages since that seemed to make more sense to
me, and couldn't think of compelling reasons for them to all be in the same
package (their existing package declarations notwithstanding). But I'm
happy to hear arguments for them all going together.


Reply to this email directly or view it on GitHub
#50 (comment).

peter-edge commented Feb 10, 2016

Let me think about it overnight? I'll try to come up with an argument,
otherwise ok :)

On Wednesday, February 10, 2016, David Symonds notifications@github.com
wrote:

There's zero diff from the google/protobuf .proto files (except for adding
a go_package option to make protoc-gen-go generate a good package name,
which I hope to upstream).

I put them in separate Go packages since that seemed to make more sense to
me, and couldn't think of compelling reasons for them to all be in the same
package (their existing package declarations notwithstanding). But I'm
happy to hear arguments for them all going together.


Reply to this email directly or view it on GitHub
#50 (comment).

@dsymonds

This comment has been minimized.

Show comment
Hide comment
@dsymonds

dsymonds Feb 11, 2016

Member

Any arguments, @peter-edge?

Member

dsymonds commented Feb 11, 2016

Any arguments, @peter-edge?

@peter-edge

This comment has been minimized.

Show comment
Hide comment
@peter-edge

peter-edge Feb 12, 2016

Nope :)

On Friday, February 12, 2016, David Symonds notifications@github.com
wrote:

Any arguments, @peter-edge https://github.com/peter-edge?


Reply to this email directly or view it on GitHub
#50 (comment).

peter-edge commented Feb 12, 2016

Nope :)

On Friday, February 12, 2016, David Symonds notifications@github.com
wrote:

Any arguments, @peter-edge https://github.com/peter-edge?


Reply to this email directly or view it on GitHub
#50 (comment).

@dsymonds

This comment has been minimized.

Show comment
Hide comment
@dsymonds

dsymonds Feb 12, 2016

Member

Okay, I'll leave it as I have it now. Neither approach is perfect, and we can consider changing it in the future, but the current state should be fine.

Member

dsymonds commented Feb 12, 2016

Okay, I'll leave it as I have it now. Neither approach is perfect, and we can consider changing it in the future, but the current state should be fine.

@azr

This comment has been minimized.

Show comment
Hide comment
@azr

azr May 16, 2017

About date/durations,

how about creating some func to get/set timestamps from golang time ?

Like func NewTimestamp(time.Time) *Timestamp and func (t *Timestamp) Time() time.Time & same for Durations ?

Directly in the timestamp pkg, I think it's fine if it's not generated.

azr commented May 16, 2017

About date/durations,

how about creating some func to get/set timestamps from golang time ?

Like func NewTimestamp(time.Time) *Timestamp and func (t *Timestamp) Time() time.Time & same for Durations ?

Directly in the timestamp pkg, I think it's fine if it's not generated.

@cybrcodr

This comment has been minimized.

Show comment
Hide comment
@cybrcodr

cybrcodr May 16, 2017

Contributor

I think you are looking for...
https://godoc.org/github.com/golang/protobuf/ptypes#Timestamp
https://godoc.org/github.com/golang/protobuf/ptypes#TimestampProto

Btw, I don't think your comment is related to the original issue.

Contributor

cybrcodr commented May 16, 2017

I think you are looking for...
https://godoc.org/github.com/golang/protobuf/ptypes#Timestamp
https://godoc.org/github.com/golang/protobuf/ptypes#TimestampProto

Btw, I don't think your comment is related to the original issue.

@azr

This comment has been minimized.

Show comment
Hide comment
@azr

azr May 24, 2017

Ah, thx @cybrcodr.
I put it here bcs #49 redirects here, not that much related, yes, sry ! Cheers !

azr commented May 24, 2017

Ah, thx @cybrcodr.
I put it here bcs #49 redirects here, not that much related, yes, sry ! Cheers !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment