Feature/better find package

[vguen]

This PR is a proposal to fix #732 using find_package in order to have better granularity for handling dependencies (this is inspired by https://gist.github.com/Som1Lse/6458d28c4bb09163a5389f24fec981db).
Thanks to the built-in variable <package_name>_DIR variable we can override the default path for find_package in order to use :

• some preinstall cmake package
• custom source repo (either download with BUILD_DEPS=ON or already download and provide by the user through a custom <Package_name>Config.cmake)

Allowing to use find_package in order to resolve out of tree dependencies allow or-tools to integrate nicely with Yocto, which is not the case today as Yocto since we cannot relay on the BUILD_DEPS switch in Yocto (it does not allow to download dependency during cmake configure time).

⚠️ Note that this PR will properly for work preinstall cmake package once abseil/abseil-cpp#182 will be merged

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

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

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

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

[vguen]

@Mizux do you think this PR could be integrate into the code ?

[Mizux]

First, thanks for your contribution !

If I understand correctly, you just split the cmake/external/CMakeLists.txt into several DependencyConfig.cmake so for each deps you could choose the system package or ortools will compile it for you.
-> I'm OK with this behaviour...

But in this case the new macro _find_package() and the filename seems strange to me.
I mean you include() them if needed, while for me you should have modified the CMAKE_MODULE_PATH instead so find_package() would find them first instead of the one already installed by your distro package manager...
And call them FindDepency.cmake e.g. FindSWIG.cmake

In Module mode, CMake searches for a file called Find<PackageName>.cmake in the CMAKE_MODULE_PATH followed by the CMake installation. If the file is found, it is read and processed by CMake. It is responsible for finding the package, checking the version, and producing any needed messages. Some find-modules provide limited or no support for versioning; check the module documentation.

ref: https://cmake.org/cmake/help/latest/command/find_package.html#basic-signature-and-module-mode
note: in this case we should have each FindDeps.cmake in a separate directory, so we can selectively add them to CMAKE_MODULE_PATH as needed...

Also, one other side topic: I think I'll have to change the CMake behavior, I would like to install deps first then consume the install, than trying to use it as add_subdirectory(). Currently it's conflict with dependency install rule or you need to hack dependencies to make it work...
see: https://travis-ci.org/google/or-tools/jobs/482350303#L904

Sorry for the delay, I'm still thinking about it and where I would like to go.
Need to play with your branch...

[Mizux]

abseil-cpp ? (I can also do it in a later commit)
just becaue there is also https://github.com/abseil/abseil-py and maybe we will need it one day too
note: shame on me of not doing it too ;)

[vguen]

@Mizux I rework the way this branch handle its dependencies in 2a003e0 the branch now donwload, build & install its file in the build directory (no more subproject).

I have however the following compilation error which I do not understand and I would like your help to see what I could have missed.

/or-tools/or-tools/ortools/data/jobshop_scheduling_parser.cc:26:1: error: C++ requires a type specifier for all declarations
DEFINE_int64(jssp_scaling_up_factor, 100000L,
^
/or-tools/or-tools/ortools/data/jobshop_scheduling_parser.cc:26:14: error: use of undeclared identifier 'jssp_scaling_up_factor'
DEFINE_int64(jssp_scaling_up_factor, 100000L,
             ^
/or-tools/or-tools/ortools/data/jobshop_scheduling_parser.cc:359:45: error: use of undeclared identifier 'FLAGS_jssp_scaling_up_factor'
          static_cast<int64>(round(weight * FLAGS_jssp_scaling_up_factor));
                                            ^
/or-tools/or-tools/ortools/data/jobshop_scheduling_parser.cc:375:19: error: use of undeclared identifier 'FLAGS_jssp_scaling_up_factor'
                  FLAGS_jssp_scaling_up_factor !=
                  ^
/or-tools/or-tools/ortools/data/jobshop_scheduling_parser.cc:385:17: error: use of undeclared identifier 'FLAGS_jssp_scaling_up_factor'
                FLAGS_jssp_scaling_up_factor);
                ^
/or-tools/or-tools/ortools/data/jobshop_scheduling_parser.cc:389:22: error: use of undeclared identifier 'FLAGS_jssp_scaling_up_factor'
              1.0L / FLAGS_jssp_scaling_up_factor);
                     ^

Edit: I did remove some patch in order to be the closest to what could be the source install by a user, I think that moving away from those patch (like some amelioration to the absl target name) would be easier to use system package.

