Move crawler/query related logic out of pom generation code

Author: simontoens

README.md

@@ -75,9 +75,10 @@ load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
 git_repository(
     name = "pomgen",
     remote = "https://github.com/salesforce/pomgen.git",
-    commit = "<git-commit-sha>"
+    commit = "<current master HEAD commit-sha>"
 )
 ```
+The `master` branch is always releasable - use the current `HEAD` commit.
 
 You can then run pomgen commands [as documented](examples/hello-world/README.md#before-running-pomgen), for example:
 
@@ -193,4 +194,4 @@ pomgen uses [ruff](https://github.com/astral-sh/ruff). Follow the installation i
 ruff check src tests
 ```
 
-Currently, we just use the default ruff linting configuration.
+Currently, we just use the default ruff linting configuration.

examples/dependency-management/README.md

@@ -1,6 +1,6 @@
 # Dependency Management POM Example
 
-This examples shows how pomgen can optionally generate a dependencyManagement "companion" pom. This pom file is generate in addition to the regular pom.xml, when the attribute `generate_dependency_management_pom` is to to `True` in the [BUILD.pom file](juicer/MVN-INF/BUILD.pom).
+This examples shows how pomgen can optionally generate a dependencyManagement "companion" pom. This pom file is generated in addition to the regular pom.xml, when the attribute `generate_dependency_management_pom` is to to `True` in the [BUILD.pom file](juicer/MVN-INF/BUILD.pom).
 
 The dependency management pom contains a `<dependencyManagement>` section with the transitive closure of all dependencies of the artifact it was generated for. It uses the `artifact_id` specified in the BUILD.pom file, suffixed with `.depmanagement`.
 

misc/extdeps_pomgen.py

@@ -50,15 +50,13 @@ def _parse_arguments(args):
 
 
 class ThirdPartyDepsPomGen(pom.DynamicPomGen):
+
     def __init__(self, workspace, artifact_def, dependencies, pom_template):
         super(ThirdPartyDepsPomGen, self).__init__(workspace, artifact_def,
                                                    dependency=None,
                                                    pom_template=pom_template)
         self.dependencies = dependencies
 
-    def _load_additional_dependencies_hook(self):
-        return self.dependencies
-
 
 def _starts_with_ignored_prefix(line):
     for prefix in IGNORED_DEPENDENCY_PREFIXES:
@@ -71,11 +69,12 @@ def main(args):
     repo_root = common.get_repo_root(args.repo_root)
     cfg = config.load(repo_root)
 
-    # For the primary function of pomgen (generating pom.xml files for publishing)
-    # there are sometimes maven_install namespaces that are ignored in .pomgenrc.
-    # These are identified as maven_install paths that begin with - .
-    # For extdeps, we need to have full access to all maven_install namespaces, so
-    # we tell maveninstallinfo to not honor the excludes.
+    # For the primary function of pomgen, generating pom.xml files for
+    # publishing to Nexus or similar, there are sometimes maven_install
+    # namespaces that are ignored in .pomgenrc
+    # These are identified as maven_install paths that begin with -
+    # For extdeps, we need to have full access to all maven_install namespaces,
+    # so we tell maveninstallinfo to not honor those excludes
     allow_excludes = False
 
     mvn_install_info = maveninstallinfo.MavenInstallInfo(cfg.maven_install_paths, allow_excludes)
@@ -141,7 +140,7 @@ def main(args):
 
     pomgen = ThirdPartyDepsPomGen(ws, artifact_def, dependencies,
                                   cfg.pom_template)
-    pomgen.process_dependencies()
+
     return pomgen.gen(pom.PomContentType.RELEASE)
 
 

src/crawl/artifactprocessor.py

@@ -22,8 +22,14 @@ def augment_artifact_def(repo_root_path,
                          art_def,
                          source_exclusions,
                          change_detection_enabled):
+
+    # library path
     art_def.library_path = _get_library_path(repo_root_path, art_def)
 
+    # build file exists?
+    art_def._has_build_file = _has_build_file(repo_root_path, art_def)
+
+    # release state
     if art_def.released_version is None or art_def.released_artifact_hash is None:
         # never released?
         art_def.requires_release = True
