Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[platform_requires] and consumers use of dependencies['somelib'] #16659

Closed
1 task done
klausholstjacobsen opened this issue Jul 12, 2024 · 10 comments
Closed
1 task done

Comments

@klausholstjacobsen
Copy link

klausholstjacobsen commented Jul 12, 2024

Hello
I've read a previous thread about this issue, and understand the complications and limitations.

It would be nice if recipes could detect if a dependency is a normal dependency or a platform_requires.

In the boost recipe I have similar challenges with libiconv. One solution could be to to just probe dependencies for the existence of a dependency and otherwise asume it is an platform_requires? Like this:

image

Regards
Klaus

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@klausholstjacobsen
Copy link
Author

Maybe replace_requires is the way to go. So I create a replacement package for my toolchain/system library and "inject" it with replace_requires?

//Klaus

@AbrilRBS
Copy link
Member

Hi @klausholstjacobsen I'd caution that using replace_requires might have some unintended issues if your recipes expect the original name, see #16443 where we're exploring solutions to these problems, but nothing concrete yet

@memsharded
Copy link
Member

Hi @klausholstjacobsen

I have done some progress in my PR: #16443, if you want to give it a try and see if it works in your case, that would be very valuable feedback.

@memsharded
Copy link
Member

#16443 was merged, providing "referential transparency" for replaced requires in self.dependencies["name"] and components checks, so thig might be the way to go.

Closing this ticket as solved, but please don't hesitate to create new tickets for any further question or issue, thanks for your feedback.

@curlybeast
Copy link

Hmm, this is not quite working for me and I can't figure out why.
My profile uses:

[platform_requires]
zlib/1.2.11
openssl/3.0.7
libcurl/7.76.0

