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.

Sign up for GitHub

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

[feature] Add hooks for validate and validate_build methods #17856

Draft
wants to merge 7 commits into
base: develop2
Choose a base branch
from
Draft

[feature] Add hooks for validate and validate_build methods #17856

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f0b295f
Add test for pre_validate_build hook
uilianries Feb 26, 2025
2b92afb
Add pre_validate_build hook
uilianries Feb 26, 2025
a7511f9
Add post_validate_build hook
uilianries Feb 26, 2025
e3715f2
Adapt test to support both validate and validate_build
uilianries Feb 26, 2025
51bdc7d
Add pre_validate hook
uilianries Feb 26, 2025
58cae01
Add post_validate hook
uilianries Feb 26, 2025
16fafd4
Pass hook manager by Compatibility
uilianries Feb 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions conans/client/graph/compatibility.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from conan.internal.cache.home_paths import HomePaths
from conans.client.graph.compute_pid import run_validate_package_id
from conans.client.loader import load_python_file
from conans.client.hook_manager import HookManager
from conan.internal.errors import conanfile_exception_formatter, scoped_traceback
from conan.errors import ConanException
from conans.client.migrations import CONAN_GENERATED_COMMENT
Expand Down Expand Up @@ -106,13 +107,15 @@ def _should_migrate_file(file_path):

class BinaryCompatibility:

def __init__(self, compatibility_plugin_folder):
def __init__(self, cache_folder):
compatibility_plugin_folder = HomePaths(cache_folder).compatibility_plugin_path
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)

def compatibles(self, conanfile):
compat_infos = []
Expand Down Expand Up @@ -143,7 +146,7 @@ def compatibles(self, conanfile):
conanfile.settings = c.settings
conanfile.settings_target = c.settings_target
conanfile.options = c.options
run_validate_package_id(conanfile)
run_validate_package_id(conanfile, self._hook_manager)
pid = c.package_id()
if pid not in result and not c.invalid:
result[pid] = c
Expand Down
10 changes: 7 additions & 3 deletions conans/client/graph/compute_pid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from conan.internal.model.info import ConanInfo, RequirementsInfo, RequirementInfo, PythonRequiresInfo


def compute_package_id(node, modes, config_version):
def compute_package_id(node, modes, config_version, hook_manager):
"""
Compute the binary package ID of this node
"""
Expand Down Expand Up @@ -50,7 +50,7 @@ def compute_package_id(node, modes, config_version):
config_version=config_version.copy() if config_version else None)
conanfile.original_info = conanfile.info.clone()

run_validate_package_id(conanfile)
run_validate_package_id(conanfile, hook_manager)

if conanfile.info.settings_target:
# settings_target has beed added to conan package via package_id api
Expand All @@ -60,24 +60,28 @@ def compute_package_id(node, modes, config_version):
node.package_id = info.package_id()


def run_validate_package_id(conanfile):
def run_validate_package_id(conanfile, hook_manager):
# IMPORTANT: This validation code must run before calling info.package_id(), to mark "invalid"
if hasattr(conanfile, "validate_build"):
with conanfile_exception_formatter(conanfile, "validate_build"):
hook_manager.execute("pre_validate_build", conanfile=conanfile)
with conanfile_remove_attr(conanfile, ['cpp_info'], "validate_build"):
try:
conanfile.validate_build()
except ConanInvalidConfiguration as e:
# This 'cant_build' will be ignored if we don't have to build the node.
conanfile.info.cant_build = str(e)
hook_manager.execute("post_validate_build", conanfile=conanfile)

if hasattr(conanfile, "validate"):
with conanfile_exception_formatter(conanfile, "validate"):
hook_manager.execute("pre_validate", conanfile=conanfile)
with conanfile_remove_attr(conanfile, ['cpp_info'], "validate"):
try:
conanfile.validate()
except ConanInvalidConfiguration as e:
conanfile.info.invalid = str(e)
hook_manager.execute("post_validate", conanfile=conanfile)

# Once we are done, call package_id() to narrow and change possible values
if hasattr(conanfile, "package_id"):
Expand Down
12 changes: 8 additions & 4 deletions conans/client/graph/graph_binaries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from conan.api.model import PkgReference
from conan.api.model import RecipeReference
from conans.util.files import load
from conans.client.hook_manager import HookManager


class GraphBinariesAnalyzer:
Expand All @@ -31,15 +32,16 @@ def __init__(self, conan_app, global_conf):
self._remote_manager = conan_app.remote_manager
# 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(conan_app.cache_folder)
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can make the batch processing at scale in repos like ConanCenter very slow, as this will be instantiated tons of times. Need to be factored out from here (this might be complicated and require some re-architecture)

cc @AbrilRBS

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just need to access the Conan App to capture the hook paths, then, instantiate the hook manager. It would be great doing it in run_validate_package_id directly, but I have no access to Conan App. Is there a way to capture the Conan App globally? I also don't like this current approach of forwarding the hook manager til run_validate_package_id

Copy link
Member

@AbrilRBS AbrilRBS Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that there's an easy lazy load optimization available for the HookManager that would solve the issue, something like this might be worth looking into:
that would almost solve it, but you still would need to modify the class instance instead of self._hooks to avoid the slowness:

diff --git a/conans/client/hook_manager.py b/conans/client/hook_manager.py
index d213b209d..9eb87bdb3 100644
--- a/conans/client/hook_manager.py
+++ b/conans/client/hook_manager.py
@@ -12,11 +12,18 @@ valid_hook_methods = ["pre_export", "post_export",
 
 
 class HookManager:
+    _hooks = None
 
     def __init__(self, hooks_folder):
         self._hooks_folder = hooks_folder
-        self.hooks = {}
-        self._load_hooks()  # A bit dirty, but avoid breaking tests
+
+    @property
+    def hooks(self):
+        if self._hooks is None:
+            # Have this before the call below, or its accesses to hook will be recursive
+            self._hooks = {}
+            self._load_hooks()
+        return self._hooks
 
     def execute(self, method_name, conanfile):
         assert method_name in valid_hook_methods, \

Copy link
Member

@AbrilRBS AbrilRBS Feb 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then if you access/modify just HookManager._hooks, you'll have issues with the tests keping states between each call so, pick your poison I guess



@staticmethod
def _evaluate_build(node, build_mode):
Expand Down Expand Up @@ -397,7 +399,8 @@ def _config_version(self):
return RequirementsInfo(result)

def _evaluate_package_id(self, node, config_version):
compute_package_id(node, self._modes, config_version=config_version)
compute_package_id(node, self._modes, config_version=config_version,
hook_manager=self._hook_manager)

# TODO: layout() execution don't need to be evaluated at GraphBuilder time.
# it could even be delayed until installation time, but if we got enough info here for
Expand Down Expand Up @@ -458,7 +461,8 @@ def _evaluate_single(n):
if node.path is not None:
if node.path.endswith(".py"):
# For .py we keep evaluating the package_id, validate(), etc
compute_package_id(node, self._modes, config_version=config_version)
compute_package_id(node, self._modes, config_version=config_version,
hook_manager=self._hook_manager)
# To support the ``[layout]`` in conanfile.txt
if hasattr(node.conanfile, "layout"):
with conanfile_exception_formatter(node.conanfile, "layout"):
Expand Down
2 changes: 2 additions & 0 deletions conans/client/hook_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from conan.errors import ConanException

valid_hook_methods = ["pre_export", "post_export",
"pre_validate_build", "post_validate_build",
"pre_validate", "post_validate",
"pre_source", "post_source",
"pre_generate", "post_generate",
"pre_build", "post_build", "post_build_fail",
Expand Down
35 changes: 35 additions & 0 deletions test/integration/extensions/hooks/hook_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import textwrap
import pytest

from conan.test.assets.genconanfile import GenConanfile
from conan.test.utils.tools import TestClient
Expand All @@ -21,6 +22,18 @@ def post_export(conanfile):
assert conanfile.export_sources_folder, "export_sources_folder not defined"
assert conanfile.recipe_folder, "recipe_folder not defined"

def pre_validate_build(conanfile):
conanfile.output.info("Hello")

def post_validate_build(conanfile):
conanfile.output.info("Hello")

def pre_validate(conanfile):
conanfile.output.info("Hello")

def post_validate(conanfile):
conanfile.output.info("Hello")

def pre_source(conanfile):
conanfile.output.info("Hello")
assert conanfile.source_folder, "source_folder not defined"
Expand Down Expand Up @@ -111,6 +124,8 @@ def test_complete_hook(self):
assert f"pkg/0.1: {hook_msg} post_package(): Hello" in c.out
assert f"pkg/0.1: {hook_msg} pre_package_info(): Hello" in c.out
assert f"pkg/0.1: {hook_msg} post_package_info(): Hello" in c.out
assert "pre_validate_build()" not in c.out
assert "post_validate_build()" not in c.out

def test_import_hook(self):
""" Test that a hook can import another random python file
Expand Down Expand Up @@ -176,3 +191,23 @@ def build(self):
assert "conanfile.py: [HOOK - my_hook/hook_my_hook.py] post_build_fail(): Hello" in c.out
assert "ERROR: conanfile.py: Error in build() method, line 5" in c.out

@pytest.mark.parametrize("method", ["validate_build", "validate"])
def test_validate_hook(self, method):
""" The validate_build and validate hooks are executed only if the method is declared
in the recipe.
"""
c = TestClient()
hook_path = os.path.join(c.paths.hooks_path, "testing", "hook_complete.py")
save(hook_path, complete_hook)

conanfile = textwrap.dedent("""
from conan import ConanFile
class Pkg(ConanFile):
def {method}(self):
pass
""".format(method=method))
c.save({"conanfile.py": conanfile})

c.run("create . --name=foo --version=0.1.0")
assert f"foo/0.1.0: [HOOK - testing/hook_complete.py] pre_{method}(): Hello" in c.out
assert f"foo/0.1.0: [HOOK - testing/hook_complete.py] post_{method}(): Hello" in c.out
Loading