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

Improvements to make SPARTA easier to use as a dependency #24

Closed
wants to merge 4 commits into from

Conversation

Technius
Copy link
Contributor

This PR includes some improvements to the CMake configuration that makes SPARTA easier to use as a dependency (or at least I've found these useful for a project I'm working on). Specifically:

  • Generate a sparta-config.cmake file that is created in both the build folder and for the install target. This enables the library to be used with find_package.
  • (Breaking change) Install the sparta_target.cmake to $LIBDIR/cmake/sparta instead of $PREFIX/cmake to support *nix systems as well as find_package. While this is a breaking change, it's unlikely that anyone used the installed file due to the bugs observed in Miscellaneous bugfixes #23.
  • (Breaking change) Use sparta:: as the target namespace when exporting the sparta target. Projects that previously included the sparta_target.cmake will need to be updated to use sparta::sparta instead of sparta when specifying it as a dependency.
  • Update the README with additional tips on how to use SPARTA in CMake projects.

This PR is stacked on top of #23, and I'll rebase after it is merged.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Dec 14, 2023
@Technius
Copy link
Contributor Author

Here are some files I've used to test this PR:

CMakeLists.txt
cmake_minimum_required(VERSION 3.20)

project(Fakeproject)

find_package(Boost REQUIRED COMPONENTS thread graph)

if(EXISTS sparta_subproject)
  add_subdirectory(sparta_subproject)
else()
  find_package(sparta REQUIRED)
endif()
message(STATUS "Found sparta: ${sparta_DIR}")

add_executable(fakeproject main.cpp)
target_link_libraries(fakeproject PRIVATE sparta::sparta)
main.cpp
#include <iostream>
#include <sparta/AbstractDomain.h>

int main() { return 0; }
test.sh
#!/usr/bin/env bash
rm -rf build sparta_subproject

# Local git checkout of SPARTA
LOCAL_SPARTA_CHECKOUT=/path/to/your/local/SPARTA

# Enable one of the following:

# - use sparta installed using `make install` with prefix set to e.g. /tmp/sparta
#SPARTA_ARGS=(-Dsparta_DIR=/tmp/sparta/lib64/cmake/sparta)

# - use sparta from the build directory of a local checkout
SPARTA_ARGS=(-Dsparta_DIR=~/projects/SPARTA/build)

# - use sparta as a subproject
# git clone "$LOCAL_SPARTA_CHECKOUT" sparta_subproject

cmake -B build "${SPARTA_ARGS[@]}"
cmake --build build

@arthaud
Copy link
Contributor

arthaud commented Dec 15, 2023

This is awesome, thanks. I will test this once you have rebased on top of master :)

This introduces causes a CMake package config file named
sparta-config.cmake to be generated in sparta's build directory. The
config file will be installed to $LIBDIR/cmake/sparta so that it can be
located with find_package().

As a breaking change, sparta_target.cmake is now also installed to
$LIBDIR/cmake/sparta for compatibility with *nix systems [1].

[1]: https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-search-procedure
This changes the exported targets to be namespaced with sparta::, so
that CMake will raise an error if the imported target is referenced but
not found.
This commit fixes seveal bugs preventing the project from being used as
a CMake subproject:

* The Commons module cannot be found because the path to the
cmake_modules folder is constructed relative to the CMAKE_SOURCE_DIR
rather than the CMAKE_CURRENT_SOURCE_DIR.

* googletest is downloaded to the CMAKE_BINARY_DIR rather than the
CMAKE_CURRENT_BINARY_DIR.
@facebook-github-bot
Copy link

@arthaud has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit to facebook/redex that referenced this pull request Dec 19, 2023
Summary:
This PR includes some improvements to the CMake configuration that makes SPARTA easier to use as a dependency (or at least I've found these useful for a project I'm working on). Specifically:

* Generate a `sparta-config.cmake` file that is created in both the build folder and for the install target. This enables the library to be used with `find_package`.
* (Breaking change) Install the `sparta_target.cmake` to `$LIBDIR/cmake/sparta` instead of `$PREFIX/cmake` to support \*nix systems as well as `find_package`. While this is a breaking change, it's unlikely that anyone used the installed file due to the bugs observed in facebook/SPARTA#23.
* (Breaking change) Use `sparta::` as the target namespace when exporting the `sparta` target. Projects that previously included the `sparta_target.cmake` will need to be updated to use  `sparta::sparta` instead of `sparta` when specifying it as a dependency.
* Update the README with additional tips on how to use SPARTA in CMake projects.

This PR is stacked on top of facebook/SPARTA#23, and I'll rebase after it is merged.

X-link: facebook/SPARTA#24

Reviewed By: arnaudvenet

Differential Revision: D52238117

Pulled By: arthaud

fbshipit-source-id: abfad1312d5357a5adc64335d7a51e80935cf111
@facebook-github-bot
Copy link

@arthaud merged this pull request in e2c1715.

facebook-github-bot pushed a commit to facebook/redex that referenced this pull request Dec 19, 2023
Summary:
This PR includes some improvements to the CMake configuration that makes SPARTA easier to use as a dependency (or at least I've found these useful for a project I'm working on). Specifically:

* Generate a `sparta-config.cmake` file that is created in both the build folder and for the install target. This enables the library to be used with `find_package`.
* (Breaking change) Install the `sparta_target.cmake` to `$LIBDIR/cmake/sparta` instead of `$PREFIX/cmake` to support \*nix systems as well as `find_package`. While this is a breaking change, it's unlikely that anyone used the installed file due to the bugs observed in facebook/SPARTA#23.
* (Breaking change) Use `sparta::` as the target namespace when exporting the `sparta` target. Projects that previously included the `sparta_target.cmake` will need to be updated to use  `sparta::sparta` instead of `sparta` when specifying it as a dependency.
* Update the README with additional tips on how to use SPARTA in CMake projects.

This PR is stacked on top of facebook/SPARTA#23, and I'll rebase after it is merged.

X-link: facebook/SPARTA#24

Reviewed By: arnaudvenet

Differential Revision: D52238117

Pulled By: arthaud

fbshipit-source-id: abfad1312d5357a5adc64335d7a51e80935cf111
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants