Fix absolute libdirpath builds

[StillerHarpo]

This fixes builds where CMAKE_INSTALL_LIBDIR is absolute. This is the case on nixos (NixOS/nixpkgs#144170)

[bjornfor]

I think you can simplify these two hunks to just:

-   DESTINATION ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/pkgconfig
+   DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig

CMake will ensure that CMAKE_INSTALL_LIBDIR gets prefixed as needed.

[bjornfor]

This fixes builds where CMAKE_INSTALL_FULL_LIBDIR is absolute.

I think you mean CMAKE_INSTALL_LIBDIR here, becasue CAKE_INSTALL_FULL_LIBDIR is always absolute.

[StillerHarpo]

This fixes builds where CMAKE_INSTALL_FULL_LIBDIR is absolute.

I think you mean CMAKE_INSTALL_LIBDIR here, becasue CAKE_INSTALL_FULL_LIBDIR is always absolute.

Yes this is what I meant

[bjornfor]

LGTM.

[bjornfor]

Nit: the PR message still mentions the wrong variable.

[michaelonken]

In theory, I could merge this, yes :-) I am not familiar with this part of the CMake toolchain files in DCMTK, I will check tomorrow with someone from the team.

Is the patch "clean" meanwhile (variable naming,..)? Are you quite sure it doesn't hurt on other OS?

Thank you for the contribution :)

[cfhammill]

Thanks @michaelonken, sorry for the terse message (which I deleted), I thought I was commenting on another PR. @StillerHarpo are you able to answer the above questions?

[michaelonken]

After talking to the rest of the DCMTK yesterday: If the patch is confirmed (from your side) to be clean, I'm ready to merge it :)

[StillerHarpo]

I build dcmtk with this patch on rasbian and it worked (only other os i have besides NixOS :))

The second hunch (in CMakeLists.txt) wasn't needed for nixos (it somehow got slipped in from experimenting). Im not sure if I should this hunk before merging. Im not very familiar with cmake. @bjornfor you seem to be more familiar with cmake. What do you think?

[bjornfor]

The second hunch (in CMakeLists.txt) wasn't needed for nixos (it somehow got slipped in from experimenting). Im not sure if I should this hunk before merging. Im not very familiar with cmake. @bjornfor you seem to be more familiar with cmake. What do you think?

I think the 2nd hunk is a good idea, ref #75 (comment). (Actually I'm surprised it worked without it on NixOS.)

[cfhammill]

@bjornfor it's not just NixOS, worked on ubuntu when installed with nixpkgs

[bjornfor]

@cfhammill: yes, I should have said nixpkgs, not nixos. Nixpkgs builds the same no matter what distro you run it on.

[StillerHarpo]

@michaelonken I'm comfortable merging this

[michaelonken]

I merged this into DCMTK's internal testing branch (04b0d6c). The commit will be visible on public master within the next days. Thank you for the patch :-)