@@ -49,6 +55,15 @@ def augment_artifact_def(repo_root_path,
     return art_def
 
 
+def _has_build_file(repo_root_path, art_def):
+    path = os.path.join(repo_root_path, art_def.bazel_package)
+    if os.path.exists(os.path.join(path, "BUILD")):
+        return True
+    if os.path.exists(os.path.join(path, "BUILD.bazel")):
+        return True
+    return False
+
+
 def _get_library_path(repo_root_path, art_def):
     """
     Starts at the path the specified artifact lives at and "walks up" to find  

src/crawl/buildpom.py

@@ -17,7 +17,7 @@
 
 class MavenArtifactDef(object):
     """
-    Represents an instance of a maven_artifact rule defined in BUILD.pom file.
+    Represents an instance of a maven_artifact rule defined in a BUILD.pom file.
     Information from the BUILD.pom.released file is added, if that file exists.
 
 
@@ -67,34 +67,39 @@ class MavenArtifactDef(object):
 
     ==== Read out of the optional BUILD.pom.released file ====
 
-    released_version: the previously released version to Nexus
+    released_version: the previously released version to Nexus.
 
     released_artifact_hash: the hash of the artifact at the time it was 
-        previously released to Nexus
+        previously released to Nexus.
 
 
-
-    ===== Internal attributes (never specified by the user) ====
+    ===== Internal attributes (never specified by the user in config/md files)
 
     deps: additional targets this package depends on; list of Bazel labels.
-        For example: deps = ["//projects/libs/servicelibs/srpc/srpc-thrift-svc-runtime"]
+        For example: deps = ["//projects/libs/srpc/srpc-thrift-svc-runtime"]
+        The deps attribute is typically only used by tests, that's why it is
+        listed here under "internal attributes", although it is specified in the
+        BUILD.pom file.
 
-        The deps attribute is only used by tests.
+    bazel_package: the bazel package (relative path to the directory) where the 
+        MVN-INF directory and the build file live. The build file is not
+        guaranteed to exist (see attr below) because pomgen supports "pom only"
+        artifacts, which are foreign to bazel.
 
-    bazel_package: the bazel package the BUILD (and MVN-INF/) files live in
+    has_build_file: whether the bazel package actually has a build file.
 
-    bazel_target: the bazel target that builds this artifact
+    bazel_target: the bazel target that builds this artifact.
 
     library_path: the path to the root directory of the library this artifact
-        is part of
+        is part of.
 
     requires_release: whether this artifact should be released (to Nexus
-        or local Maven repository)
+        or local Maven repository).
 
-    release_reason: the reason for releasing this artifact
+    release_reason: the reason for releasing this artifact.
 
     released_pom_content: if the file pom.xml.released exists next to the 
-        BUILD.pom file, the content of the pom.xml.released file
+        BUILD.pom file, the content of the pom.xml.released file.
     =====
 
 
