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: Fix paths in .pc files #815

Merged
merged 2 commits into from
Aug 17, 2020

Conversation

jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Aug 11, 2020

It is not generally true that CMAKE_INSTALL_<dir> variables are relative paths:

https://github.com/jtojnar/cmake-snips#concatenating-paths-when-building-pkg-config-files

With 0b26a9d,
CMake concatenates ${prefix} with the GNUInstallDirs variables, which leads to
includedir=${prefix}/nix/store/x6wqbsys0px1xjpy17h01f0aqwj7khvg-openexr-2.5.2-dev/include,
when the variables are absolute.

Let's join them properly as paths, not strings.

Signed-off-by: Jan Tojnar jtojnar@gmail.com

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 11, 2020

CLA Check
The committers are authorized under a signed CLA.

@cary-ilm
Copy link
Member

@jtojnar, it looks like the copyright in cmake/JoinPaths.cmake is different from the OpenEXR project copyright. In order to accept the change, the copyright will need to read:

SPDX-License-Identifier: BSD-3-Clause

Copyright Contributors to the OpenEXR Project.

Can you make this submission under these conditions? Thanks.

It is not generally true that `CMAKE_INSTALL_<dir>` variables are relative paths:

https://github.com/jtojnar/cmake-snips#concatenating-paths-when-building-pkg-config-files

With AcademySoftwareFoundation@0b26a9d,
CMake concatenates ${prefix} with the GNUInstallDirs variables, which leads to
`includedir=${prefix}/nix/store/x6wqbsys0px1xjpy17h01f0aqwj7khvg-openexr-2.5.2-dev/include`,
when the variables are absolute.

Let's join them properly as paths, not strings.

Signed-off-by: Jan Tojnar <jtojnar@gmail.com>
@jtojnar
Copy link
Contributor Author

jtojnar commented Aug 11, 2020

@cary-ilm sure, relicensed.

Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with addressing cmake pathing post Imath refactor. lgtm !

@cary-ilm cary-ilm merged commit 023e879 into AcademySoftwareFoundation:master Aug 17, 2020
@jtojnar jtojnar deleted the fix-pc-path branch August 17, 2020 01:20
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.

None yet

3 participants