> On April 14, 2014, 5:09 a.m., Benjamin Hindman wrote: > > include/mesos/containerizer.proto, lines 23-24 > > <https://reviews.apache.org/r/20141/diff/3/?file=555024#file555024line23> > > > > Sorry Till, I wasn't explicit enough about this refactor. Let's put > > this file in 'include/mesos/containerizer/containerizer.proto'. Then we can > > make the Java package match the C++ one, i.e., > > 'org.apache.mesos.containerizer'. Then the outer classname can be > > consistent with the others and just be 'Protos'.
Ah, ok. Makes sense. > On April 14, 2014, 5:09 a.m., Benjamin Hindman wrote: > > src/Makefile.am, lines 108-110 > > <https://reviews.apache.org/r/20141/diff/3/?file=555025#file555025line108> > > > > Now this should be able to be just: > > > > JAVA_PROTOS = \ > > java/generated/org/apache/mesos/Protos.java \ > > java/generated/org/apache/mesos/containerizer/Protos.java > > > > > > And then below you should be able to define the Java targets like what > > was done for C++ and what you did for Python. I may be missing something but IIUC, then using a subfolder for that containerizer.proto does force me to add more rules like: [...] containerizer/%.pb.cc containerizer/%.pb.h: $(CONTAINERIZER_PROTO) $(MKDIR_P) $(@D) $(PROTOC) $(PROTOCFLAGS) --cpp_out=. $^ java/generated/org/apache/mesos/Protos.java: $(MESOS_PROTO) $(MKDIR_P) $(@D) $(PROTOC) $(PROTOCFLAGS) --java_out=java/generated $^ java/generated/org/apache/mesos/containerizer/Protos.java: $(CONTAINERIZER_PROTO) $(MKDIR_P) $(@D) $(PROTOC) $(PROTOCFLAGS) --java_out=java/generated $^ python/src/mesos_pb2.py: $(MESOS_PROTO) $(MKDIR_P) $(@D) $(PROTOC) $(PROTOCFLAGS) --python_out=python/src $^ python/src/containerizer/containerizer_pb2.py: $(CONTAINERIZER_PROTO) $(MKDIR_P) $(@D) $(PROTOC) $(PROTOCFLAGS) --python_out=python/src $^ I will also have to adapt setup.py to cover subpackages: setup(name = 'mesos', version = '0.19.0', description = 'Mesos', package_dir = { '': 'src' }, packages = ['.', 'containerizer'], install_requires = ['protobuf>=2.5.0'], ext_modules = [mesos_module]) I would be happy to do so, but please confirm that this is what you aim for. > On April 14, 2014, 5:09 a.m., Benjamin Hindman wrote: > > src/Makefile.am, lines 214-220 > > <https://reviews.apache.org/r/20141/diff/3/?file=555025#file555025line214> > > > > You'll want to update these with the new containerizer stuff too so > > that it gets installed. And feel free to create a 'containerizer.hpp' which > > wraps containerizer.pb.h just as we've done with 'mesos.hpp'. Yes, that makes sense. > On April 14, 2014, 5:09 a.m., Benjamin Hindman wrote: > > src/slave/containerizer/containerizer.hpp, line 24 > > <https://reviews.apache.org/r/20141/diff/3/?file=555026#file555026line24> > > > > As of now this isn't being installed and the convention is that we only > > use <> for things that get installed. > > > > Also, if you create a wrapper then you can use it here instead, i.e., > > 'mesos/containerizer/containerizer.hpp' (as we do with mesos/mesos.hpp). Silly me :) .... should be fixed by introducing the wrapping header which will get installed, so this becomes #include <mesos/containerizer/containerizer.hpp> - Till ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20141/#review40228 ----------------------------------------------------------- On April 11, 2014, 2:03 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20141/ > ----------------------------------------------------------- > > (Updated April 11, 2014, 2:03 a.m.) > > > Review request for mesos, Benjamin Hindman and Niklas Nielsen. > > > Repository: mesos-git > > > Description > ------- > > Adds containerizer.proto containing protobuf message definitions for > "Launch", "Update" and "Termination". All of those are used in the upcoming > External Containerizer. > These protos are defined within their own C++ namespace: "containerizer". > > Replaced slave::Containerizer::Termination with the > containerizer::Termination protobuf, gaining a serializable Termination > message. > > The protoc results are added towards the Python Egg (containerizer_pb2.py) > and the Java package (ContainerizerProtos.java). > > > Diffs > ----- > > include/mesos/containerizer.proto PRE-CREATION > src/Makefile.am 95f133d > src/slave/containerizer/containerizer.hpp d9ae326 > src/slave/containerizer/mesos_containerizer.hpp ee1fd30 > src/slave/containerizer/mesos_containerizer.cpp 1ce41d7 > src/slave/slave.hpp 08f6005 > src/slave/slave.cpp cddb241 > src/tests/containerizer.hpp a9f1531 > src/tests/containerizer.cpp bfb9341 > > Diff: https://reviews.apache.org/r/20141/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
