Build examples in CMake #1643

yeswalrus · 2016-06-03T02:28:17Z

Based on https://github.com/podsvirov/protobuf/tree/topic-cmake-project

Adds support for building the examples
Adds support for using the build directory as a local install directory (set protobuf_DIR to the build folder)
Fixes breaks in protobuf-module.cmake

Examples may be built separately or as part of a full build of protobuf using protobuf_BUILD_EXAMPLES. This is done using ExternalProject_Add to help ensure that building the examples as part of a full build is as close as possible to building them independently.
protobuf_BUILD_EXAMPLES_MULTICONFIG adds 3 additional targets useful for testing the cmake configuration files.

@podsvirov This addresses my comments on #1642 and is much more limited in scope so should be easier to merge.

On using sub_directory

Originally I used sub_directory to include the examples repo, however there was a cascade of issues caused by calling find_package on a project that is currently in the configuration step, particularly with protobuf_MODULE_COMPATILBITY set.

CMP0024 disallows including the result of an export command, which forces the definition of alias targets
The protobuf-module.cmake file uses IMPORTED_LOCATION properties to determine locations, but those fields are not available for alias targets.
CMP0026 disallows reading LOCATION properties, so the best we can do is using generator expressions

If the additional complexity in protobuf-config and protobuf-module is acceptable, or if we're ok not supporting protobuf_MODULE_COMPATIBILTIY when generating in that mode, sub_directory might be worth reconsidering

grpc-kokoro · 2016-06-03T02:28:18Z

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok to test.

podsvirov · 2016-06-03T03:28:44Z

cmake/install.cmake

