Skip to content

Conversation

@tkralphs
Copy link
Member

This is needed to make --with-blas-lib and --with-lapack-lib work correctly. These arguments are generally not needed or used, which is why this has never been realized before. However, on Windows when using the cl compiler and linking to a Blas/Lapack library whose path is provided by specifying it with --with-blas-lib and --with-lapack-lib, the linking fails. Blas and Lapack are not direct dependencies and so there is typically no reason to check for them when configuring Clp. But when there is no pkg-config support and the libraries are not either built from source or auto-discovered, it seems we need to fall back to also providing the library location when building Clp. This comes up when building conda packages on Windows, see here. @svigerske , does this look OK to you?

@svigerske
Copy link
Member

svigerske commented Feb 24, 2025

Clp doesn't depend on blas/lapack, so one shouldn't have to specify where to get it from.
The pkg-config mechanism is there to communicate the linker flags necessary for linking the dependencies of Clp. So if in order to link against CoinUtils one needs to link also Blas and Lapack explicitly, then they should be included in --with-coinutils-lflags. Wouldn't that work in https://github.com/conda-forge/coin-or-clp-feedstock/pull/18/files#diff-d7075654874cb08007a21aaab3ecd4b3453a9087e7505d034d548b8938b599bc?

@tkralphs
Copy link
Member Author

Clp doesn't depend on blas/lapack, so one shouldn't have to specify where to get it from.

Yeah, I understand that, but there is no mechanism on Windows to automatically get the right linker flags, since we don't have pkg-config (except for maybe the old *_addlibs.txt mechanism, which is still there, right?). I didn't really think of just explicitly specifying the libraries as part of the CoinUtils link line. It's not a very robust solution, since if for some reason, the static library is not available anymore, the CoinUtils build will just stop using it, but it will be hard-coded into the Clp build, which will then fail. But I guess it's an easier solution for now than hacking the build system.

@svigerske
Copy link
Member

svigerske commented Feb 24, 2025

Maybe the old addlibs is still there on the these old stable branches.

Using pkg-config works for me under msys2 somehow, when using current buildtools, but I think it's also relying on libtool doing some of the work.

If you specify linker flags manually, then just make sure to specify them completely.
Clp shouldn't have to think about all possible dependencies of dependencies, which are often blas and lapack (which may then depend on a fortran runtime), and sometimes maybe glpk or asl?

@tkralphs
Copy link
Member Author

Using pkg-config works for me under msys2 somehow, when using current buildtools, but I think it's also relying on libtool doing some of the work.

Does it work for building with cl? I thought that you end up with some non-posix paths, which the non-Windows pkg-config doesn't understand. in any case, pkg-config is installed on the build machine and we're still having this issue. It's a huge pain to debug.

If you specify linker flags manually, then just make sure to specify them completely.

Yup, I think in this case, it's pretty simple because we are linking to C Blas and there is no asl or glpk.

@svigerske
Copy link
Member

I use the old or new Intel compiler, but cl should work similar.

pkg-config --libs coinmumps gives me -L/path/to/install -lcoinmumps.
These flags get used in the libtool call for linking the Ipopt lib.
libtool finds /path/to/install/libcoinmumps.la (yes, .la), which has

# Libraries that this one depends upon.
dependency_libs=' /path/to/mkl-static/mkl_core.lib /path/to/mkl-static/mkl_intel_lp64.lib /path/to/mkl-static/mkl_intel_thread.lib'

So -lcoinmumps /path/to/mkl-static/mkl_core.lib /path/to/mkl-static/mkl_intel_lp64.lib /path/to/mkl-static/mkl_intel_thread.lib are send to the "compiler" for linking the ipopt lib. The "compiler" is the compile script. This digests -L and -l and calls icl with flags it understands (-libpath, libcoinmumps.lib, etc).

Some of this logic was introduced when updating BuildTools in 2021 and also the build systems also of CoinUtils, Clp, Cbc, etc were updated for that. If you have time to spend, then it would be best spend on the blockers for making releases from master.

@tkralphs
Copy link
Member Author

If you have time to spend, then it would be best spend on the coin-or/Cbc#378.

Yes, you are right. Some conda experts were willing to help push things forward and I thought I should capitalize on having that help to just get these conda packages going quickly. But it's always a bit of a rabbit hole.

@tkralphs
Copy link
Member Author

@svigerske OK, I got it working now in the way you suggested. In the end, it turned out I needed to switch to the MKL static libraries, which worked like a charm. As far as pkg-config, it is automatically disabled when the compiler is cl. You're thinking it should work? I remember in the past that it did not. If we could enable that with cl and have it work, that would be great. With the Msys bash installed by conda and using the pkg-config files installed by conda on Windows, I get

pkg-config --libs --static coinutils
-LC:/Users/tkral/miniforge3/envs/test-blas-devel/Library/lib -lCoinUtils C:/Users/tkral/miniforge3/envs/test-blas-devel/Library/lib/lapack.lib C:/Users/tkral/miniforge3/envs/test-blas-devel/Library/lib/cblas.lib 

which has a mix of Windows paths and -lXxx. Is libtools going to be able to handle that?

@tkralphs
Copy link
Member Author

The addlibs.txt is there and contains this:

$ more share/coin/doc/CoinUtils/coinutils_addlibs.txt
-libpath:D:\bld\coin-or-utils_1724328343630\_h_env\Library\lib libCoinUtils.lib C:/Users/tkral/miniforge3/envs/test-blas-dev
el/Library/lib/lapack.lib C:/Users/tkral/miniforge3/envs/test-blas-devel/Library/lib/cblas.lib

What do you think?

@svigerske
Copy link
Member

It's the compile script that translates -L and -l into what (i)cl expects.
pkg-config is not automatically disabled anymore.
This is all when using BuildTools based on current autotools, not stable/1.17.

Using the content from addlibs may work, but the backslashes in the libpath argument could also be an issue. Better replace them by forward slashes.

But if you have something that works now, then better don't touch it again.

@tkralphs
Copy link
Member Author

pkg-config is still automatically disabled in the stable branches, which is what I'm working on. Anyway, you are right, I'll just leave well enough alone now and move on to focusing on getting new stables out. I'll just close this PR.

@tkralphs tkralphs closed this Feb 26, 2025
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