[replace_requires]
zlib/*: zlib/1.2.11
openssl/*: openssl/3.0.7
libcurl/*: libcurl/7.76.0

This is working most places, except for boost with zlib:

ERROR: boost/1.85.0: Error in build() method, line 1150
        self._create_user_config_jam(self._boost_build_dir)
while calling '_create_user_config_jam', line 1537
        contents += create_library_config("zlib", "zlib")
while calling 'create_library_config', line 1519
        aggregated_cpp_info = self.dependencies[deps_name].cpp_info.aggregated_components()
        KeyError: "'zlib' not found in the dependency set"

Looking at the conan code, it looks right to me. Any ideas?
Thanks!

@memsharded
Copy link
Member

Hi @curlybeast

I think this is expected. The moment you use [platform_requires] it really means that the recipe dependency should be completely abstracted away. There is no way to reference to dependencies["zlib"], because zlib is not a Conan dependency at all. So what we did in the PR that closed this to provide the possible replacements for [replace_requires], but I am afraid that beyond that, it is not possible to [platform_requires] zlib in this case.

This is why in the blog post and docs I think we commented this, the summary would be:

  • It is challenging to use [plaform_requires] for regular "library" requires dependencies, because some recipes will hardcode this dependency. It is in general more expected to work for [platform_tool_requires]
  • For doing this kind of replacement the way to go would be to use a [replace_requires] and use a zlib/system recipe that you provide to wrap the system zlib for your platforms.

@curlybeast
Copy link

I think this is expected. The moment you use [platform_requires] it really means that the recipe dependency should be completely abstracted away.

If this is the case, how will recipes expecting to use dependencies["zlib"] work because the dictionary lookups will fail? That's at least what I'm seeing here. This is also not a casual thing but something I see everywhere. I think the proposal is fine if maintainers know about this and/or use a formal API - or use the proper dictionary .get() API to avoid the exception. Just wondering what you would expect maintainers to do in this situation?

There is no way to reference to dependencies["zlib"], because zlib is not a Conan dependency at all.

I see the problem, but I see another problem coming with the current approach - how can we link to the system library if recipes are adding the linker flags based on a dictionary lookup that no longer exists?

This is why in the blog post and docs I think we commented this, the summary would be:

I read the blog too - thanks, good post!

@memsharded
Copy link
Member

how will recipes expecting to use dependencies["zlib"] work because the dictionary lookups will fail?

Because in many cases accessing the dependencies explicitly in recipes is not necessary, and it shouldn't be necessary. The abstractions, build system generators such as CMakeDeps are there to avoid having to do this. Recipes doing this are usually exceptions because they have some special build systems or not following best practices, so recipes need to provision explicitly for those dependencies. If the recipe for example needs to provide the build scripts with the self.dependencies["zlib"].package_folder, then there is a problem, because if we replace zlib by a "platform-requires", it is simply not possible to provide the build scripts with such package-folder. So not a Conan limitation per se.

I see the problem, but I see another problem coming with the current approach - how can we link to the system library if recipes are adding the linker flags based on a dictionary lookup that no longer exists?

Linker flags should also be unnecessary, implemented in the consumer build scripts, or abstracted away by other means. But if it is the dependency recipe, like the conanfile.py from zlib providing some flags, and zlib is replaced with a platform-requires, then those flags will be dropped. It will only work if consuming zlib doesn't depend on any specific knowledge that is in the recipe.

This is the reason why in many cases the zlib/system recipe approach via replace_requires is way more viable than the platform_requires one, because that recipe can implement flags, and provide the correct replacement for consumers needing that extra information.

In other words, if everything was easily usable without Conan, then Conan wouldn't exist. There is a reason why having Conan packages is very convenient, and completely dropping the "representation" that a Conan package recipe brings, takes users to the initial state without Conan, where it is not that easy to use dependencies. And replace_requires system recipes is the thing that can bridge the gap: relatively easy recipes that wraps the system libraries for an homogeneous representation and management of that dependency.

@curlybeast
Copy link

Recipes doing this are usually exceptions because they have some special build systems or not following best practices

Yes, it's actually boost 1.85 that's problematic for me here. I think the main reason they have code for this is because zlib is optional? I suspected they were outside the recommended approaches though with this :)

Linker flags should also be unnecessary, implemented in the consumer build scripts,

Remember, I'm talking about recipes (conanfile.py) not so much the CMake/Conan side of all this.

In other words, ...

Yea, I agree. To be fair, it's pretty hard to get this right in a general way. Where I work, some engineers want to use system libraries AND 3rd party libraries together. This is difficult to begin with, but I think this becomes increasingly problematic and it's often better to go with one approach or the other. Sadly, the main reason we're using Conan is also because the system doesn't provide the libraries we need :)

Thanks again for the help / support!

@memsharded
Copy link
Member

Yes, it's actually boost 1.85 that's problematic for me here. I think the main reason they have code for this is because zlib is optional? I suspected they were outside the recommended approaches though with this :)

Yes, this is the relevant code in boost recipe:

if self._with_iconv:
     flags.append(f"-sICONV_PATH={self.dependencies['libiconv'].package_folder}")
if self._with_icu:
     flags.append(f"-sICU_PATH={self.dependencies['icu'].package_folder}")

As Boost build system uses these cli args, then it is reasonable that they need these self.dependencies[].package_folder. The zlib case is similar, but it is enabled by default, so it will fail. The thing is that the boost build system might not have a mechanism to find things in the system, as CMake does with find_package() which will look for things in the system, and has a slightly better chance to be able to do "transparent" replacements. But it is not necessarily that boost is using discouraged approaches, it is just an effect of the lack of standardization in locating dependencies in the ecosystem, which makes quite challenging to be able to platform_requires for most regular libraries, because there are many build systems that will not have the capability to search for libraries in the system.

Yea, I agree. To be fair, it's pretty hard to get this right in a general way. Where I work, some engineers want to use system libraries AND 3rd party libraries together. This is difficult to begin with, but I think this becomes increasingly problematic and it's often better to go with one approach or the other. Sadly, the main reason we're using Conan is also because the system doesn't provide the libraries we need :)

Yes, exactly :)
Conan is a tool, it cannot obviate the need of doing architectural, design and process decisions of what dependencies are used from where, and expect the tool to absorb all possible variabilities.
The recommendation in general would be go for Conan packages for all regular C and C++ libraries that are typical "application-level" dependencies, but leave the "low-level", more "system" ones (like xorg or opengl) from the system.

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

No branches or pull requests

4 participants