@@ -100,9 +101,19 @@ configure_file(protobuf-config-version.cmake.in
 configure_file(protobuf-module.cmake.in
  protobuf-module.cmake @ONLY)

+# Allows the build directory to be used as a find directory.
+export(EXPORT protobuf-targets
+  NAMESPACE protobuf::)


Oops... CMP0024 added in CMake 3.0 and we need and we promised to support CMake version 2.8.12 which does not have export(EXPORT..., exportr(TARGETS.. it is only available (see there).

yeswalrus · 2016-06-03T04:42:22Z

@podsvirov Comments addressed.

yeswalrus · 2016-06-03T08:15:53Z

Tested working with MSVC 2015 nmake files and CMake 2.8.12

podsvirov · 2016-06-03T20:54:31Z

cmake/install.cmake

-  "${protobuf_BINARY_DIR}/protobuf-config-version.cmake"
-  "${protobuf_BINARY_DIR}/protobuf-module.cmake"
+# Allows the build directory to be used as a find directory.
+export(TARGETS libprotobuf-lite libprotobuf libprotoc protoc


No! CMP0024.

CMP0024 refers specifically to using the include command on the output of export(...). It disallows it because in cmake 3+, the file generated by the export command is not reliably available at configuration time.

This is required to allow using the build directory in the same way you would an install directory
Ex:
In protobuf-build: cmake ../protobuf/cmake, then build all
then, in protobuf-build-examples: cmake ../protobuf/examples -Dprotobuf_DIR="../protobuf-build/cmake"

Agree with @yeswalrus on this one. export is a totally legitimate concept, and there is nothing in this particular use of export that would prevent it from being used in either the new preferred manner or in the legacy manner.

A series of improvements: - Improved Protobuf module compatibility (disabled by default); - Hide advanced settings; - Added build tree configuration; - Added build of examples.

yeswalrus · 2016-06-04T02:57:32Z

Ready to merge. @xfxyjwf @podsvirov If we can't agree on something between this and #1642, The first commit in this PR could stand on it's own by allowing CMake builds to install examples that could then be built using cmake.

yeswalrus · 2016-06-04T07:01:50Z

@podsvirov I could close this and submit a pr directly to your #1642 branch if you prefer?
https://github.com/yeswalrus/protobuf/tree/topic-cmake-project-updates
contains all the fixes found here and merges with yours.

podsvirov · 2016-06-04T12:09:27Z

It does a good job. There were many discussions and corrections.

Tool of PRs allows people to communicate and make changes.
I believe that our work is consistent and take mozheno first #1642 and then #1643. To do this, you need to modify it a little bit, you just need to bring your branch in the desired state.
As variant simply forced push content of branch yeswalrus:topic-cmake-project-updates to yeswalrus:cmake-examples.

Also @yeswalrus in branch yeswalrus:topic-cmake-project-updates you merge with my changes, but "git rebase..." more elegant solution.

googlebot · 2016-06-06T19:07:56Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

yeswalrus · 2016-06-06T19:09:14Z

Done! @podsvirov, mind appeasing the googlebot?
@xfxyjwf This can now be merged on it's own, or after #1642

podsvirov · 2016-06-06T20:38:44Z

Thanks @yeswalrus and googlebot is obsolutly wrong (try repeat instructions).
Ok. All checks done. @xfxyjwf can you merged on it's own, or after #1642.

yeswalrus · 2016-06-06T22:03:32Z

@googlebot Confirming

yeswalrus · 2016-06-06T22:15:49Z

@podsvirov I think you have to confirm that you're ok with your commits being included in this PR

xfxyjwf · 2016-06-06T23:03:41Z

cmake/CMakeLists.txt

 # Options
-option(protobuf_VERBOSE "Enable for verbose output" OFF)


I was under the impression that you need to define the option before using it. No?

if you check it before it's defined, it evaluates as false. In this case that's OK since it defaults to off, so if it's on it will either already be defined from the command line or will be set in the cache.

podsvirov · 2016-06-07T03:15:27Z

@googlebot add me to this pull request

podsvirov · 2016-06-07T03:22:39Z

@xfxyjwf I do not know how to get @googlebot to take my consent.
I think he changed his mind after the merger in #1642.

xfxyjwf · 2016-06-07T17:13:41Z

googlebot is probably trying to find some special keywords in your message. I don't know what exact works it's expecting either.

xfxyjwf · 2016-06-07T17:14:12Z

Updated the label manually and merging...

googlebot added the cla: yes label Jun 3, 2016

yeswalrus changed the title ~~Build examples in with CMake~~ Build examples in CMake Jun 3, 2016

yeswalrus force-pushed the cmake-examples branch 3 times, most recently from 127824a to 95a7d1f Compare June 3, 2016 03:05

podsvirov reviewed Jun 3, 2016
View reviewed changes

yeswalrus force-pushed the cmake-examples branch 2 times, most recently from d4d1685 to 0897056 Compare June 3, 2016 04:42

yeswalrus force-pushed the cmake-examples branch 2 times, most recently from 69a1352 to 54c84fe Compare June 3, 2016 08:13

yeswalrus force-pushed the cmake-examples branch 3 times, most recently from 329ea6a to c018288 Compare June 3, 2016 18:26

podsvirov reviewed Jun 3, 2016
View reviewed changes

CMake project updates

7155629

A series of improvements: - Improved Protobuf module compatibility (disabled by default); - Hide advanced settings; - Added build tree configuration; - Added build of examples.

yeswalrus force-pushed the cmake-examples branch 2 times, most recently from 82b5f98 to dd94731 Compare June 3, 2016 21:19

podsvirov mentioned this pull request Jun 3, 2016

CMake project updates #1642

Merged

Walter Gray added 2 commits June 6, 2016 12:06

Use ExternalProject_Add to build the examples in a stand-alone fashion.

09f6a5c

Replace handwritten protobuf-targets with exported version.

23fef56

yeswalrus force-pushed the cmake-examples branch from 1ddb8b0 to 7b9bb13 Compare June 6, 2016 19:07

googlebot added cla: no and removed cla: yes labels Jun 6, 2016

xfxyjwf reviewed Jun 6, 2016
View reviewed changes

yeswalrus force-pushed the cmake-examples branch from 7b9bb13 to 23fef56 Compare June 7, 2016 07:25

xfxyjwf added cla: yes and removed cla: no labels Jun 7, 2016

xfxyjwf merged commit dfe0c9a into protocolbuffers:master Jun 7, 2016

yeswalrus deleted the cmake-examples branch June 7, 2016 18:26

yeswalrus mentioned this pull request Jun 8, 2016

Cleanup protobuf-module.cmake.in #1658

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build examples in CMake #1643

Build examples in CMake #1643

yeswalrus commented Jun 3, 2016 •

edited

Loading

grpc-kokoro commented Jun 3, 2016

podsvirov Jun 3, 2016

yeswalrus commented Jun 3, 2016

yeswalrus commented Jun 3, 2016

podsvirov Jun 3, 2016

yeswalrus Jun 3, 2016 •

edited

Loading

codemercenary Jun 3, 2016

yeswalrus commented Jun 4, 2016 •

edited

Loading

yeswalrus commented Jun 4, 2016

podsvirov commented Jun 4, 2016

googlebot commented Jun 6, 2016

yeswalrus commented Jun 6, 2016 •

edited

Loading

podsvirov commented Jun 6, 2016

yeswalrus commented Jun 6, 2016

yeswalrus commented Jun 6, 2016

xfxyjwf Jun 6, 2016

yeswalrus Jun 6, 2016 •

edited

Loading

podsvirov commented Jun 7, 2016 •

edited

Loading

podsvirov commented Jun 7, 2016

xfxyjwf commented Jun 7, 2016

xfxyjwf commented Jun 7, 2016

		# Options
		option(protobuf_VERBOSE "Enable for verbose output" OFF)

Build examples in CMake #1643

Build examples in CMake #1643

Conversation

yeswalrus commented Jun 3, 2016 • edited Loading

On using sub_directory

grpc-kokoro commented Jun 3, 2016

podsvirov Jun 3, 2016

Choose a reason for hiding this comment

yeswalrus commented Jun 3, 2016

yeswalrus commented Jun 3, 2016

podsvirov Jun 3, 2016

Choose a reason for hiding this comment

yeswalrus Jun 3, 2016 • edited Loading

Choose a reason for hiding this comment

codemercenary Jun 3, 2016

Choose a reason for hiding this comment

yeswalrus commented Jun 4, 2016 • edited Loading

yeswalrus commented Jun 4, 2016

podsvirov commented Jun 4, 2016

googlebot commented Jun 6, 2016

yeswalrus commented Jun 6, 2016 • edited Loading

podsvirov commented Jun 6, 2016

yeswalrus commented Jun 6, 2016

yeswalrus commented Jun 6, 2016

xfxyjwf Jun 6, 2016

Choose a reason for hiding this comment

yeswalrus Jun 6, 2016 • edited Loading

Choose a reason for hiding this comment

podsvirov commented Jun 7, 2016 • edited Loading

podsvirov commented Jun 7, 2016

xfxyjwf commented Jun 7, 2016

xfxyjwf commented Jun 7, 2016

yeswalrus commented Jun 3, 2016 •

edited

Loading

yeswalrus Jun 3, 2016 •

edited

Loading

yeswalrus commented Jun 4, 2016 •

edited

Loading

yeswalrus commented Jun 6, 2016 •

edited

Loading

yeswalrus Jun 6, 2016 •

edited

Loading

podsvirov commented Jun 7, 2016 •

edited

Loading