[openblas] Fix generating pkg-config file

[albertziegenhagel]

This PR makes sure that the pkg-config files are created correctly by the OpenBLAS configure/build steps.

OpenBLAS only creates the pkg-config files when pkg-config can be found during the CMake configure step (see https://github.com/xianyi/OpenBLAS/blob/efdd237a91646f0ce58815ef6507c04e393813a6/CMakeLists.txt#L392-L396).

To make sure the files are always created we find pkg-config beforehand and pass it to OpenBLAS CMake call via -DPKG_CONFIG_EXECUTABLE.

Some notes on the current implementation:

• The code to detect pkg-config was taken from the vcpkg_fixup_pkgconfig script. I would prefer to add pkg-config to vcpkg_find_acquire_program but I was not sure whether this is desired for a tool that will be installed through msys on windows.
• On my machine it was important to look for pkg-config before adding Perl to PATH. Otherwise a pkg-config distribution from within Perl at <vcpkg_root>\downloads\tools\perl\perl\bin was found, that is actually wrapped in a batch file and thus lead to errors afterwards. This might be important to take into account whenever vcpkg tries to find pkg-config.

edit: an alternative way to fix this would be to patch the OpenBLAS CMake files to always generate the pkg-config, since to my best knowledge pkg-config is actually not required to generate the files. But I kind of preferred to not introduce another patch that would have to be maintained for the next OpenBLAS version.

[Neumann-A]

an alternative way to fix this would be to patch the OpenBLAS CMake files

wondering if a one line patch changing the if in openblas to if(TRUE) would be better. It is only one line needing that change.

vcpkg_find_acquire_program but I was not sure whether this is desired

see #12626

[cenit]

Maybe just passing -DPKG_CONFIG_FOUND=TRUE is enough? Since the variable is set in cache by pkgconfig module, presetting it might be enough?

[MVoz]

vcpkg_acquire_msys(MSYS_ROOT PACKAGES pkg-config perl) # +perl ?

why keep PERL separate, it's built on MSYS2, not MSVC

[albertziegenhagel]

I've changed the PR to just apply a patch to generate the *.pc file unconditionally.

Just setting -DPKG_CONFIG_FOUND=ON did not seem to work. I assume it is because PKG_CONFIG_FOUND is not a cache variable in this case. See https://github.com/Kitware/CMake/blob/master/Modules/FindPkgConfig.cmake#L67.

A PR to upstream the changes to OpenBLAS can be found here: OpenMathLib/OpenBLAS#2785

[albertziegenhagel]

@Neumann-A wrote in a different PR:

I cannot remember if the prefix variable is defined in openblas.pc or not. If not it would be nice if you could add it to the upstream PR.
BTW: are you back due to lapack-reference being added to vcpkg? Haven't seen you around for a while. Do you have any knowledge of the progress of the flang driver on windows? I would like to test the lapack-reference with a fortran compiler outside of vcpkg.

Do you mean the changes you apply here: https://github.com/microsoft/vcpkg/blob/master/ports/openblas/portfile.cmake#L91-L103 ? If yes, I will add them to the PR or open a new one, depending on what the OpenBLAS people prefer.

And yes I am back due to lapack-reference being added. Thanks for the great work! I decided that this is a good reason to update all my third-party libraries.
I am currently trying to add a port for ScaLAPACK (written in Fortran as well), but I seem to have problems to make gcc link to the reference-lapack library as well as msmpi. Let's see how far I can get.

[Neumann-A]

Do you mean the changes you apply here

Yeah that are the changes i mean. A valid pc file always requires a valid prefix variable. You can make it relocatable with using pcfiledir. (Currently on mobile so correctly formatting or code references are a bit difficult.)

[albertziegenhagel]

Yeah that are the changes i mean. A valid pc file always requires a valid prefix variable. You can make it relocatable with using pcfiledir.

I am not sure what the best way to write the pkg-config file for OpenBLAS would be then. I think using

prefix=@CMAKE_INSTALL_PREFIX@
libdir=@CMAKE_INSTALL_FULL_LIBDIR@
libsuffix=@SUFFIX64_UNDERSCORE@
includedir=@CMAKE_INSTALL_FULL_INCLUDEDIR@

would not help much. And using

prefix=@CMAKE_INSTALL_PREFIX@
libdir=${prefix}/@CMAKE_INSTALL_LIBDIR@
libsuffix=@SUFFIX64_UNDERSCORE@
includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@

would not necessarily be correct under all circumstances, as far as I understand (see https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html#special-cases).

Making it based on ${pcfiledir} seems to be potentially problematic as well. See e.g.
google/googletest@095b311#diff-1935cf8c0499811814140fa33396d0ef
where they introduced it to googletest and then
google/googletest@5126ff4#diff-1935cf8c0499811814140fa33396d0ef
where they reverted those changes because they experienced problems.

(sorry, I do not have a lot of experience with generating pkg-config files in combination with GNUInstallDirs yet).

[Neumann-A]

(sorry, I do not have a lot of experience with generating pkg-config files in combination with GNUInstallDirs yet).

same here ;)

I think the problem with the gtest change is that the relative path to the prefix was hardcoded and not calculated.
I my opinion there are two possible solutions:
a) hardcode the full path to the prefix via @CMAKE_INSTALL_FULL_PREFIX@. On linux everything is typically hardcoded and not relocatable (since linux doesn't know how to be relocatable; install once never move)
or
b) calculate the relative path from pcfiledir to prefix via file(RELATIVE_PATH <variable> $PCFILE_DIR_INSTALL_LOCATION ${CMAKE_INSTALL_PREFIX}) (needs one get_filename_component on the filepath) and use prefix=${pcfiledir}/<calculatedrelpath>.
This should work in all cases unless some very obscure corner cases arise which we don't need to cover

[BillyONeal]

Thanks for your contribution!