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

CMake cleanup #1060

Merged
merged 1 commit into from
Oct 18, 2022
Merged

CMake cleanup #1060

merged 1 commit into from
Oct 18, 2022

Conversation

jpr42
Copy link
Contributor

@jpr42 jpr42 commented Oct 15, 2022

  • Move all install logic inside gsl_install.cmake
    • This makes reading the logic easier, and avoids the enable language issue with GNUInstallDirs by having it included after the project call
  • Have all functions inside gsl_functions.cmake
  • Use CMake idiom PROJECT_IS_TOP_LEVEL
  • Update README.md

@jpr42
Copy link
Contributor Author

jpr42 commented Oct 15, 2022

I tested that find_package still works as expected by using the tests directory which invokes find_package(Microsoft.GSL REQUIRED) if it consumed as a top level project.

  • This makes reading the logic easier, and avoids the enable language issue with GNUInstallDirs by having it included after the project call

By issue I just mean how the logic was a bit awkward. Since the guidelineSupportLibrary.cmake relied on GNUInstallDirs but couldn't directly include itself being included before a language is enabled.

include(gsl_functions)

project(GSL
    VERSION 4.0.0
    LANGUAGES CXX // Language is enabled here <- Now you can use GNUInstallDirs
)

Now GNUInstallDirs is included by gsl_install.cmake which handles all the installation.

- Move all install logic inside gsl_install.cmake
  - This makes reading the logic easier, and avoids the enable language
    issue with `GNUInstallDirs` by having it included after the project
    call
- Have all functions inside gsl_functions.cmake
- Use CMake idiom PROJECT_IS_TOP_LEVEL
- Update README.md
@dmitrykobets-msft
Copy link
Member

Awesome, thanks a lot @jpr42 for this cleanup.

@dmitrykobets-msft dmitrykobets-msft merged commit c52bad3 into microsoft:main Oct 18, 2022
@jpr42 jpr42 deleted the cmake_fixes branch October 18, 2022 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants