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

Namespace might conflict with other packages #27

Closed
bobvanderlinden opened this issue Sep 12, 2015 · 3 comments
Closed

Namespace might conflict with other packages #27

bobvanderlinden opened this issue Sep 12, 2015 · 3 comments

Comments

@bobvanderlinden
Copy link
Contributor

The namespace pb might conflict with other packages as it is too generic. Protobuf uses a namespace that is more descriptive and unique (google.protobuf). When we want to install the protobuf files into the right directories, we need to make sure it doesn't overlap with other packages.

I do not know what the consequences are for nanopb, but I think this is the right place to discuss that.

I think this needs to be done before we roll out separate packages as described in #24, #25 and #26.

@mhaberler
Copy link
Contributor

makes sense, so let's go through the shopping list:

  • fix issue need to add syntax specification to all .proto files #28 so we're safe for proto3 (unrelated, but comes in one maintenance day ;)
  • protoc-generated Python files to to lib/python/machinetalk (currently lib/python)
  • do we need an empty init.py file there?
  • using code: this currently affects Alex's stuff and lib/python/hal_glib.y; fallout detection by grep -r import .|grep _pb2; I can take those

C++ fallout (1) - dereferenced names: `grep -r namespace .|grep 'pb;'

./machinetalk/support/encdec.cc:using namespace pb;
./machinetalk/support/position.cc:using namespace pb;

easy, I can take that - should go for fully qualified names in C++ from now on

C++ fallout (2) - fully qualified names
$ grep -lr pb::Container .|wc -l
35

will take that, single emacs mass reedit

nanopb C fallout - not much, can take that

Since the wireformat doesnt change, that change is local to the machinekit repo

@Strahlex - what would be involved on your side to make QtQuick VCP build from a mutated machinetalk-protobuf branch? will your source be backwards compatible or is that forward only?

I think step 1 would be to temporarily introduce a branch in this repo which has the new namespace conventions, then merge that into machinekit as a subtree - that enables fixing the mk repo

independently Alex can have a got at it, until then keeps using the current branch

depending on results we might have to do a synced upgrade, or a independent one

@machinekoder
Copy link
Member

@mhaberler The code should be pretty much compatible since I do not use the Makefile to generate my protobuf classes anyway.

I think we can go forward on this topic as it simplifies using Machinetalk on non-Machinekit computers significantly:
#36

@machinekoder
Copy link
Member

Has been merged.

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