[bug] Unexpected None package_folder

[mrjoel]

Describe the bug

In finishing our Conan 2 conversion, I'm running into some unexpected cases where package_folder is unexpectedly resulting in being None.

1. In a meta-deps package we have, we iterate through all direct and transitive dependencies, gathering and generating CMake install() commands for aggregating license content information (most packages built as shared=True). In the meta-deps package generate() method we do something roughly like the following. However, for a subset of packages we've found that package_folder at this point is None, causing issues. We've since guarded it to print a warning and skip processing, but with warnings as errors we can't proceed with missing any transitive license information.

   for require, dependency in self.dependencies.items():
       cmake_install_helper(require.ref, dependency.package_folder)
   

   a. The first class of issues we've found this with is where package_type = "header-library", and simply commenting out the package_type offers a crude workaround.

   b. The second package we've found is double-conversion as a transitive dependency of Qt, which is package_type = "library". We're building it as a static package, with or without package_type commented out gives the same results.

2. In a Qt build with shared zlib and zstd, as a kludge (there's probably a cleaner solution), we are copying the zlib and zstd libraries as part of the package() method. In this case, as an example, self.dependencies["zlib"].package_folder is None.

I'm working on a standalone reproducer, but wanted to get the issue out to see if it's already known or obvious.

How to reproduce it

No response

[memsharded]

Hi @mrjoel

Thanks for your feedback.

In finishing our Conan 2 conversion, I'm running into some unexpected cases where package_folder is unexpectedly resulting in being None.

This is not a bug, but expected behavior. There are 2 scenarios where package_folder is None:

• When not installing the package binaries, that is, commands that do not perform a full installation: conan graph, conan lock. These commands will build the dependency graph, but not install the binaries, so the packages will not be guaranteed to be installed, package_info() will not be called, and package_folder will be None
• When package binaries are marked as Skipped binaries. You can see the Skip status in the command output. When a certain package binary is not needed, for example, when a shared library depends on a static library, or an executable depends on a static library, when someone depends on the shared library or executable, the binary for the static library is unnecessary, as it doesn't need to be propagated downstream. Conan mark them as "Skip", and avoid downloading and unzipping those binaries (or even building them), and the package_info() method is not executed and package_folder for those skipped binaries will be None.

Please let me know if this clarifies the behavior.

I will also likely move this to the docs repo, to add there some more explicit documentation of this behavior.

[memsharded]

Just in case, there is a conf to disable this:

$ conan config list skip
...
tools.graph:skip_binaries: Allow the graph to skip binaries not needed in the current configuration (True by default)

Setting it to False forces to not skip the unnecessary dependencies binary packages.

[mrjoel]

When package binaries are marked as Skipped binaries. You can see the Skip status in the command output.

conf to disable this: tools.graph:skip_binaries

Yes, this helps clarify expectations as to the current behavior exhibited, thanks. However, in light of the updated understanding I still have some remaining questions about recommended approaches.

1. When building as shared libraries, instead of copying zlib.dll and zstd1.dll into the Qt package directory (so moc, uic, etc. can be run directly from package folder) there's likely a better way to enable execution to be able to locate dependent shared libraries such as that. What would be the recommended and better supported way to handle that case?

2. For the time being I'm fine configuring a warning which advises to set tools.graph:skip_binaries=False in order to get full license content included representing what is actually used. (That allows us to have warnings-as-errors for CI, but allow developers to ignore the warning locally). However that seems like an odd state of affairs since license content is by convention in the package_folder. Is there a better approach to accomplish this, or is this just the inherent tradeoff required?

[memsharded]

When building as shared libraries, instead of copying zlib.dll and zstd1.dll into the Qt package directory (so moc, uic, etc. can be run directly from package folder) there's likely a better way to enable execution to be able to locate dependent shared libraries such as that. What would be the recommended and better supported way to handle that case?

Regarding the "Skipping" binary behavior, just to clarify, a shared-library will never be "skipped", as it is necessary. Conan needs to be aware that it is a shared library or necessary at runtime, at least with one of:

• package_type = "shared-library"
• or shared=True option
• or the consumer requires it with self.requires("dep/version", run=True) trait

Regarding the execution, lets say that you have an executable moc.exe inside a qt/version package, that depends on a zlib/version package that contains a shared zlib.dll.

When a consumer project depends on self.tool_requires("qt/version"), it should automatically get defined in the generated conanbuild.bat (that includes other files such as conanbuildenv.bat, etc), that will define the PATH environment variable containing the zlib/version "bindir" folder containing the zlib.dll, and the qt/version "bindir" containing the moc.exe. When the consumer calls self.run("moc") the default env="conanbuild" will load and will find both in the system PATH and execute them.

For the time being I'm fine configuring a warning which advises to set tools.graph:skip_binaries=False in order to get full license content included representing what is actually used. (That allows us to have warnings-as-errors for CI, but allow developers to ignore the warning locally). However that seems like an odd state of affairs since license content is by convention in the package_folder. Is there a better approach to accomplish this, or is this just the inherent tradeoff required?

Depending on the license collection model and also the binary distribution model, there can be other approaches. For example, when you are building a shared library that links statically with some other static libraries, at that point in time of the package creation you might want to collect the licenses of the transitive dependencies and re-package them close to the built shared library. That way, that package contains a full "deliverable", that can be extracted and deployed, sent to customers or whatever is necessary.

For the strategy in which all licenses from the dependency graph are captured, I am not sure if I follow. Such warnings might not be necessary. The license collection should probably happen fully automatically in CI, so the tools.graph:skip_binaries=False is defined in the CI, the CI collect the licenses, and that's it? The tradeoff is that all binaries will be downloaded for that specific command collecting the licenses, but this is necessary, as the licenses are artifacts in the package folder. Not sure if I fully understand this question.

[mrjoel]

Regarding the "Skipping" binary behavior, just to clarify, a shared-library will never be "skipped", as it is necessary. Conan needs to be aware that it is a shared library or necessary at runtime, at least with one of

Yep, this now makes sense, thanks.

or the consumer requires it with self.requires("dep/version", run=True) trait

Ah, this might be an interesting option and good fit for our meta-deps package approach. It doesn't seem to be able to apply to transitive requires, but may still be partially useful.

When the consumer calls self.run("moc") the default env="conanbuild" will load and will find both in the system PATH and execute them.

That's indeed the case when running self.run() from Conan is an option. However for our usage it's much more prevalent that the issue is at build execution time where it needs to be run, using CMake AUTO{MOC,RCC,UIC} options, with CMake executing the relevant discovered executable from within the located bin directory. Those executions have nothing to do with Conan and as such aren't impacted by the Conan build env. I'd consider submitting the patch back into conan-center-index, but am not sure what the recommended practice is for situations like this.

you might want to collect the licenses of the transitive dependencies and re-package them close to the built shared library. That way, that package contains a full "deliverable", that can be extracted and deployed, sent to customers or whatever is necessary.

Yeah, we'd considered that, but it's more divergence from conan-center-index base recipes than we'd like to take on, even if it means always forcing tools.graph:skip_binaries=False for CI builds. As you describe, we'll likely have CI jobs always run with that, but still be a warning so a) a misconfiguration triggers a failure since we'll also run CI with warnings-as-errors (except deprecated), and b) local developers doing the build are aware that they're in a non-standard content configuration.

