fix: generated pkg-config (.pc) files on CMake build

[luau-project]

Description

Right after cminpack v1.3.9 was released, I opened a pull request on MSYS2 to insert cminpack on their repositories https://github.com/msys2/MINGW-packages/pull/21025. During the review process, the reviewer gave some valuable inputs:

1. Asked if there was a way to avoid the conversion from LF to CRLF on Windows (which I addressed on https://github.com/devernay/cminpack/pull/63);
2. When building the static library through CMake, cminpack adds _s suffix to the library name. The reviewer noticed that the generated pkg-config files (*.pc) didn't match well the generated static library files. Investigating further, I noticed that all the installed .pc files had a wrong link flag (-l).

Thus, this pull request addresses the second point above and fixes the installed .pc files.

Issue reproduction

The current released version (v1.3.9.tar.gz)

To have a clear understand of what is happening in the actual version of cminpack (v1.3.9), I prepared a simple script to be run on Debian-based distros (Debian 12 stable, Ubuntu 22.04 +, etc).

1. Install the tooling:

   sudo apt install -y gcc make git cmake

2. Build, test and install the static version of cminpack v1.3.9 from the released tarball

cd /tmp
wget https://github.com/devernay/cminpack/archive/v1.3.9.tar.gz
tar -xf v1.3.9.tar.gz
mkdir build-v1.3.9
cd build-v1.3.9/
cmake -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF -DUSE_BLAS=OFF --install-prefix $PWD/static -S ../cminpack-1.3.9/ -B _static
cmake --build _static --config Release
ctest --test-dir _static/ -C Release
cmake --install _static/ --config Release

3. If you cat static/lib/pkgconfig/cminpack.pc the installed pkg-config file for the double-precision version, you get the following screen:
4. As you can see in the image above, the -l flag on the pkg-config file for the double-precision version has -lcminpack, but the file is shown as /tmp/build-v1.3.9/static/lib/pkgconfig/libcminpack_s.a, which is an issue;
5. The same kind of issue also happens for the single and long-double precision versions:

The fixed version on my fork

1. Build, test and install the static version of cminpack from my forked repo branch

cd /tmp
git clone --branch=fix-pkg-config https://github.com/luau-project/cminpack.git cminpack-fork
mkdir fork-v1.3.9
cd fork-v1.3.9/
cmake -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF -DUSE_BLAS=OFF --install-prefix $PWD/static -S ../cminpack-fork/ -B _static
cmake --build _static/ --config Release
ctest --test-dir _static/ -C Release
cmake --install _static/ --config Release

2. I decided to rename the pkg-config file to match the installed library name. So, if you cat static/lib/pkgconfig/cminpack_s.pc the installed pkg-config file for the double-precision version, you get the following screen:
3. As you can see in the image above, the -l flag on the pkg-config file for the double-precision version has -lcminpack_s, which is correct compared to the file shown /tmp/build-v1.3.9/static/lib/pkgconfig/libcminpack_s.a;
4. The same fix was applied for all other precisions as you can see:

How

1. I had to make a few changes regarding the suffix. In the old setup, it was used config flags like CMAKE_RELEASE_POSTFIX. I changed the way how it works in the CMakeLists.txt and propagated all the changes down to tests on examples/CMakeLists.txt;
2. I also added a guard to only test the library if the double precision version is being built. Currently, the tests are only implemented for the double precision version. These changes were employed on examples/CMakeLists.txt;

Conclusion

I tested locally the shared and static versions. In my view, everything is working as expected now.

[luau-project]

Update

Hello. Yesterday, I noticed that the generated pkg-config files for single precision and long-double precision had an incorrect Cflags attribute, as you can see in the last picture of my post above, because:

• the single precision library must ship with a define -D__cminpack_float__;
• and the long-double precision library must ship with a defined -D__cminpack_long_double__.

The updated fix

1. Build, test and install the static version of cminpack from my forked repo branch

   cd /tmp
   git clone --branch=fix-pkg-config https://github.com/luau-project/cminpack.git cminpack-fork-updated
   mkdir fork-updated-v1.3.9
   cd fork-updated-v1.3.9/
   cmake -DCMAKE_BUILD_TYPE=Release -DBUILD_SHARED_LIBS=OFF -DUSE_BLAS=OFF --install-prefix $PWD/static-updated -S ../cminpack-fork-updated/ -B _static-updated
   cmake --build _static-updated/ --config Release
   ctest --test-dir _static-updated/ -C Release
   cmake --install _static-updated/ --config Release

2. Capturing the content of the pkg-config file for the double precision (cat static-updated/lib/pkgconfig/cminpack_s.pc) has the same behavior as earlier, because they were correct:
3. The most interesting part, the Cflags for the single and long-double precision versions are now fixed, as you can see the captures (cat static-updated/lib/pkgconfig/cminpacks_s.pc) and (cat static-updated/lib/pkgconfig/cminpackld_s.pc):

[devernay]

Nice! Can you please confirm that double-precision shared minpack will be unaffected by this? My fear is that some dependencies may depend on this, so we shouldn't change the package name (cminpack.pc).

[luau-project]

Hello @devernay .

I saw that there were many commits in the last few days, and I will need some time to understand what happened.

Hello @jschueller .

I saw that you asked a few questions about chunks of scripts on the CMakeLissts.txt file. If you follow the diff, you can see that I didn't change many things, I mostly moved things to different places, trying to preserve the maximum amount of work that was done by the people who implemented this CMake script in the past.

In short, I agree with you that the current CMake script is a bit confuse, but it was just derived from past work.

In my honest opinion, I think the CMakeLists.txt script should be reworked from scratch, because it is hard to maintain and to add features.

I'll work on a complete rewrite of the CMake build in my fork. However, this will not happen for now.

[devernay]

my changes were simply fixing BLAS support in the code

[devernay]

Should we close this one in favor of #71 ?

[luau-project]

Should we close this one in favor of #71 ?

In my view, yes. #71 seems better.