[Mizux]

Hi,

I revamped your PR in #1116 (commit a49b914) but with few changes:

• Mostly Keep BUILD_dependencies options BUT

  • by default don't build dependencies (easier for distro package maintainer) unless you build wrappers
    (ed BTW PEP 513 for python native package doesn't allow us to use system libraries, so if we have to ship them...)
  • you can still enable BUILD_* if you want...
  • if BUILD_DEPS=ON all BUILD_* options are forced to ON
    

    or-tools/CMakeLists.txt

    Lines 49 to 50 in 4477092

    	CMAKE_DEPENDENT_OPTION(BUILD_ZLIB "Build the ZLIB dependency Library" OFF
    	"NOT BUILD_DEPS" ON)

    
    which is the opposite of your
    https://github.com/victor-uavia/or-tools/blob/2a003e0938c15c885ee9976835d22568bbc8534a/CMakeLists.txt#L42-L43

• I don't use a Find*.cmake but simply call find_package(* REQUIRED) and let CMake to do its stuff

  • user can use CMAKE_PREFIX_PATH/CMAKE_MODULE_PATH if they want to hack/add hook etc..

• all deps, if build, are installed in CMAKE_BINARY_DIR/dependencies/install/ so I just need to append this to CMAKE_PREFIX_PATH to see all available prebuilt dependencies.

• I use a template https://github.com/google/or-tools/blob/master/cmake/dependencies/CMakeLists.txt.in for ExternalProject along a function in utils.cmake
  

  or-tools/cmake/utils.cmake

  Lines 66 to 126 in 4477092

  	# build_git_dependency()
  	#
  	# CMake function to download, build and install (in staging area) a dependency at configure
  	# time.
  	#
  	# Parameters:
  	# NAME: name of the dependency
  	# REPOSITORY: git url of the dependency
  	# TAG: tag of the dependency
  	# APPLY_PATCH: apply patch
  	# CMAKE_ARGS: List of specific CMake args to add
  	#
  	# build_dependency(
  	# NAME
  	# abseil-cpp
  	# URL
  	# https://github.com/abseil/abseil-cpp.git
  	# TAG
  	# master
  	# APPLY_PATCH
  	# ${CMAKE_SOURCE_DIR}/patches/abseil-cpp.patch
  	# )
  	function(build_git_dependency)
  	set(options "")
  	set(oneValueArgs NAME REPOSITORY TAG APPLY_PATCH)
  	set(multiValueArgs CMAKE_ARGS)
  	cmake_parse_arguments(GIT_DEP
  	"${options}"
  	"${oneValueArgs}"
  	"${multiValueArgs}"
  	${ARGN}
  	)
  	message(STATUS "Building ${GIT_DEP_NAME}: ...")
  	if(GIT_DEP_APPLY_PATCH)
  	set(PATCH_CMD "git apply \"${GIT_DEP_APPLY_PATCH}\"")
  	else()
  	set(PATCH_CMD "\"\"")
  	endif()
  	configure_file(
  	${CMAKE_CURRENT_SOURCE_DIR}/CMakeLists.txt.in
  	${CMAKE_CURRENT_BINARY_DIR}/${GIT_DEP_NAME}/CMakeLists.txt @ONLY)
  	execute_process(
  	COMMAND ${CMAKE_COMMAND} -H. -Bproject_build -G "${CMAKE_GENERATOR}"
  	RESULT_VARIABLE result
  	WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/${GIT_DEP_NAME})
  	if(result)
  	message(FATAL_ERROR "CMake step for ${GIT_DEP_NAME} failed: ${result}")
  	endif()
  	execute_process(
  	COMMAND ${CMAKE_COMMAND} --build project_build --config ${CMAKE_BUILD_TYPE}
  	RESULT_VARIABLE result
  	WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/${GIT_DEP_NAME})
  	if(result)
  	message(FATAL_ERROR "Build step for ${GIT_DEP_NAME} failed: ${result}")
  	endif()
  	message(STATUS "Building ${GIT_DEP_NAME}: ...DONE")
  	endfunction()

• I use my new Cbc split repo forks of the official ones

• I use the fixed abseil-cpp name i.e. absl::base instead of "broken" absl::absl_base since my internal path make its way to the public abseil-cpp repo.

Any feedback are welcome, hope you'll better understand what I'm aiming for....

[Mizux]

I'm closing this PR since we have #1116
Still open to any proposal, critics etc....