Incidentally, I'm definitely feeling the loss of the conan config set and conan profile update commands for being able to provide easy guidance to developers for targeted values to change. I get that the preference is to use version controlled profiles and settings, but for various reasons our cross-organizational distribution just doesn't make that practical. As a fallback then, it's more arduous to have a developer who isn't (and shouldn't need to be) familiar with Conan to have to run a sed or manually edit a file.

[mrjoel]

There are 2 scenarios where package_folder is None:

I'm continuing to test this and form further feedback. One immediate point is that, even when doing a conan create which should populate the full graph, the graph does not appear to be populated by the time validate() is called.

I was hoping to check the direct behavior desired, i.e. if self.dependencies["zlib"].package_folder is None, however it is always None. I instead will look at checking the tools.graph:skip_binaries config value as an indirect check.

[memsharded]

Incidentally, I'm definitely feeling the loss of the conan config set and conan profile update commands for being able to provide easy guidance to developers for targeted values to change. I get that the preference is to use version controlled profiles and settings, but for various reasons our cross-organizational distribution just doesn't make that practical. As a fallback then, it's more arduous to have a developer who isn't (and shouldn't need to be) familiar with Conan to have to run a sed or manually edit a file.

For developer occasional use, we added the -cc/--core-conf command line arguments that can define core confs, and it is always possible to define regular confs in -c arguments. Also developers don't necessarily need to modify existing profiles. They can have a local file with their conf, and just apply it with multi-profile: -pr=organization_profile -pr=myownprofile_with_my_confs.

The problem here is that profiles now are jinja templates, and that cannot implement round-trip, so conan config set/get are impossible for modifying profiles.

I'm continuing to test this and form further feedback. One immediate point is that, even when doing a conan create which should populate the full graph, the graph does not appear to be populated by the time validate() is called.

Indeed. The validate() call happens before things are installed (downloaded or built), so the packages are not there yet, and package_folder is None there.

I think I still don't fully understand the issue. If I had to collect all transitive licenses, I'd use something like:

conan install -c tools.graph:skip_binaries=False --deployer=mylicense_deployer

Without needing to modify any recipe.

[memsharded]

Any further question? If not, I will move this ticket to the docs repo, to try to improve the documentation regarding this. Thanks for the feedback.

[mrjoel]

I do have more questions I'm trying to work through. I'm inferring that the issues we're encountering are that our pattern of an intermediate meta-deps package breaks the Conan expectations of inferring whether a package is required or not. Even with skip_binaries=False we still aren't getting the desired results of package inclusion, so we're having to comment out the package_type = "header-library" lines from several local recipes.

I'm working on getting a working state with what earlier Conan 2.x supports, and will then dive into the details of implied handling from various package_type trait inference are being manifest. I //think// what I might want is a way in a requires() to say "remove a level of dependency, treat second-level dependencies as though they were direct".

We use our meta-deps as a mechanism to provide versioned package sets, but all packages (including static and header-only) are in fact direct dependencies.

So while Conan expects:

class ProductConan(ConanFile):
    def requirements(self):
        self.requires("libraryA/1.2.3")
        self.requires("libraryB/2.3.4")
        self.requires("libraryC/3.4.5")

What we end up having is:

class ProductConan(ConanFile):
    def requirements(self):
        self.requires("meta-deps/7.8.9")

class MetaDepsConan(ConanFile):
    def requirements(self):
        self.requires("libraryA/1.2.3")
        self.requires("libraryB/2.3.4")
        self.requires("libraryC/3.4.5")

I think most of our issues would go away with a consumer-level requires option to do something like the following, where it needn't actually turn into direct dependencies, but only influence the package trait inference handling.

class ProductConan(ConanFile):
    def requirements(self):
        self.requires("meta-deps/7.8.9", infer_traits_as_if_direct=True)

[memsharded]

One possible recommendation for artificial "version sets" would be to put that logic in a python-requires and reuse it from there. That way you can achieve both, declare all versions in one place, still let users to easily use that version set.

Also, I think that header-only libraries are mostly pass-through, so adding package_type = "header-only" to the meta-deps package, should have the effect that you are looking for in practice, isn't it the case?

If that is not the case, then it would be necessary to have a fully reproducible example that shows the issue. Thanks again for your feedback.