Decoupled adhoc static plugin method invocations on hooks management

[imonteroperez]

Highlights

• Current code of PCT on TransformPom and MultiParentCompileHook hooks is highly coupled to some AbstractMultiParentHook instances (like StructsHook, BlueOceanHook, etc.)

• Current proposal remove the need to explicitly mention each hook involved in as a bad smell design

• In addition, with the current code, if we want to fork the PCT and include new hooks, every time we sync with upstream we will have to deal with including our additional hooks on these two ones (TransformPom and MultiParentCompileHook)

• What is proposed here is to provide a public method on PluginCompatTesterHooks to dynamically retrieve the registered hooks for a given stage, and to use that method on the affected ones to be refactored

• Also it provides a new option arg named -externalHooksJars to specify a comma-separated list of external hooks jar file locations

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!

• Ensure that the pull request title represents the desired changelog entry

• Please describe what you did

• Link to relevant issues in GitHub or Jira

• Link to relevant pull requests, esp. upstream and downstream changes

• Ensure you have provided tests - that demonstrates feature works or fixes the issue

[imonteroperez]

The failure on buildtriggerbadge test seems happening on master branch too. See: https://ci.jenkins.io/blue/organizations/jenkins/jenkinsci-libraries%2Fplugin-compat-tester/detail/master/159/pipeline

[jtnord]

the external hook jars looks OK (would be nice if it did not fail silently though).

Do not understand the other code we enough to spot anything obviously wrong.

[imonteroperez]

Do not understand the other code we enough to spot anything obviously wrong

I tried to explain it in the PR description. Mostly this PR is providing support to decouple everything related with hooks to make it feasible to use and provide external ones from outside the PCT code by itself.

So, some code, mostly from MultiParentCompileHook and TransformPom has been adapted/refactored to avoid having things like:

boolean isStructs = StructsHook.isStructsPlugin(pomData);
boolean isBO = BlueOceanHook.isBOPlugin(pomData);
boolean isDeclarativePipeline = DeclarativePipelineHook.isDPPlugin(pomData);
boolean isCasC = ConfigurationAsCodeHook.isCascPlugin(pomData);
boolean isPipelineStageViewPlugin = PipelineStageViewHook.isPipelineStageViewPlugin(pomData);
boolean isSwarm = SwarmHook.isSwarmPlugin(pomData);
boolean isDeclarativePipelineMigration = DeclarativePipelineMigrationHook.isPlugin(pomData);
boolean isWarningsMG = WarningsNGCheckoutHook.isWarningsNG(pomData);
[...]
if (isDeclarativePipeline || 
    isBO || 
    isCB || 
    isStructs ||
    isCasC ||
    isPipelineStageViewPlugin ||
    isSwarm ||
    isDeclarativePipelineMigration ||
[...]

Which seems like a bad smell and makes it hard to use these hooks if you want to apply it for external ones too. So proposed to get all this dynamically as shown in the PR content, mostly using a new method to retrieve all the hooks before a specific stage. It would get internal ones (from the PCT by itself) and external ones provided by --externalHooksJars and --hookPrefixes.

Happy to clarify more doubts here if needed @jtnord
Hope it helps!

[imonteroperez]

Checks have the same results as master branch. Safe to be merged.

[jglick]

Just found this PR. Is it too late to request that this be improved to use either ServiceLoader (with metainf-services-annotation) or https://github.com/jglick/sezpoz? Scanning bytecode for classes that happen to implement some interface is pretty fragile (as well as more work than the clean solutions).

[MRamonLeon]

I suggested the former, so 👍🏽

[jglick]

I understand that PCT was already using bytecode scanning, but this PR switches it from an unfortunate implementation detail to a public API.