Support to JEP-229 / Fixing compilation on multimodule projects #318

imonteroperez · 2021-11-18T17:41:29Z

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

jglick · 2021-11-18T17:57:55Z

plugins-compat-tester/src/main/java/org/jenkins/tools/test/hook/MultiParentCompileHook.java

+                throw new IOException(String.format("Something is really wrong here! %s is not the parent folder of %s", parentFolder, path.getAbsolutePath()));
+            }
+            // "process-test-classes" not working properly on multi-module plugin. See https://issues.jenkins.io/browse/JENKINS-62658
+            // using "mvn clean install -DskipTests -Dinvoker.skip -Denforcer.skip to skip testing and just compile and install dependencies"


BTW -Pquick-build works but only for plugins using a tolerably recent parent.

plugins-compat-tester/src/main/java/org/jenkins/tools/test/hook/MultiParentCompileHook.java

jglick

Looks OK. Does it in fact work?

plugins-compat-tester/src/main/java/org/jenkins/tools/test/PluginCompatTester.java

jglick · 2021-11-23T15:38:02Z

plugins-compat-tester/src/main/java/org/jenkins/tools/test/PluginCompatTester.java

+            if (!StringUtils.startsWith(line.trim(), "<string>")) {
+                continue;
+            }
+            String mvnModule = line.replace("<string>", "").replace("</string>", "").trim();


This could be a regexp match perhaps, but why are you attempting to parse XML this way? Use e.g. DOM4j.

Yep, I will update, trying a quick-and-dirty approach just to test the entire solution

Yes. Good enough for now I think.

plugins-compat-tester/src/main/java/org/jenkins/tools/test/PluginCompatTester.java

plugins-compat-tester/src/main/java/org/jenkins/tools/test/hook/MultiParentCompileHook.java

jglick · 2021-11-23T15:47:13Z

plugins-compat-tester/src/main/java/org/jenkins/tools/test/hook/MultiParentCompileHook.java

+        if (localCheckoutDir != null) {
+            return false;
+        }


I do not understand why this clause is here. As discussed earlier, having a local checkout dir (with a plain old MRP plugin) is one of the reasons why you might in fact need this patch.

🤔 you are right, I will review this in depth. I included at the very beginning because I wrongly understood #318 (comment)

Definitely an edge case, not very important, just a point of confusion.

plugins-compat-tester/src/main/java/org/jenkins/tools/test/hook/MultiParentCompileHook.java

Co-authored-by: Jesse Glick <jglick@cloudbees.com>

imonteroperez · 2021-11-23T15:58:51Z

Looks OK. Does it in fact work?

Yes! it worked fine, before this PR

[ERROR] Failed to execute goal on project aws-java-sdk-ec2: Could not resolve dependencies for project org.jenkins-ci.plugins.aws-java-sdk:aws-java-sdk-ec2:hpi:1.12.101-999999-SNAPSHOT: Could not find artifact org.jenkins-ci.plugins.aws-java-sdk:aws-java-sdk-minimal:jar:1.12.101-999999-SNAPSHOT

and after this PR

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Amazon Web Services SDK :: Parent 1.12.101-999999-SNAPSHOT:
[INFO] 
[INFO] Amazon Web Services SDK :: Parent .................. SUCCESS [  0.477 s]
[INFO] Amazon Web Services SDK :: Minimal ................. SUCCESS [  0.130 s]
[INFO] Amazon Web Services SDK :: EC2 ..................... SUCCESS [  0.065 s]
[INFO] ------------------------------------------------------------------------
[...]
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running InjectedTest
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 8.335 s - in InjectedTest
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0

jglick · 2021-11-23T16:07:02Z

Probably we should be adding the AWS SDK plugin to jenkinsci/bom once this is released.

cathychan

LGTM!

jglick · 2021-11-29T18:59:23Z

@imonteroperez you need to set a category label for CD before merging a PR. Running https://github.com/jenkinsci/plugin-compat-tester/actions/runs/1517465133 manually to compensate.

Support to JEP-229 / Fixing compilation on multimodule projects

be183e6

jglick mentioned this pull request Nov 18, 2021

Use newer PCT jenkinsci/bom#657

Closed

jglick reviewed Nov 18, 2021

View reviewed changes

Update JEP-229 conditions, evaluate SNAPSHOT and no local override

9a29863

jglick reviewed Nov 19, 2021

View reviewed changes

plugins-compat-tester/src/main/java/org/jenkins/tools/test/hook/MultiParentCompileHook.java Outdated Show resolved Hide resolved

jglick requested a review from Vlatombe November 19, 2021 20:43

Fix retrieval of maven module

3ca1352

imonteroperez marked this pull request as ready for review November 23, 2021 08:59

imonteroperez requested review from jglick, raul-arabaolaza and cathychan November 23, 2021 08:59

jglick approved these changes Nov 23, 2021

View reviewed changes

Apply suggestions from code review

65f57b4

Co-authored-by: Jesse Glick <jglick@cloudbees.com>

cathychan approved these changes Nov 23, 2021

View reviewed changes

imonteroperez merged commit 5a9f15e into jenkinsci:master Nov 24, 2021

jglick added the bug label Nov 29, 2021

jglick mentioned this pull request Jan 4, 2022

Structs plugin has been flatten for versions higher than 1.24 #334

Merged

6 tasks

jglick mentioned this pull request Jul 26, 2022

Teach PCT about Mina SSHD API plugins #375

Merged

olamy mentioned this pull request Oct 29, 2022

use a shallow clone as we do not need history here (do not use anymore maven-scm but use more simple and faster direct git cli) #382

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support to JEP-229 / Fixing compilation on multimodule projects #318

Support to JEP-229 / Fixing compilation on multimodule projects #318

imonteroperez commented Nov 18, 2021

jglick Nov 18, 2021

jglick left a comment

jglick Nov 23, 2021

imonteroperez Nov 23, 2021

jglick Nov 23, 2021

jglick Nov 23, 2021

imonteroperez Nov 23, 2021

jglick Nov 23, 2021

imonteroperez commented Nov 23, 2021

jglick commented Nov 23, 2021

cathychan left a comment

jglick commented Nov 29, 2021 •

edited

Loading

Support to JEP-229 / Fixing compilation on multimodule projects #318

Support to JEP-229 / Fixing compilation on multimodule projects #318

Conversation

imonteroperez commented Nov 18, 2021

jglick Nov 18, 2021

Choose a reason for hiding this comment

jglick left a comment

Choose a reason for hiding this comment

jglick Nov 23, 2021

Choose a reason for hiding this comment

imonteroperez Nov 23, 2021

Choose a reason for hiding this comment

jglick Nov 23, 2021

Choose a reason for hiding this comment

jglick Nov 23, 2021

Choose a reason for hiding this comment

imonteroperez Nov 23, 2021

Choose a reason for hiding this comment

jglick Nov 23, 2021

Choose a reason for hiding this comment

imonteroperez commented Nov 23, 2021

jglick commented Nov 23, 2021

cathychan left a comment

Choose a reason for hiding this comment

jglick commented Nov 29, 2021 • edited Loading

jglick commented Nov 29, 2021 •

edited

Loading