If you want to see what the FileDescriptorProto looks like:

package main

import (
    "bytes"
    "compress/gzip"
    "fmt"
    "io/ioutil"
    "os"

    "go.pedge.io/lion/proto"

    "github.com/golang/protobuf/proto"
    "github.com/golang/protobuf/protoc-gen-go/descriptor"
)

func main() {
    if err := do(); err != nil {
        fmt.Fprintf(os.Stderr, "%s\n", err.Error())
        os.Exit(1)
    }
    os.Exit(0)
}

func do() error {
    // golang/protobuf has gzipped file descriptors built in
    // https://github.com/golang/protobuf/blob/6aaa8d47701fa6cf07e914ec01fde3d4a1fe79c3/protoc-gen-go/descriptor/descriptor.pb.go#L1707
    fileDescriptor, _ := (&descriptor.FileDescriptorSet{}).Descriptor()
    gzipReader, err := gzip.NewReader(bytes.NewReader(fileDescriptor))
    if err != nil {
        return err
    }
    data, err := ioutil.ReadAll(gzipReader)
    if err != nil {
        return err
    }
    message := &descriptor.FileDescriptorProto{}
    if err := proto.Unmarshal(data, message); err != nil {
        return err
    }
    // my logging library that has specific printing of protocol buffer messages
    protolion.Print(message)
    return nil
}

Are you ok with giving your copyright over to us for this pull request?

Ya of course

The copyright specifically for this PR, that is

yes

I think we can do better.

This is an improvement over the existing code, is there no reason not to
merge?

On Monday, February 1, 2016, Walter Schulze [email protected]
wrote:

I think we can do better.

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

If you have an idea for something better, please tell me, otherwise I'm not
sure how to proceed here.

On Monday, February 1, 2016, Peter Edge [email protected] wrote:

This is an improvement over the existing code, is there no reason not to
merge?

On Monday, February 1, 2016, Walter Schulze <[email protected]
javascript:_e(%7B%7D,'cvml','[email protected]');> wrote:

I think we can do better.

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

What about a filepath.Split ?

How is that different? Outlawing cases like "protodescriptor.proto"?

Much better :)
Now I am just wondering if its correct.
Have you ran it manually on a descriptor.proto and another google.protobuf package to manually test if it is actually solving your problem?

I'm sure it is, but I can test it to double check.

On Tue, Feb 2, 2016 at 4:38 PM, Walter Schulze [email protected]
wrote:

Much better :)
Now I am just wondering if its correct.
Have you ran it manually on a descriptor.proto and another google.protobuf
package to manually test if it is actually solving your problem?

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

Never be sure. Always test.

I have confirmed that this fixes the issue with a manual compile

@awalterschulze just checking in, that means that yes, i tested it and i confirmed it works :)

Yes sorry my bad. I have not forgotten about this pull request, but I have been quite busy and I also wanted to test this myself.
But now I am thinking about this change again.

I first want to see what golang/protobuf does with its new proto3 google/protobuf types.
So I think we should wait for
golang/protobuf#50

This also links to proto3 #57 not being fully supported yet in gogoprotobuf.

I don't want to make a change now that I will possibly have to revert.
So we can keep this pull request open until golang/protobuf has fixed issue 50 and I have designed how gogoprotobuf will play with these new types.
Sorry for the inconvenience.

Protobuf/50 is irrelevant to this PR man. Anything that happens there has
no effect here. This is really frustrating as a contributor, both that this
work is getting rejected after all this, and that you are rejecting it for
a reason that isn't actually true.

On Tuesday, February 9, 2016, Walter Schulze [email protected]
wrote:

Yes sorry my bad. I have not forgotten about this pull request, but I have
been quite busy and I also wanted to test this myself.
But now I am thinking about this change again.

I first want to see what golang/protobuf does with its new proto3
google/protobuf types.
So I think we should wait for
golang/protobuf#50 golang/protobuf#50

This also links to proto3 #57 #57
not being fully supported yet in gogoprotobuf.

I don't want to make a change now that I will possibly have to revert.
So we can keep this pull request open until golang/protobuf has fixed
issue 50 and I have designed how gogoprotobuf will play with these new
types.
Sorry for the inconvenience.

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

Do the types in Protobuf/50 not also have a package name "google.protobuf" ?

Yep, but this isn't affected by this PR - the compiler will see no
difference. Hit me up on hangouts if you want to chat quickly? Heh.

On Tuesday, February 9, 2016, Walter Schulze [email protected]
wrote:

Do the types in Protobuf/50 not also have a package name "google.protobuf"
?

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

I am happy to talk like this.
With this change the compiler will now possibly generate extra code for these types, which I have not decided is a thing, yet.

But that does not have to do with this PR - even if it does, then this
merely allows them to be compiled. This does nothing to prevent or
improperly prevent that.

On Tue, Feb 9, 2016 at 12:18 PM, Walter Schulze [email protected]
wrote:

I am happy to talk like this.
With this change the compiler will now possibly generate extra code for
these types, which I have not decided is a thing, yet.

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

I don't know what to say anymore, but please be patient.
Why can't we just wait.

We can, it's certainly your decision, but we're having a fundamental
miscommunication and understanding here. You are saying to wait for issue
50, and I'm trying to point out that issue 50 is irrelevant, ie if that's
the reason to wait, I think this code shows that there is no reason.

On Tue, Feb 9, 2016 at 12:24 PM, Walter Schulze [email protected]
wrote:

I don't know what to say anymore, but please be patient.
Why can't we just wait.

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

I think they are connected, maybe you can show me how given a new proto3 type you do not change the generated code with this change.

The only change here is that anything in google.protobuf that is not in descriptor.proto will be allowed to have special gogoprotobuf code generated for it. This means, for example, that google.protobuf.Timestamp will have the additional Marshal/Unmarshal related methods (like Size) attached to it.

No other code, or generated code, is affected. This actually fixes a bug, where if you use gofast/gogofast/gogofaster/gogoslick, that if you have a message such as this:

import "google/protobuf/timestamp.proto";

message Foo {
  Timestamp timestamp = 1;
}

And you use gogofast etc, that correct code will actually be produced. Otherwise you will get a bug saying that no method for Size can be found on Timestamp.

The only generated code that changes is if you generate code for google.protobuf.* that is not in descriptor, which should change regardless of issue 50 (in fact, for issue 50 to work when you merge in the changes from golang/protobuf, you will need this PR to produce valid code with gogofast etc).

I can guess how golang/protobuf is going to handle these new proto3 types, but I can not know for sure.
gogoprotobuf does not handle these types yet, because it is waiting for golang/protobuf issue 50.
gogoprotobuf will possibly make exactly the change as proposed in this pull request, but it can not know for sure.
This definitely changes how timestamp.proto will be generated, as you have just said, and that is the reason I do not want to merge this until I am sure how gogoprotobuf is going to handle these types.

@awalterschulze
Could we perhaps make this a flag? This is now the only reason why we maintain a fork of Gogo, since we use Timestamp in our APIs.

I am busy merging in ptypes, when that is done I'll revisit this issue.
#155
But my work in currently being blocked.
Will resume it as soon as it is unblocked.

Great, would greatly appreciate it :)