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

Install protobuf from cmake project #673

Merged
merged 6 commits into from
Aug 13, 2015

Conversation

podsvirov
Copy link
Contributor

Additional export as "protobuf" package
for importing from other cmake projects

Testing binaries in draft pre-release:
https://github.com/podsvirov/protobuf/releases/tag/v3.0.0-5500b0

Importing example avaliable there:
https://github.com/podsvirov/impprotobuf

Additional export as "protobuf" package
for importing from other cmake projects
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@podsvirov
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

option(BUILD_TESTING "Build tests" ON)
option(BUILD_SHARED_LIBS "Build Shared Libraries" OFF)
if (MSVC)
option(ZLIB "Build with zlib support" OFF)
endif (MSVC)

# Path to common header
set(protobuf_COMMON_HEADER "../src/google/protobuf/stubs/common.h")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract the version number from configure.ac instead?
https://github.com/google/protobuf/blob/master/configure.ac#L15

common.h only has version number like 3000000, but we probably want something like 3.0.0-alpha-4.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Jul 31, 2015

Thanks very much for the change! I was thinking adding installation support but didn't know how to do that.

Also does this PR fix #578?

@podsvirov
Copy link
Contributor Author

Hi xfxyjwf!

I still do not know much protobuf structure of the project, but not bad versed in CMake.
About half a year ago, I, too, tried to port to CMake protobuf but the project stalled.

On account configure.as think you are right and it is better to place to parse the version of the project. If I understand correctly, but this is the main entry point.

Initially, in the version I was looking for just a number. CMake using 4 numbers (MAYOR.MINOR.PATCH.TWEAK).

However, additional information may also be useful.

To CMake project and export the package need only numbers.

I'll get a modification of the algorithm search numbers from configure.ac.

Of course all this can not affect the module FindProtobuf.

There are still questions arise naming conventions ... How best to call
package. Large or small letters are used, and so on.

@xfxyjwf
Copy link
Contributor

xfxyjwf commented Aug 13, 2015

Hi @podsvirov, sorry for the late response. I haven't got time to try the added script on each of the supported platforms but the change itself looks good to me. It seems this PR also exposes some public functions for users to use (PROTOBUF_GENERATE_CPP?). Could you help add some documentation in the cmake/README.md file for it? I'm merging this PR as I don't want to block you further. Thanks again for helping with this change!

xfxyjwf added a commit that referenced this pull request Aug 13, 2015
Install protobuf from cmake project
@xfxyjwf xfxyjwf merged commit f818183 into protocolbuffers:master Aug 13, 2015
@podsvirov
Copy link
Contributor Author

Excellent! The beginning of a great friendship :-)And this means that the front is big but interesting work!Of course, that these changes should be reflected in cmake/README.md, but there are still a few comments.So other developers could use Protobuf in your projects running CMakewe should take care about creating a "Relocatable Package". Now this is not always the case.
I have questions regarding the use of ZLib:
(a) something wrong with the search package for ZLib MSVC. I see not a standard behavior. Don't need to do.
(b) When linking ZLib needs to use the imported target ZLIB::ZLIB.
If there are no objections, I soon will be doing it and create another PR.
Plans for future developments:
On the basis of this work may forgive in the world following topics on which I work:
https://github.com/podsvirov/grpc/tree/topic-cmake-project
https://github.com/podsvirov/grpc-common/tree/topic-cmake-project
Then the CMake community will increasingly use a bunch CMake-Protobuf-gRPC!

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

Successfully merging this pull request may close these issues.

3 participants