@@ -144,6 +149,7 @@ def __init__(self,
         self._requires_release = requires_release
         self._release_reason = None
         self._released_pom_content = released_pom_content
+        self._has_build_file = False
 
         # data cleanup/verification/sanitization
         # these are separate methods for better readability
@@ -213,6 +219,10 @@ def released_artifact_hash(self, value):
     def bazel_package(self):
         return self._bazel_package
 
+    @property
+    def has_build_file(self):
+        return self._has_build_file
+
     @property
     def bazel_target(self):
         return self._bazel_target

src/crawl/crawler.py

@@ -534,9 +534,15 @@ def _crawl(self, package, dep, parent_node, follow_references):
 
             self.package_to_artifact[package] = artifact_def
             self.library_to_artifact[artifact_def.library_path].append(artifact_def)
-            pomgen = self._get_pom_generator(artifact_def, dep)
+            if dep is None:
+                # make a real dependency instance here
+                # this is a bootstrapping problem: the root
+                # artifacts (that we start with) have nothing pointing at them
+                dep = dependency.new_dep_from_maven_artifact_def(artifact_def)
+            pomgen = pom.get_pom_generator(self.workspace, self.pom_template,
+                                           artifact_def, dep)
             self.pomgens.append(pomgen)
-            source_deps, ext_deps, all_deps = pomgen.process_dependencies()
+            source_deps, ext_deps, all_deps = self._discover_dependencies(artifact_def, dep)
             self.target_to_dependencies[target_key] = all_deps
             if self.verbose:
                 logger.debug("Determined deps for artifact: [%s] with target key [%s]" % (artifact_def, target_key))
@@ -556,14 +562,58 @@ def _crawl(self, package, dep, parent_node, follow_references):
             self._store_if_leafnode(node)
             return node
 
-    def _get_pom_generator(self, artifact_def, dep):
-        if dep is None:
-            # make a real dependency instance here so we can pass it along
-            # into the pom generator
-            dep = dependency.new_dep_from_maven_artifact_def(artifact_def)
-        return pom.get_pom_generator(self.workspace, 
-                                     self.pom_template, artifact_def,
-                                     dep)
+    def _discover_dependencies(self, artifact_def, dep):
+        """
+        Discovers the dependencies of the given artifact (bazel target).
+
+        This method returns a tuple of 3 (!) lists of Dependency instances:
+            (l1, l2, l3)
+            l1: all source dependencies (== references to other bazel packages)
+            l2: all external dependencies (maven jars)
+            l3: l1 and l2 together, in "discovery order"
+        """
+        assert artifact_def is not None
+        assert dep is not None, "dep is None for artifact %s" % artifact_def
+        all_deps = ()
+        if artifact_def.deps is not None:
+            all_deps = self.workspace.parse_dep_labels(artifact_def.deps)
+        if artifact_def.has_build_file:
+            all_deps += self._query_dependencies(artifact_def, dep)
+
+        source_dependencies = []
+        ext_dependencies = []
+        for dep in all_deps:
+            if dep.bazel_package is None:
+                ext_dependencies.append(dep)
+            else:
+                source_dependencies.append(dep)
+        
+        return (tuple(source_dependencies), 
+                tuple(ext_dependencies), 
+                tuple(all_deps))
+
+    # this method delegates to bazel query to get the value of a bazel target's 
+    # "deps" and "runtime_deps" attributes
+    def _query_dependencies(self, artifact_def, dependency):
+        if not artifact_def.include_deps:
+            return ()
+        else:
+            assert artifact_def.bazel_package is not None
+            assert dependency.bazel_target is not None
+            assert len(dependency.bazel_target) > 0
+            label = "%s:%s" % (artifact_def.bazel_package, dependency.bazel_target)
+            try:
+                # the rule attributes to query for dependencies, typically
+                # "deps" and "runtime_deps"
+                attrs = artifact_def.pom_generation_mode.dependency_attributes
+                dep_labels = bazel.query_java_library_deps_attributes(
+                    self.workspace.repo_root_path, label, attrs,
+                    self.workspace.verbose)
+                deps = self.workspace.parse_dep_labels(dep_labels)
+                return self.workspace.normalize_deps(artifact_def, deps)
+            except Exception as e:
+                msg = e.message if hasattr(e, "message") else type(e)
+                raise Exception("Error while processing dependencies: %s %s caused by %s\nOne possible cause for this error is that the java_libary rule that builds the jar artifact is not the default bazel package target (same name as dir it lives in)" % (msg, artifact_def, repr(e)))
 
     @classmethod
     def _get_target_key(clazz, package, dep, artifact_def=None):

src/crawl/pom.py

@@ -9,7 +9,6 @@
 
 from common import pomgenmode
 import copy
-from crawl import bazel
 from crawl import pomparser
 import os
 import re
@@ -90,38 +89,6 @@ def bazel_package(self):
     def dependency(self):
         return self._dependency
 
-    def process_dependencies(self):
-        """
-        Discovers the dependencies of this artifact (bazel target).
-
-        This method *must* be called before requesting this instance to generate
-        a pom.
-
-        This method returns a tuple of 3 (!) lists of Dependency instances: 
-            (l1, l2, l3)
-            l1: all source dependencies (== references to other bazel packages)
-            l2: all external dependencies (maven jars)
-            l3: l1 and l2 together, in "discovery order"
-
-        This method is not meant to be overwritten by subclasses.
-        """
-        all_deps = ()
-        if self._artifact_def.deps is not None:
-            all_deps = self._workspace.parse_dep_labels(self._artifact_def.deps)
-        all_deps += self._load_additional_dependencies_hook()
-
-        source_dependencies = []
-        ext_dependencies = []
-        for dep in all_deps:
-            if dep.bazel_package is None:
-                ext_dependencies.append(dep)
-            else:
-                source_dependencies.append(dep)
-        
-        return (tuple(source_dependencies), 
-                tuple(ext_dependencies), 
-                tuple(all_deps))
-
     def register_dependencies(self, dependencies):
         """
         Registers the dependencies the backing artifact references explicitly.
@@ -162,15 +129,6 @@ def get_companion_generators(self):
         """
         return ()
 
-    def _load_additional_dependencies_hook(self):
-        """
-        Returns a list of dependency instances referenced by the current 
-        package.
-
-        Only meant to be overridden by subclasses.
-        """
-        return ()
-
     def _artifact_def_version(self, pomcontenttype):
         """
         Returns the associated artifact's version, based on the specified 
@@ -279,10 +237,6 @@ class NoopPomGen(AbstractPomGen):
     def __init__(self, workspace, artifact_def, dependency):
         super(NoopPomGen, self).__init__(workspace, artifact_def, dependency)
 
-    def _load_additional_dependencies_hook(self):
-        return _query_dependencies(self._workspace, self._artifact_def, 
-                                   self._dependency)
-
 
 class TemplatePomGen(AbstractPomGen):
 
@@ -543,10 +497,6 @@ def gen(self, pomcontenttype):
                 "#{dependencies}", self._gen_dependencies(pomcontenttype))
         return content
 
-    def _load_additional_dependencies_hook(self):
-        return _query_dependencies(self._workspace, self._artifact_def,
-                                   self._dependency)
-
     def _gen_dependencies(self, pomcontenttype):
         content = ""
         content, indent = self._xml(content, "dependencies", indent=_INDENT)
@@ -687,9 +637,6 @@ def gen(self, pomcontenttype):
     def get_companion_generators(self):
         return (self.depmanpomgen,)
 
-    def _load_additional_dependencies_hook(self):
-        return self.pomgen._load_additional_dependencies_hook()
-
 
 _INDENT = pomparser.INDENT
 
@@ -702,34 +649,3 @@ def _sort(s):
     the_list = list(s)
     the_list.sort()
     return the_list
-
-
-# this method delegates to bazel query to get the value of a bazel target's 
-# "deps" and "runtime_deps" attributes. it really doesn't belong in this module,
-# because it has nothing to do with generating a pom.xml file.
-# it could move into common.pomgenmode or live closer to the crawler
-def _query_dependencies(workspace, artifact_def, dependency):
-    if not artifact_def.include_deps:
-        return ()
-    else:
-        try:
-            label = _build_bazel_label(artifact_def.bazel_package,
-                                       dependency.bazel_target)
-
-            # the rule attributes to query for dependencies
-            dep_attrs=artifact_def.pom_generation_mode.dependency_attributes
-            dep_labels = bazel.query_java_library_deps_attributes(
-                workspace.repo_root_path, label, dep_attrs,
-                workspace.verbose)
-            deps = workspace.parse_dep_labels(dep_labels)
-            return workspace.normalize_deps(artifact_def, deps)
-        except Exception as e:
-            msg = e.message if hasattr(e, "message") else type(e)
-            raise Exception("Error while processing dependencies: %s %s caused by %s\nOne possible cause for this error is that the java_libary rule that builds the jar artifact is not the default bazel package target (same name as dir it lives in)" % (msg, artifact_def, repr(e)))
-
-
-def _build_bazel_label(package, target):
-    assert package is not None, "package should not be None"
-    assert target is not None, "target should not be None"
-    assert len(target) > 0, "target should not be an empty string for package [%s]" % package
-    return "%s:%s" % (package, target)