Skip to content

Install the library into a multiarch path#110

Open
mapreri wants to merge 1 commit intotrendmicro:masterfrom
mapreri:multiarch
Open

Install the library into a multiarch path#110
mapreri wants to merge 1 commit intotrendmicro:masterfrom
mapreri:multiarch

Conversation

@mapreri
Copy link
Contributor

@mapreri mapreri commented Oct 12, 2021

CMAKE_INSTALL_LIBDIR defaults to "lib" or "lib64" or
lib/, depending on the system.

CMAKE_INSTALL_INCLUDEDIR is pretty much always "include".

https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html

Please take this patch so that distributors can more easily install the library into standard paths.

CMAKE_INSTALL_LIBDIR defaults to "lib" or "lib64" or
lib/<multiarch-tuple>, depending on the system.

CMAKE_INSTALL_INCLUDEDIR is pretty much always "include".

https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html

Signed-off-by: Mattia Rizzolo <mattia@mapreri.org>
@jonjoliver
Copy link
Collaborator

Hi mapreri

The current install was in pull request #107 by cgull
I am happy to merge your pull request, but I want to make sure that it does not break cgull's (and related people) install
I am checking with him

Cheers
jono

@mapreri
Copy link
Contributor Author

mapreri commented Oct 12, 2021

Sure! I'd also be happy to rework and adapt so it fits everybody's needs!

@jonjoliver
Copy link
Collaborator

The current official release is 4.8.2 which incorporates cgull's fix.
Lets give cgull an opportunity to reply.
And if he does not reply in a week, then we incorporate your pull request and do that as a pre-release
And see if anyone raises issues.

@cgull
Copy link
Contributor

cgull commented Nov 23, 2021

Sorry I missed this earlier. I'm working on FreeBSD and this change works fine there. @jonjoliver this is pretty clearly the right thing to do (another level of indirection!); go ahead and merge it. It might even help Cygwin or Haiku or something, too.

(On FreeBSD, lib is lib/ and include is include/, but both are in /usr/local for ports, but CMake already handles that in the install() function.)

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.

3 participants

Comments