Skip to content
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

Python packaging/namespacing #22

Closed
bobvanderlinden opened this issue Aug 24, 2015 · 8 comments
Closed

Python packaging/namespacing #22

bobvanderlinden opened this issue Aug 24, 2015 · 8 comments

Comments

@bobvanderlinden
Copy link
Contributor

As discussed in bobvanderlinden/pymachinetalk-client#1, it would be nice to have namespacing/directory structure so that machinetalk-protobuf can be packaged on its own without conflicting with other packages.

The protobuf library itself is an example of how to do the namespacing/packaging and how to handle each language: https://github.com/google/protobuf

@bobvanderlinden
Copy link
Contributor Author

I've tried to replicate this for Python, so that a package can be generated using setup.py. This should make it possible to put it on pypi, so that people can just pip install machinetalk-protobuf. It is not yet ready, since the Makefile is now broken, but I'll handle that next.

bobvanderlinden@d16c8aa

@mhaberler
Copy link
Contributor

wow ;)

@bobvanderlinden
Copy link
Contributor Author

Haha, thanks! It should be very similar to https://github.com/google/protobuf/tree/master/python and https://github.com/google/protobuf/tree/master/python/setup.py. But, it is possible to install setup.py to a virtualenv (or globally on the system) and use it thereafter with from machinetalk_protobuf.message_pb2 import Container (yay).

@bobvanderlinden
Copy link
Contributor Author

I did some more work on getting things running properly on my system: https://github.com/bobvanderlinden/machinetalk-protobuf

This version fixes the makefile for C++ and the dependency/object files. The C++ headers can be included using "machinetalk/protobuf/message.pb.h", instead of just "message.pb.h". This is the same convention that is used for Protobuf itself: "google/protobuf/message.h". We might want to go for "machinetalk/protobuf/message.h" (dropping ".pb" from the filename), but it doesn't have any significant benefit.

Also, I think it might be a good idea to compile the .cc files into a static/shared object that can be linked to. It seems protobuf itself also does this, but I'm not sure whether this has negative consequences.

I still need to add an install rule to install all files in the correct places on the system.

I wasn't entirely sure where the dependency files (.d) were used for. Since I couldn't generate those earlier I thought they were meant to be d source code files (the D programming language). What are they exactly used for?

The changes should also work for Javascript, but I have not tried this yet, because I lack proto2js. It seems proto2js is also deprecated for protobuf.js 4: https://github.com/dcodeIO/ProtoBuf.js/wiki/proto2js. From what I can see in the documentation, you'd usually not compile the .proto files into .js files, but just load the .proto files at runtime using the protobuf.js library.

Compiling the .js files is also a bit tricky, because it depends on the environment how dependencies should be loaded. nodejs uses commonjs for dependencies. The browser could use AMD/RequireJS, commonjs, or whatever else people came up with. There isn't one way to do this, so I'm leaning towards dropping JS-file generation.

@zultron
Copy link

zultron commented Aug 30, 2015

On 08/29/2015 08:07 AM, Bob van der Linden wrote:

I wasn't entirely sure where the dependency files (|.d|) were used for.
Since I couldn't generate those earlier I thought they were meant to be
d source code files (the D programming language). What are they exactly
used for?

Those are dependency files. They're sourced in the Makefile, and help
make figure out when to rebuild after a header file changes.

@bobvanderlinden
Copy link
Contributor Author

Ah, nice. This wasn't working correctly in my version, but that problem is now fixed.

On that topic, could you take a look at #23? Generating the .d files wasn't working on my machine because of a text<->bytes problem. It is a different problem than the namespacing, so I created a new PR for that.

Also, I've added a install rule that'll basically copy the .proto files to /usr/local/include (or whatever DESTDIR is set to). It'll also install the .h files, but does not yet compile the .c files into a .so/.a file (and so does not yet install the .so/.a). I have not yet changed the package names (package pb; to package machinetalk.protobuf;). Let me know if this is indeed the way to go, or that there are still problems with nanopb?

Lastly, I've started on a node API for machinetalk. It was much easier to set this up compared to Python, because all required libraries (mdns/zeroconf, zmq and protobuf) were already there. (In Python I had the problem that the libraries weren't cooperating correctly). It does discovery, it connects and it reads out full and incremental task-status updates. The repository can be found here: https://github.com/bobvanderlinden/node-machinetalk

@mhaberler
Copy link
Contributor

what ?! one day later and you're over the hurdles ;-?

great!

@bobvanderlinden
Copy link
Contributor Author

I feel like this issue covers a number of issues, so I have split this issue into #27 (package namespacing), #24 (make install), #25 (NodeJS package) and #26 (Python package). I'll close this one so that discussion can focus on a single issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants