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: Honor standard variables #277

Merged
merged 2 commits into from Dec 7, 2020
Merged

CMake: Honor standard variables #277

merged 2 commits into from Dec 7, 2020

Conversation

nbraud
Copy link
Contributor

@nbraud nbraud commented Nov 23, 2020

  • Do not ignore the variables that control CMake's RPATH handling,
    such as CMAKE_INSTALL_RPATH (which is enabled by default anyway)
  • Honor the standard CMAKE_INSTALL_* variables for install directories.

Both of those changes came up when packaging v2.1.1 for Debian,
as the CMake-based build system was failing to obey any of the standard
configuration variables.

This change should be backwards compatible, in that the defaults should
stay the same, and users who previously set YKPIV_* still override any
other variables.

This makes life much easier for distributions shipping this software,
if things work when the standard variables are set, rather than having
to special-case `yubico-piv-tool` and its `YKPIV_INSTALL_*` variables.

See https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html
@aveenismail aveenismail merged commit 2cfe909 into Yubico:master Dec 7, 2020
@nbraud nbraud deleted the pr/CMake_variables branch January 31, 2021 12:39
@nbraud
Copy link
Contributor Author

nbraud commented Jan 31, 2021

6a91dcd got merged, then immediately reverted by 283e2e7.
Could you explain why / what happened?

@aveenismail
Copy link
Member

Hi @nbraud

It was necessary to specifically set the RPATH when building for MacOS, otherwise the library and the commandline tool weren't linking correctly. We didn't see that setting the RPATH made a difference when building for a Linux system, and that's why we left it there.

Did you see a different behavior with Debian? Do you see a downside for doing it this way?

@nbraud
Copy link
Contributor Author

nbraud commented Feb 5, 2021

Did you see a different behavior with Debian? Do you see a downside for doing it this way?

Yes, this makes the build non-reproducible, i.e. rebuilding the same source, with the same toolchain, will give different results depending on the build path. Setting CMAKE_BUILD_RPATH_USE_ORIGIN or CMAKE_BUILD_WITH_INSTALL_RPATH might work to have builds that are both reproducible-by-default and work on macOS.

@aveenismail
Copy link
Member

An update was committed to set INSTALL_RPATH only when building on MacOS. I hope this resolves the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants