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

Fix cmake install targets #9822

Merged
merged 2 commits into from
Apr 21, 2022

Conversation

jonringer
Copy link
Contributor

Tried to capture most of the context in the commit messages, so I'll just paste them here:

Avoid exporting build tree targets to installation directory
export(TARGETS ...) is meant to allow for build trees to be found
from other projects, however, writing the file to an installation
directory causes buildtree paths to be hardcoded in the installed
protobuf-targets.cmake

Install protobuf-targets.cmake from target, not build tree
This allows for installation prefixes to match the intended
installation directories, instead of being copied from
the build tree.

Fixes: #9809

TL;DR:
export(TARGETS ...) isn't meant to be used for installation, only local development. We should be using install(EXPORT ...) for providing relocatable target locations.

`export(TARGETS ...)` is meant to allow for build trees to be found
from other projects, however, writing the file to an installation
directory causes buildtree paths to be hardcoded in the installed
protobuf-targets.cmake
This allows for installation prefixes to match the intended
installation directories, instead of being copied from
the build tree.
@jonringer
Copy link
Contributor Author

I can't apply labels, but they should be just be cmake

@acozzette acozzette merged commit c80808c into protocolbuffers:main Apr 21, 2022
@acozzette
Copy link
Member

@jonringer Thanks!

@jonringer jonringer deleted the fix-cmake-install-targets branch April 21, 2022 01:39
@jonringer
Copy link
Contributor Author

@acozzette of course :)

Thanks for the speedy review

acozzette added a commit to acozzette/protobuf that referenced this pull request May 26, 2022
This reverts commit c80808c.

Thix fixes protocolbuffers#10045. Somehow the change caused these four .cmake files to
stop being installed:

protobuf-config.cmake
protobuf-config-version.cmake
protobuf-module.cmake
protobuf-options.cmake

After reverting the change, I confirmed that the files are being
installed again.
acozzette added a commit that referenced this pull request May 26, 2022
This reverts commit c80808c.

Thix fixes #10045. Somehow the change caused these four .cmake files to
stop being installed:

protobuf-config.cmake
protobuf-config-version.cmake
protobuf-module.cmake
protobuf-options.cmake

After reverting the change, I confirmed that the files are being
installed again.
srmainwaring added a commit to srmainwaring/protobuf that referenced this pull request Apr 30, 2024
- Apply protocolbuffers#9822

Signed-off-by: Rhys Mainwaring <rhys.mainwaring@me.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cmake incorrectly installs config files when passed absolute values
3 participants