Feature/validate

[memsharded]

Changelog: Feature: Introduce a new BINARY_INVALID mode for more flexible definition and management of invalid configurations.
Docs: conan-io/docs#1947

Close #7591

This PR satisfies 2 current proposals/requests:

• The need to specify "impossible" binaries after the graph is fully compute, options are propagated down to consumers, real requirements are also updated (not declared ones, but the actual computed ones, there can be overrides). Feature request in [feature] Method to be run after the graph is resolved #7591

• The need to specify that something cannot be built, but it is possible to have a compatible package defined in package_id(), compatible_packages. PR [feat] Introduce exception raised in 'configure' that will propagate only if 'build' #6952

The current proposal provides a relatively straightforward solution for both by adding a new explicit model of BINARY_INVALID, in the same way we already have a BINARY_UNKNOWN. This model will also be good for future installation f build_requires that do not exist in a host system, because they only exist in the build context, without needing to explicitly define the build profile.

It also supports asking for "what/if need to build" with conan info, it can explicitly return ID: INVALID without raising an exception, so it can be easily used in ConanCenter for example.

There are still some corner cases to be managed (like the processing after a BINARY_UNKNOWN, if that case can happen), or how BINARY_INVALID can produce a BINARY_UNKNOWN if required downstream, but I prefered to propose the core concept before completing it.

[jgsogo]

I would like to have this new error/exception/mark closer to the place where ConanInvalidConfiguration is supposed to be. I think it will make recipes more maintainable, these two events usually share some common logic.

def configure(self):
    if self.settings.os == 'Windows' and self.settings.compiler == 'Visual Studio':
       if self.options.shared:
          raise ConanInvalidConfiguration
       if self.settings.compiler.version < 14:
          raise ConanInvalidConfiguration
       if self.settings.compiler.version > 15 and self.settings.compiler.cppstd >= 14:
         # New mark, it is not going to build
         self.XXXXX-invalid/build_error_message = "It won't build with C++14"
     

When you start to add options and versions to the conditions above it starts to be hard to maintain

[memsharded]

I would like to have this new error/exception/mark closer to the place where ConanInvalidConfiguration is supposed to be. I think it will make recipes more maintainable, these two events usually share some common logic.

Yes, but then what to do with all the logic that needs to be evaluated after the graph is fully computed (downstream propagation of info), like the versions of the dependencies, or options of the dependencies? That was the request of the validate() function. I might move this logic from the package_id() to a possible validate() function, but I think the issue is still the same, the validation checks needs to move from configure() to the new place, either validate() or package_id().

[jgsogo]

No, once the validate() is available, everything from configure() related to ConanInvalidConfiguration error will be moved there.

Right now some checks have been moved from configure() to build() just because we haven't the validate() ready.

[memsharded]

No, once the validate() is available, everything from configure() related to ConanInvalidConfiguration error will be moved there.

Good, I can implement explicitly the validate() function, the truth is that I did exactly that, and I changed it to be in the package_id() because it was way easier regarding compatible_packages. Because in package_id() it is already there, explicitly and clear, so zero extra effort is needed. If moved to another validate() function, you need to be a bit careful with the compatible_packages.

I'll do that change again, I like the idea that is a dedicated function with those semantics, and we will see the rough edges with compatible_packages there.

[jgsogo]

Yes, maybe this feature has to be implemented on top of the validate() function. Remember that the validate() was first requested to check the final value of options coming from dependencies (not for this raise-if-cannot-build).

[memsharded]

Moved to a validate() function, please review. Note the necessary syntax to invalidate compatible packages if necessary (depending on the user logic).

[jgsogo]

About the validate method, probably it deserves its own pull-request:

• We need to test exhaustively that all the options (requirements) are set and accessible and they have the final value. This will be the immediate usage of the validate() function in ConanCenter... there is a lot of people waiting for it according to [feature] Method to be run after the graph is resolved #7591, and they are top contributors of ConanCenter eager to use the feature.
• Once we start to raise a ConanInvalidConfiguration from this function, the CI will use it to detect configurations that it shouldn't build via conan info.

I'm not sure if the validate() function can be experimental.

---

Then, we have the feature of this PR, an exception that will be raised only if we are trying to build the package. I don't understand the test here:

           class Pkg(ConanFile):
               settings = "os", "build_type"

               def validate(self):
                   if self.settings.os == "Windows":
                       self.info.invalid = "Windows not supported"
                       for c in self.compatible_packages:
                           c.invalid = "Windows not supported"

               def package_id(self):
                   if self.settings.build_type == "Debug":
                       compatible_pkg = self.info.clone()
                       compatible_pkg.settings.build_type = "Release"
                       self.compatible_packages.append(compatible_pkg)

Why is it needed to provide a way to invalidate compatible packages? Those packages are not going to be built (in this command), this exception will raise only if the user runs a command to build the package. I'm missing something here.

[memsharded]

Changes done, now validate() doesn't need to manage compatible_package and launches ConanInvalidConfiguration. Added more tests for options and requires, please review @jgsogo

[jgsogo]

LGTM! 🎉

I'll also write a test checking the JSON output of the conan info command:

[
    {
        "reference": "pkg/0.1",
        ...
        "id": "INVALID",
        ...
    }
]

If ConanCenter will start to use this new value ("id": "INVALID",) to identify these configurations, we need a test.

[jgsogo]

The one documented is self.deps_cpp_info['dep'].version, which is accesible in build() method, and it is already used in some recipes. Is it possible to have that information populated here (for consistency, maybe Conan 2.0 provides a different way to access information from nodes)?

[memsharded]

This would be extremely challenging, as the deps_cpp_info is propagated at the "BinaryInstaller" stage, when we are going to install the binaries, so this would be impossible to be evaluated in a conan info context. We would need to move all the logic of propagating the cpp_info from the "BinaryInstaller" stage to the "BinaryAnalyzer" stage, and the risk is too high. I would say we need to move the recipes to the new syntax self.requires["dep"].ref.version (and I don't love it either, but I think that belongs to a different discussion in which we better model the way to access upstream information)

[jgsogo]

I think that belongs to a different discussion in which we better model the way to access upstream information

Maybe this PR should wait, checking the version will be used for sure (API breaks, i.e fmt). Is there any proposal?

[memsharded]

No, there is no proposal, and that would be probably connected somehow to the graph model. I think this proposal is important and we want to start working on it asap, but that also means that this validate() proposal will be stuck for a few months. If we are fine with waiting that long, then good, we can wait. Otherwise the syntax self.requires["dep"].ref.version seems to work, there is already 1 recipe in ConanCenter using (and parsing from text self.requires["dep"] to extract the version), maybe we want to provide a simplified access at the moment to be able to release the validate()?

[jgsogo]

We can define how it will look like and start to connect bits of information, IMO it is not all-or-nothing. We can define the API and provide (and document) now only the version for direct requirements, in future releases we will add more information step by step.

[memsharded]

I have added the feature and testing for invalidating dependents of invalid packages that depend with > full_package_mode.

Please @jgsogo have a look.

[jgsogo]

👍 Like it, something really needed

cc/ @uilianries @SSE4 @danimtb Let's wait a bit before using it for recipes in ConanCenter, especially we need to decide how to get the version of the requirements