[feature] Add hooks for validate method #17856

uilianries · 2025-02-26T10:13:28Z

Motivation

Hello!

This PR brings a new hooks for validate() methods

The intention is to help CI services validate a build without touching the recipe, by adding an extra layer of validations. For instance, the CI environment could not build a determined package due to custom settings, or some system library is not available because the O.S. is too old.

Details

The validate don't have a specific command, like export or build, so I used conan create to get a ride and check if is working.

Also, the method is not defined by default, which means, the hook will only run when this method is defined in the recipe.

EDIT: This PR originally covered validate_build too, but the PR scope is now reduced to only cover validate. See the thread #17856 (comment)

Automation

Changelog: Feature: Add hooks for validate method: pre_validate and post_validate.
Docs: conan-io/docs#4013

/cc @jcar87

Refer to the issue that supports this Pull Request.
If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
I've read the Contributing guide.
I've followed the PEP8 style guides for Python code.
I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

conans/client/graph/graph_binaries.py

memsharded · 2025-03-03T16:00:13Z

#17879 has been merged to develop2, please merge from develop2 to get rid of HookManager instantiation issue.

memsharded · 2025-03-03T21:54:55Z

conans/client/graph/compatibility.py

        compatibility_file = os.path.join(compatibility_plugin_folder, "compatibility.py")
        if not os.path.exists(compatibility_file):
            raise ConanException("The 'compatibility.py' plugin file doesn't exist. If you want "
                                 "to disable it, edit its contents instead of removing it")
        mod, _ = load_python_file(compatibility_file)
        self._compatibility = mod.compatibility
+        self._hook_manager = HookManager(HomePaths(cache_folder).hooks_path)


You shouldn't instantiate HookManager here anymore.

Done, consuming hook_manager from conan_api.config.hook_manager instead. Please, see the commit 59c6a83

memsharded · 2025-03-03T21:55:31Z

conans/client/graph/graph_binaries.py

        unknown_mode = global_conf.get("core.package_id:default_unknown_mode", default="semver_mode")
        non_embed = global_conf.get("core.package_id:default_non_embed_mode", default="minor_mode")
        # recipe_revision_mode already takes into account the package_id
        embed_mode = global_conf.get("core.package_id:default_embed_mode", default="full_mode")
        python_mode = global_conf.get("core.package_id:default_python_mode", default="minor_mode")
        build_mode = global_conf.get("core.package_id:default_build_mode", default=None)
        self._modes = unknown_mode, non_embed, embed_mode, python_mode, build_mode
+        self._hook_manager = HookManager(HomePaths(self._home_folder).hooks_path)


Do not instantiate here a new HookManager

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries · 2025-03-04T13:35:47Z

conan/internal/conan_app.py

