Code Generator produces messages with broken dependencies

[bkerley]

This is part of the output of running the generator from 67f4894 against https://github.com/basho/riak_pb/blob/master/src/riak_kv.proto :

class RpbContent
  include Beefcake::Message


  required :value, :bytes, 1
  optional :content_type, :bytes, 2
  optional :charset, :bytes, 3
  optional :content_encoding, :bytes, 4
  optional :vtag, :bytes, 5
  repeated :links, RpbLink, 6
  optional :last_mod, :uint32, 7
  optional :last_mod_usecs, :uint32, 8
  repeated :usermeta, RpbPair, 9
  repeated :indexes, RpbPair, 10
  optional :deleted, :bool, 11

end

class RpbLink
  include Beefcake::Message


  optional :bucket, :bytes, 1
  optional :key, :bytes, 2
  optional :tag, :bytes, 3

end

The issue is that RpbContent.links cannot be defined, because it depends on the RpbLink class that hasn't been read yet.

What I'll try and do this week is find a way to create all the classes up front, so they all exist when they try to get tagged in messages.

[bkerley]

This includes and supersedes #27 .

[matttproud]

Hi, @bkerley! @bmizerany and I spoke a week ago about maintainership, and we have transitioned Beefcake to the @protobuf-ruby group on GitHub.

I would like to review your pull request, but it looks out of date. Can you rebase?

[matttproud]

HEAD does some of this now, so you'll definitely want to rebase.

[matttproud]

Hey, so overall this looks OK. Would you be willing to include inline a before-and-after of the generated code or include them in Gist for review? I would like to compare this a bit more precisely.

Without having the diff, I am curious, why not output the messages with a topological sort instead of defining the class specification a priori?

[bkerley]

Just force-pushed the rebase, will generate before & after.

[bkerley]

https://gist.github.com/bkerley/6284225 contains before and after. The issue this branch fixes is at https://gist.github.com/bkerley/6284225#file-before-riak_kv-pb-rb-L221

[bkerley]

Taking a look at the jruby failure on travis: https://travis-ci.org/protobuf-ruby/beefcake/jobs/10417342

[matttproud]

So quick practical question: Why not use TSort (http://www.ruby-doc.org/stdlib-2.0/libdoc/tsort/rdoc/TSort.html) to sort the messages by the message types of their child fields? You'd emit less code to the user that way.

[bkerley]

TSort doesn't cover the mutual recursion case, where two or more messages can refer to each other.

[matttproud]

Interesting. I did not know that was legal with Protocol Buffer definitions, and I don't think I've ever seen that before (past life as a Googler). Cool. That part looks fine, then.

[matttproud]

Just a few open questions in https://github.com/protobuf-ruby/beefcake/pull/28/files.

[matttproud]

You will need to update this to github.com/protobuf-ruby/beefcake.

[matttproud]

Almost ready for submission. Just take care of the last simple remarks, and we're good to merge.

[matttproud]

LGTM. Thank you for your work and patience and thoroughness!