Open SpaceIm opened 1 year ago
The thing is that MSVC
in CMakeLists.txt belongs to build time, while package_info()
must work irrespective of the build. Then it is not possible to know the CMake generator that was used at package_info()
, because also the binary would be the same irrespective of the generator. That means that the library name is not well defined for all cases, it seems there is nothing Conan can do to implement a correct is_cl_like()
helper for all cases.
I'd say that this needs to be fixed at the package()
level? Why not just renaming the lib (can also be done at build time, or even patch the build scripts)?
Then it is not possible to know the CMake generator that was used at package_info()
MSVC value doesn't depend on generator, only compiler. It's deterministic at package_info()
time as soon as compiler settings allow to know whether it's a cl like compiler.
I'd say that this needs to be fixed at the package() level? Why not just renaming the lib (can also be done at build time, or even patch the build scripts)?
In conan-center we honor lib file names of upstream. And moreover we can't patch upstream CMakeLists forever, libs maintainers won't accept a patch like that in their source code which would introduce a breaking change to lib name (due to a conan limitation).
MSVC value doesn't depend on generator, only compiler. It's deterministic at package_info() time as soon as compiler settings allow to know whether it's a cl like compiler.
No, it is not. The clang
compiler, in Windows, building MSVC compatible binaries linking with the MSVC runtime and building with Ninja or MinGW Makefiles, does not have the MSVC
variable set in CMakeLists.txt.
MSVC value doesn't depend on generator, only compiler. It's deterministic at package_info() time as soon as compiler settings allow to know whether it's a cl like compiler.
No, it is not. The
clang
compiler, in Windows, building MSVC compatible binaries linking with the MSVC runtime and building with Ninja or MinGW Makefiles, does not have theMSVC
variable set in CMakeLists.txt.
You mean that the same compiler can work with Visual Studio generatorI, and it would set MSVC to TRUE? It's a complete mess, looks like a CMake bug.
Yes, if using Visual Studio generator, it uses the internal bundled ClangCL, but it is the same compiler as external LLVM/Clang, and they are binary compatible, so for Conan settings
, they are the same, and independent from the actual CMake generator used.
I see. Well that's a big problem, because such complexity to properly handle libraries having MSVC
branching (it's very common) can't be delegated to conan-center recipes (I mean like patching CMake files, renaming libs etc), it's too much work & it's fragile, just to handle a specific compiler.
I think that a rename
in the package()
method should work relatively well and without major issues.
Or why not collect_libs()
? I really don't like it, but it is massively used also in ConanCenter, and that avoids completely deducing different names for libraries?
conan build should be deterministic and predictable, it means:
collect_libs
makes build unpredicated, on one machine it could produce one library name, on another machine it could produce a different one (e.g. if someone specific conf for CONAN_CMAKE_GENERATOR
in his profile). it makes troubes and headache to troubleshoot that.
rename
makes conan build diverge from the upstream, breaking user's expectations and changing behavior. it also requires people to change their makefiles (e.g. replace -lz
via lzlib
or whatever), means it's too intrusive.
collect_libs makes build unpredicated, on one machine it could produce one library name, on another machine it could produce a different one (e.g. if someone specific conf for CONAN_CMAKE_GENERATOR in his profile). it makes troubes and headache to troubleshoot that.
Should we ban collect_libs
from ConanCenter, is that what you mean? Because it is used in >150 recipes.
In any case collect_libs
collect the names created by the build system. This is nothing related to Conan. If the build system creates different names under different circumstances without being well-defined, the alternatives are:
package()
methodcollect_libs
or some similar strategy.rename makes conan build diverge from the upstream, breaking user's expectations and changing behavior. it also requires people to change their makefiles (e.g. replace -lz via lzlib or whatever), means it's too intrusive.
I don't care about divergence from the upstream if things are broken. The priority should be things should work, then match to upstream, not the opposite. If people are consuming Conan packages, it is also not expected that they will have -lzlib
hardcoded in their build scripts. There are Conan generators to abstract away the dependencies information, it is impossible to match every upstream expectation and not requiring any change, ever in any consumer build script. And recall that we are talking about some CMake build scripts that will output a different library binary name depending on a very specific configuration of the build, even if they output clang binary compatible binaries. Do we really think that the name of Zlib library should be different if built with LLVM/Clang and Ninja generator and Visual Studio generator, if the compiler is always CLang?
collect_libs makes build unpredicated, on one machine it could produce one library name, on another machine it could produce a different one (e.g. if someone specific conf for CONAN_CMAKE_GENERATOR in his profile). it makes troubes and headache to troubleshoot that.
Should we ban
collect_libs
from ConanCenter, is that what you mean? Because it is used in >150 recipes.
Not ban, but avoid as much as possible. We use collect_libs either for historical reasons, or because original recipe author was too lazy to find true lib name, or because it's not predictable based on settings/options (generator dependent for example, which is ugly I agree).
collect_libs
is not used when there are components.
In any case
collect_libs
collect the names created by the build system. This is nothing related to Conan. If the build system creates different names under different circumstances without being well-defined, the alternatives are:
- Patch the build system to get the same name
- Rename the files in
package()
method- Collect the generated library names, with
collect_libs
or some similar strategy.rename makes conan build diverge from the upstream, breaking user's expectations and changing behavior. it also requires people to change their makefiles (e.g. replace -lz via lzlib or whatever), means it's too intrusive.
I don't care about divergence from the upstream if things are broken. The priority should be things should work, then match to upstream, not the opposite. If people are consuming Conan packages, it is also not expected that they will have
-lzlib
hardcoded in their build scripts. There are Conan generators to abstract away the dependencies information, it is impossible to match every upstream expectation and not requiring any change, ever in any consumer build script. And recall that we are talking about some CMake build scripts that will output a different library binary name depending on a very specific configuration of the build, even if they output clang binary compatible binaries. Do we really think that the name of Zlib library should be different if built with LLVM/Clang and Ninja generator and Visual Studio generator, if the compiler is always CLang?
You don't take into account that downstream users are not always conan users, but also recipes based on build systems which are not aware of conan paradigm. We have to avoid patches as much as possible, because it's a maintenance burden.
Should we ban
collect_libs
from ConanCenter, is that what you mean? Because it is used in >150 recipes.
I personally never liked and recommended this one since its inception, I wish it could be banned - it's completely non-transparent what does it capture (it may capture different things for different users) and causes issues (like, it may capture more libraries than needed - some libraries are not needed to be linked by default; or libraries with same name in different directories, or whatever). realistically speaking, bad might be too sound, but it might be flagged as a general warning and slowly deleted in new PRs.
Patch the build system to get the same name
IMO patch should always be a last measure - all these patches need to be maintained (=rebased for new releases), and they make behavior of conan build diverge from the upstream (which is a source of confusion by itself)
Rename the files in package() method
you may rename, but it might be too fragile (especially, for things like #pragma comment(lib)
or dlopen
in source code, whenever library name is significant) and cause even more bugs. but, I agree, sometimes it's unavoidable, e.g. if produced name are incorrect and cannot be used (like, generating .a for MSVC builds is in general a misbehavior). the same as with patch method, it causes conan build diverge from upstream.
Collect the generated library names, with collect_libs or some similar strategy.
about collect_libs
- already answered.
but an actual and right strategy is to try to integrate conan closer with build systems such as CMake, and define some interoperability layer, e.g. with CMake File API, or similar approach for other build system. for many libraries, package_info
already could be hundreds of lines long, and populated with a lot of complicated conditions to describe correct library names for all the possible options and settings combinations. very fragile, very hard to maintain. IMO working together with build system developers for better C++ ecosystem is the right way to go.
I don't care about divergence from the upstream if things are broken.
I don't like wording I don't care
, but here it's a solid border to define what is broken. if upstream decided to do something, we follow, as they might have their own good reasons to do it (by default, we assume good will and no mistake, in other words, praesumptio innocentiae). if it's really a broken thing, we first report upstream and ask to correct. as a last stand, if upstream is not responsive, e.g. abandoned or closed project, we may try to fix ourselves, but that's an already a custom build, which is different from vanilla by its definition (= custom cci.date release).
The priority should be things should work, then match to upstream, not the opposite.
yes, working builds, but different from upstream, are better than no builds at all, or non-working builds. but here it's not the situation of non-working build, it's just a sophisticated upstream logic to select a library name.
If people are consuming Conan packages, it is also not expected that they will have -lzlib hardcoded in their build scripts.
they already used it even before conan, and will continue to use. conan might be used by some projects, but they still have a requirement to be able to be built without conan, with old-fashioned methods (like, with dependencies installed from apt-get or whatever).
Do we really think that the name of Zlib library should be different if built with LLVM/Clang and Ninja generator and Visual Studio generator, if the compiler is always CLang?
IMO, it's not our business to decide. upstream chooses to produce library names for their own logic and their own use-cases, and we should follow. for instance, in some build systems (e.g. b2) you can build x86 and x64 together in one shot, and ask b2 to encode architecture into the library name as a tag, so libraries for the same architecture may perfectly co-live in the same folder. in case of zlib, underlying compiler might not be the only important thing between these two builds, what if they activate different compiler flags or preprocessor definitions in these two builds? they might not be 100% compatible.
if we still think different library name per generator is abuse or misuse, let's report it upstream and see their motivation. if they don't answer, we can try to fix ourselves, but again, people using -lzlib
, #pragma comment lib
, dlopen
or similar interfaces might be affected by that patch (despite the fact we all dislike hard-coding library names anywhere, and recommending higher level abstractions, like pkg-config or cmake find package, it's still widely encountered situation).
In the same vein, a helper is_mingw()
would be very useful, I still don't know how to detect from a properly written profile if compiler comes from MinGW or is a cl like compiler, so we can't predict what to write in package_info()
when there is a logic like if(MINGW)
in a CMakeLists.
I read https://blog.conan.io/2022/10/13/Different-flavors-Clang-compiler-Windows.html, and from my understanding current settings in conan are not sufficient to disambiguate each clang flavor in a recipe. Different clang flavors share the same settings, so recipes can't know from settings if compiler is clang-cl or llvm-clang. Without proper fields in profile to disambiguate, and helpers in conan, there is no hope for robust recipes support of these different clang windows flavors.
Helpers I would advice:
Improve msvc_runtime_flags & is_msvc_static_runtime to support compiler with a runtime attribute.
@memsharded there are really too many issues in conan-center related to compilation with different windows clang flavors. It's really important that conan profiles can explicitly express which flavor a user is using, so that recipes can rely on this information to go into correct recipe branch when it's relevant.
And it's not just about lib name depending on MSVC
variable in CMake we could handle with collect_libs()
(I don't even take into account recipes with many components where collect_libs()
is a no go). There are several other build logic which can change depending on clang flavor:
CMake has been supporting this with CMAKE_<LANG>_SIMULATE_ID
ever since 3.0 for Intel ICL pretending to be MSVC. Since v3.14 CMAKE_<LANG>_COMPILER_FRONTEND_VARIANT
is also supported.
What do you recommend end users do about this problem at the moment?
Any update for this issue?
I've started to put code like this in conancenter, but it's not ideal and most contributors don't know it's needed for proper support of ClangCL in several recipes: https://github.com/conan-io/conan-center-index/pull/24071 https://github.com/conan-io/conan-center-index/pull/24181 https://github.com/conan-io/conan-center-index/pull/24951 https://github.com/conan-io/conan-center-index/pull/25040
There might be different situations:
MSVC
branch in upstream CMakeLists in order to:
package_info()
(example: libpng, minizip-ng, protobuf, sdl)MSBuild
or NMake
instead of Autotools
in recipes with several build systems (example: libdeflate, libpgeg, libmp3lame, openssl)
Currently conan provides
is_msvc()
method which returns True ifcompiler=Visual Studio
orcompiler=msvc
in settings. In conan-center recipes, this method is widely used for different logics, and one of them is for some branching in package() and package_info() depending onMSVC
variable in upstream CMakeLists (conditional lib name, renaming of files, injection of definitions etc). ButMSVC
variable in CMake is not only True for Visual Studio, but also for any cl like compiler like clang-cl: https://cmake.org/cmake/help/latest/variable/MSVC.html The problem is that depending on clang flavor on Windows, it may or may not be a cl like compiler for CMake. It leads to wrong assumption in several recipes, for example zlib wherecpp_info.libs
may be wrong depending on clang flavor: https://github.com/conan-io/conan-center-index/issues/13474For example a CMakeLists may have this:
so that in conan recipe we usually write:
It's very common, and it's broken for clang-cl.
So some folks using clang-cl were disappointed and began to put this kind of snippet in CCI recipes (and this time it's broken for non cl-like clang flavors on windows, so other folks complain that their clang flavor on windows doesn't work anymore):
So it would be nice if conan could provide a method to perfectly mimic https://cmake.org/cmake/help/latest/variable/MSVC.html logic. It may be another method than
is_msvc
, or an additional argument tois_msvc
in order to switch to CMakeMSVC
behavior.