@@ -44,6 +44,7 @@ def __init__(self, conan_api):
        """
        global_conf = conan_api.config.global_conf
        self.global_conf = global_conf
+        self.conan_api = conan_api


Talked with @AbrilRBS about this. It looks better than changing more methods because we need to forward the hook manager to run_validate_package_id

memsharded

I'd like to learn more about the real use cases to be able to figure out the scope of definition and application of these hooks.

memsharded · 2025-03-04T14:58:55Z

conans/client/graph/graph_binaries.py

        # These are the nodes with pref (not including PREV) that have been evaluated
        self._evaluated = {}  # {pref: [nodes]}
        compat_folder = HomePaths(conan_app.cache_folder).compatibility_plugin_path
-        self._compatibility = BinaryCompatibility(compat_folder)
+        self._compatibility = BinaryCompatibility(compat_folder, self._hook_manager)


Do we really want to execute the validate() pre/post hooks for all combinations of binary compatibility?
Can I have some examples of potential usages of the hook, and what does it mean to execute it for every binary compatibility hypothesis?

This is still an open question

I dont think so, I could remove it and add in the future, in case having a case.

For now, we only have the CI case to be covered.

Then lets remove it. It is better to be minimalistic first, then add things when real use cases are requesting it.

Done, removed on the commit 57878a1

I'd still like to see a more real use case for the feature, I haven't fully understood if it makes sense to be in the compatibility or not.

The only real case listed right now is related to the CI: When the CI needs to skip a build, being checked in the pre_validate, but we do not want to change the recipe, by adding a new rule to raise ConanInvalidConfiguration.

About the compatibility I do not see a real usage right now, because it's related to binary compatibility mechanism, sounds be something unrelated to the validate stage.

memsharded · 2025-03-04T14:59:46Z

conans/client/hook_manager.py

+                      "pre_validate_build", "post_validate_build",
+                      "pre_validate", "post_validate",


If we can cover the real-world use cases with just 1 of these hooks, I'd go for that, do the minimal changes to satisfy the need.

So far, we have pre_validate requested in CCI as a real case, so the CI could skip the build for those cases when the glibc version is too old but the project requires a newer version (e.g. Qt), without injecting changes in the validate() which is really consumed by users.

For other cases, @jcar87 can tell us more about it.

pre_validate as a minimum
I think post_validate is useful too, in case ConanInvalidConfiguration is raised in the implementation

@memsharded I'll drop the hook for validate_build.

Done, removed validate_build hook on the commit a71eaaa

Signed-off-by: Uilian Ries <uilianries@gmail.com>

…k-validate

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries added 6 commits February 26, 2025 08:16

Add test for pre_validate_build hook

f0b295f

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Add pre_validate_build hook

2b92afb

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Add post_validate_build hook

a7511f9

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Adapt test to support both validate and validate_build

e3715f2

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Add pre_validate hook

51bdc7d

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Add post_validate hook

58cae01

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries requested review from jcar87 and memsharded February 26, 2025 10:13

AbrilRBS self-requested a review February 26, 2025 10:16

Pass hook manager by Compatibility

16fafd4

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded reviewed Feb 26, 2025

View reviewed changes

conans/client/graph/graph_binaries.py Outdated Show resolved Hide resolved

memsharded mentioned this pull request Mar 3, 2025

moving hookManager to api #17879

Merged

Merge branch 'develop2' into feature/hook-validate

333e335

memsharded requested changes Mar 3, 2025

View reviewed changes

uilianries added 3 commits March 4, 2025 10:04

Revert hook-manager usage

0615850

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Use hook manager from conan API

59c6a83

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Pass missing hook manager to compute_package_id

13e971b

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries commented Mar 4, 2025

View reviewed changes

uilianries marked this pull request as ready for review March 4, 2025 13:37

uilianries requested a review from memsharded March 4, 2025 13:37

memsharded reviewed Mar 4, 2025

View reviewed changes

czoido added this to the 2.14.0 milestone Mar 10, 2025

memsharded self-assigned this Mar 10, 2025

Remove hook support for validate_build

a71eaaa

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries requested a review from memsharded March 10, 2025 10:31

uilianries changed the title ~~[feature] Add hooks for validate and validate_build methods~~ [feature] Add hooks for validate method Mar 10, 2025

uilianries mentioned this pull request Mar 10, 2025

[feature] Add pre_validate post_validate hooks in reference conan-io/docs#4013

Merged

uilianries added 2 commits March 10, 2025 11:59

Merge branch 'develop2' of github.com:conan-io/conan into feature/hoo…

be0cb56

…k-validate

Remove hook manager for compatibility

499afd9

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Remove any trace of hook manager from compatilibity

57878a1

Signed-off-by: Uilian Ries <uilianries@gmail.com>

memsharded approved these changes Mar 10, 2025

View reviewed changes

memsharded merged commit 7b4f3a0 into conan-io:develop2 Mar 10, 2025
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] Add hooks for validate method #17856

[feature] Add hooks for validate method #17856

uilianries commented Feb 26, 2025 •

edited by czoido

Loading

memsharded commented Mar 3, 2025

memsharded Mar 3, 2025

uilianries Mar 4, 2025

memsharded Mar 3, 2025

uilianries Mar 4, 2025

uilianries Mar 4, 2025

memsharded left a comment

memsharded Mar 4, 2025

memsharded Mar 10, 2025

uilianries Mar 10, 2025

memsharded Mar 10, 2025

uilianries Mar 10, 2025

memsharded Mar 10, 2025

uilianries Mar 10, 2025

memsharded Mar 4, 2025

uilianries Mar 4, 2025

jcar87 Mar 10, 2025

uilianries Mar 10, 2025

uilianries Mar 10, 2025

		"pre_validate_build", "post_validate_build",
		"pre_validate", "post_validate",

[feature] Add hooks for validate method #17856

[feature] Add hooks for validate method #17856

Conversation

uilianries commented Feb 26, 2025 • edited by czoido Loading

Motivation

Details

Automation

memsharded commented Mar 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

memsharded left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uilianries commented Feb 26, 2025 •

edited by czoido

Loading