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

Revert "only set install targets when not doing unit tests" #83

Merged
merged 1 commit into from
Feb 28, 2022

Conversation

olifre
Copy link
Contributor

@olifre olifre commented Feb 27, 2022

This reverts commit b5855a7.

Closes #82

As explained in the linked issue, this removes the need for downstream workarounds (not using make install or manually patching CMakeLists.txt) to allow to run unit tests in distro packaging.

@olifre
Copy link
Contributor Author

olifre commented Feb 27, 2022

This PR now also adds:

  • Not attempting to install the bundled gtest if BUILD_UNITTESTS=YES and EXTERNAL_GTEST=NO.
  • Running the GitHub actions tests both with internal and external gtest.

@djw8605
Copy link
Contributor

djw8605 commented Feb 28, 2022

@olifre could you merge the conflicts, if you want to change what is in #79. The github actions stuff looks good.

This catches different handling for install targets.
@olifre olifre force-pushed the install-targets-also-if-testing branch from f41cbb9 to 0ff3588 Compare February 28, 2022 19:55
@olifre
Copy link
Contributor Author

olifre commented Feb 28, 2022

@djw8605 Done, thanks for merging #79 , I missed that before opening this PR.
I have now dropped my other changes (I think the solution by @ellert is better ), and only kept the GitHub actions stuff ;-).

@djw8605 djw8605 merged commit 2f22c4f into scitokens:master Feb 28, 2022
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.

Lbrary and binary not installed if -DBUILD_UNITTESTS=YES is set
2 participants