Why in the world has this travis build been waiting for 12 hours to start?

I don't know why.

I just made quite a disrupting change. Maybe you want to merge this into your branch.

@awalterschulze Will do

Now then, all the oneof compare tests are still failing, with 1 != -1. I think there's an assumption somewhere in the compare tests that is being challenged. Any ideas?

I'm not sure I understand exactly what the compare plugin return values mean. I think s1.Compare(s2) returning 1 means they are not the same, and returning 0 would mean they are, but what does returning -1 mean? Since that is what the Compare test expects.

does: negative if the first argument is “less” than the second, zero if they are “equal”, and positive if the first argument is “greater”

https://www.gnu.org/software/libc/manual/html_node/Comparison-Functions.html

The idea is that -1 is less than and +1 is greater than.
This way you can easily write a Less method in one line, given the generated Compare method.

There might be a wrong assumption.
You are absolutely right, but I haven't had time to check yet.

I see, thanks for the explanation, I wasn't aware of this general rule of GNU comparison functions. I'll take a look through the oneof parts and see if there's somewhere obvious where s1.Compare(s2) != -1*s2.Compare(s1).

Please tell me if you need help here.

Yeah, I've put this to the side for now, so I would appreciate any pointers you have.

To be explicit: Yes I need help.

Don't worry I got that.
I had a quick look this morning and it wasn't obvious.
Now I just need to find some proper time.

Cheers, no rush, just thought I wasn't explicit enough last time 😄

No problem :)

The problem happens here

func (this *AllTypesOneOf_Field1) Compare(that interface{}) int {
    if that == nil {
        if this == nil {
            return 0
        }
        return 1
    }

    that1, ok := that.(*AllTypesOneOf_Field1)
    if !ok {
        that2, ok := that.(AllTypesOneOf_Field1)
        if ok {
            that1 = &that2
        } else {
            panic("not this one")
            return 1
        }
    }

When oneof types dont match each of them will return 1, instead of checking whether the type has a smaller or bigger field number (or something else deterministic).

Are you still interested in trying to fix this?

It will require a bit of work in plugin/compare/compare.go

I would like to but unfortunately I don't think I'll have time anytime soon - please go ahead and make the changes yourself if you get the time.

I am still thinking about how to do this.
I mean in this case we have all the types and can generate code that checks if one type is smaller than the other.
But its also interesting to think about what a not protobuf generated Compare method would do.
If you have to compare two objects of type interface{}
How would you do this in a consistent manner.

If the types are the same you can simply compare the values, but in the case where the types are different, hmmm.
I thought about using the pointer value. This would give me a consistent result for those specific instances, but it would result in inconsistent behaviour when the values are the same inside different instances of objects.

Any decision on how best to solve this?

How about a design something like this
https://play.golang.org/p/88SJej8go6
?

But using unsafe is not really an option.
So I guess I am still thinking.

Ok we have to generated a type switch for each possible one of type for this and that.
This will then assign the oneof field number for each type to a thisType+NumGen.Next() and thatType+NumGen.Next() variable.
Then we compare the numbers, if they are equal, the Compare method for that field is called.
If they differ we return 1 or -1 as appropriate.
This is the design.
Are you keen to program it?

I wont have any time, feel free to commandeer this pull request if you can find the time. Thanks!