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

PROTON-1330: [python] bundle the C source in the python source distri… #85

Closed
wants to merge 1 commit into from

Conversation

kgiusti
Copy link
Contributor

@kgiusti kgiusti commented Nov 7, 2016

No description provided.

@astitcher
Copy link
Member

A couple of comments:

  • It seems entirely arbitrary (to me anyway) to move generating encoding.h and protocol.h from the C build process to generating them when producing the C part of the python install. We know that we have python available at the time of build the C code (well duh!) so we could keep on generating at C build time when we do the python install.

This seems to me to minimise the divergence between the packaged C source for the python package and the real C source tree.

  • Given the c code built by the python install is somewhat selective anyway - it makes sense (again to me anyway) only to package C source which is actually used in the library used by the python installation.

@kgiusti
Copy link
Contributor Author

kgiusti commented Nov 14, 2016

re: encoding.h and protocol.h:

I didn't want to duplicate the generation of these files in the python CMakeLists.txt but as it stands now these files are generated during the build phase - not the cmake configuration pass. I can either 1) require the C compilation is done prior to building the Python source distribution or 2) move the file generation to the cmake configuration phase.

re: packaging C source

I'm not entirely sure I understand - are you suggesting some of the C files are not needed by the Python bindings? I assume I can hunt around for these files, but isn't all Proton/C functionality accessible via the python bindings, since the binding is used by the python unit tests?

@kgiusti
Copy link
Contributor Author

kgiusti commented Nov 15, 2016

I've got a different approach that may address the issues you've raised Andrew.

@kgiusti kgiusti closed this Nov 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants