Modernize the tool #39

MarkEWaite · 2022-08-02T23:53:50Z

Modernize the tool

Reduce spotbugs warnings.

Remove unread field
Fix default encoding spotbugs warnings
Do not use same class name as super class
Don't ignore return values from File methods
Exclude EI_EXPOSE_REP2 spotbugs warning
Exclude command injection spotbugs warning

Squash merge please, so that the clutter is not included in the upstream repository history.

Add spotbugs exclusions Correct a deprecated execution goal

FileUtilsExt is not much harder to read than FileUtils

This reverts commit 0cee6e9.

MarkEWaite · 2022-08-03T12:26:48Z

Needs a detailed comparison of the console output on the master branch with the results from this pull request. Some of the messages in the pull request build that are not in the master build include:

Failed to process BlazeMeterJenkinsPlugin
Failed to process DotCi-Fig-template
Failed to process JDK_Parameter_Plugin
Failed to process JiraTestResultReporter
Failed to process Surround-SCM-plugin
Failed to process TestFairy
Failed to process advanced-installer-msi-builder
Failed to process all-changes
Failed to process appaloosa-plugin
Failed to process artifact-diff-plugin
Failed to process assembla-merge-request-builder
Failed to process aws-bucket-credentials
Failed to process aws-lambda
Failed to process aws-yum-parameter
Failed to process bds-plugin
Failed to process bitbucket-approve
Failed to process build-env-propagator
Failed to process build-environment
Failed to process build-publisher
Failed to process build-requester
Failed to process build-timestamp
Failed to process cerberus-testing
Failed to process changelog-history
Failed to process clamav
Failed to process cli-commander
Failed to process cloudtest
Failed to process cloverphp
Failed to process cluster-stats

basil · 2022-09-07T21:29:45Z

This PR introduces a number of new failures at runtime compared to #41 that is the opposite, an 85% reduction in failures; however, this PR tackles some SpotBugs issues which #41 does not. Suggest merging #41 first and then turning this PR into a dedicated SpotBugs cleanup PR.

basil · 2022-09-07T21:39:36Z

Actually the latest run of this PR is even better than #41:

Fixed failures:
1296

New failures:
1
+++ newf        2022-09-07 14:37:53.148885328 -0700
+Failed to process AnchorChain

So maybe we should take this PR instead after all!

basil · 2022-09-07T21:47:53Z

I think I have one theory why this PR only adds 1 new test failure while #41 adds 167 new failures: this PR is continuing to compile the code with Java 8 bytecode and then running it with Java 8, while #41 compiles the code with Java 11 bytecode and runs with Java 11. I suspect the 167 new failures in #41 are plugins that don't compile with Java 11. Since those 167 new failures are all relatively obscure plugins, it might be worth preferring #41 to get to Java 11 faster. If that long tail of 167 plugins is deemed important, I could update #41 to retain Java 8 compilation and Java 8 at runtime and that should get that PR down to the same set of new failures as in this PR.

MarkEWaite · 2022-09-08T15:34:36Z

If that long tail of 167 plugins is deemed important, I could update #41 to retain Java 8 compilation and Java 8 at runtime and that should get that PR down to the same set of new failures as in this PR.

That is the approach I would prefer. I think we need to retain Java 8 compilation and Java 8 runtime for now. Even though the 167 new failures are relatively obscure plugins, I'd rather retain them in the output than lose them.

Could we set a timeout and declare that we'll switch this tool to Java 11 in 6 months and accept that in 6 months some portion of those 167 relatively obscure plugins will be dropped from the extension point documentation?

basil · 2022-09-08T15:51:45Z

Could we set a timeout and declare that we'll switch this tool to Java 11 in 6 months and accept that in 6 months some portion of those 167 relatively obscure plugins will be dropped from the extension point documentation?

That makes sense to me, though in practice I doubt the list will have changed in 6 months and I think the only practical effect this will have will be to delay progress by 6 months. But the principle of announcing a deprecation period and giving consumers time to adapt is sound. I have pushed commit 7c641b6 to #41; let's see if the failure rate now matches this PR (hopefully it should).

MarkEWaite · 2022-09-08T16:47:19Z

Could we set a timeout and declare that we'll switch this tool to Java 11 in 6 months and accept that in 6 months some portion of those 167 relatively obscure plugins will be dropped from the extension point documentation?

That makes sense to me, though in practice I doubt the list will have changed in 6 months and I think the only practical effect this will have will be to delay progress by 6 months.

I think you're right. I looked at those 167 plugins and saw none of them that were valuable enough to retain in extension documentation. I think we do not need to wait the 6 months. Those 167 plugins are very obscure. If a maintainer of one of those plugins chooses to modernize it, the plugin will reappear in the extensions list.

basil · 2022-09-08T17:03:55Z

Let's at least complete the thought experiment in #41 to prove or disprove my theory. If my theory is correct then you can make the final decision about whether to keep commit 7c641b6 or to retain it.

basil · 2022-09-08T18:15:31Z

OK, my latest run of #41 with commit 7c641b6 and commit 5a81e5b confirms my theory:

Fixed failures:
1206

New failures:
3
+++ newf        2022-09-08 11:10:23.972654095 -0700
+Failed to process JiraTestResultReporter
+Failed to process jobConfigHistory
+Failed to process machine-learning

So running backend-extension-indexer with Java 11 will effectively drop support for the 167 least-maintained Jenkins plugins of all time. I took a look at them and they are all 7-12 years old and dormant.

So how do you want to proceed Mark? My preference would be:

Squash merge Refresh component #41, either with commit 7c641b6 and commit 5a81e5b (thus retaining support for the 167 least-maintained Jenkins plugins) or without them (thus dropping support for the 167 least-maintained Jenkins plugins)
Rebase this PR on top of the result and turn it into a dedicated SpotBugs cleanup PR

The reason I want to take #41 over this PR for the non-SpotBugs changes is that I went a little further than you did by dropping support for Sorcerer which is a good future-proofing move.

MarkEWaite · 2022-09-08T18:50:33Z

So how do you want to proceed Mark? My preference would be:

Squash merge Refresh component #41, either with commit 7c641b6 and commit 5a81e5b (thus retaining support for the 167 least-maintained Jenkins plugins) or without them (thus dropping support for the 167 least-maintained Jenkins plugins)

Rebase this PR on top of the result and turn it into a dedicated SpotBugs cleanup PR

The reason I want to take #41 over this PR for the non-SpotBugs changes is that I went a little further than you did by dropping support for Sorcerer which is a good future-proofing move.

I think we should procced with your preference. I'm very glad that you dropped sorcerer and went further on the spotbugs fixes.

I haven't done comparisons of the files written by the tool before this change compared to the files written by the tool after the change. Is that comparison worth the effort or would we rather perform the merge and then review the results on the www.jenkins.io extensions page so that we can see them in context?

Addendum - I just read the comment on #41 that notes it has significantly reduced the number of failures reported by the current version of the tool. That seems like it makes it unreasonable to compare the output. Better to merge #41, run the indexer, build the www.jenkins.io site, and explore the extensions pages there.

basil · 2022-09-08T18:55:28Z

I think we should proceed with your preference.

OK great, so can you make a decision about the 167 least-maintained plugins of all time (which I intentionally left out of my preference). If you want to retain support for them I will retain commit 7c641b6 and commit 5a81e5b in #41. If you want to drop support for them I will drop commit 7c641b6 and commit 5a81e5b from #41.

I'm very glad that you dropped sorcerer and went further on the spotbugs fixes.

Well I didn't go further on the SpotBugs fixes at all; in fact, I didn't tackle them at all. That's why I was proposing to keep this PR open and rebase it on top of #41 as a SpotBugs-focused PR.

Is that comparison worth the effort

I will do some more testing on the proudced output as you have requested.

MarkEWaite · 2022-09-08T19:05:53Z

OK great, so can you make a decision about the 167 least-maintained plugins of all time?

Drop support for them, don't include commit 7c641b6 or commit 5a81e5b from #41

basil · 2022-09-08T19:41:43Z

Great, I have dropped those commits from #41, which is now back to its starting state of fixing 1154 failures and introducing 167 new failures (for the 167 least-maintained plugins of all time).

Next I will focus on performing the additional testing you have requested.

basil · 2022-09-08T20:24:24Z

Next I will focus on performing the additional testing you have requested.

Done in #41 (comment)

basil

Looks great! Nice job!

pom.xml

src/main/java/org/jenkinsci/extension_indexer/ExtensionPointListGenerator.java

basil · 2022-09-09T15:33:18Z

src/main/java/org/jenkinsci/extension_indexer/ExtensionPointListGenerator.java

@@ -186,7 +190,7 @@ public static void main(String[] args) throws Exception {
    public JSONObject getJsonUrl(String url) throws MalformedURLException, IOException {
        try (
                InputStream is = new URL(url).openStream();
-                InputStreamReader isr = new InputStreamReader(is);
+                InputStreamReader isr = new InputStreamReader(is, "UTF-8");


This made sense in Java 8, but now that the code base is on Java 11 we can use the more specific type:

Suggested change

InputStreamReader isr = new InputStreamReader(is, "UTF-8");

InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8);

(Plus import java.nio.charset.StandardCharsets; at the top.)

Resolved in 48046e6

basil · 2022-09-09T15:33:55Z

src/main/java/org/jenkinsci/extension_indexer/ExtensionPointListGenerator.java

+        try (PrintWriter w = new PrintWriter(new File(asciidocOutputDir, "index.adoc"), "UTF-8")) {
+            IOUtils.copy(new InputStreamReader(getClass().getResourceAsStream("index-preamble.txt"), "UTF-8"), w);


Ditto:

Suggested change

try (PrintWriter w = new PrintWriter(new File(asciidocOutputDir, "index.adoc"), "UTF-8")) {

IOUtils.copy(new InputStreamReader(getClass().getResourceAsStream("index-preamble.txt"), "UTF-8"), w);

try (PrintWriter w = new PrintWriter(new File(asciidocOutputDir, "index.adoc"), StandardCharsets.UTF_8)) {

IOUtils.copy(new InputStreamReader(getClass().getResourceAsStream("index-preamble.txt"), StandardCharsets.UTF_8), w);

Resolved in 48046e6

basil · 2022-09-09T15:34:09Z

src/main/java/org/jenkinsci/extension_indexer/ExtensionPointListGenerator.java

+            try (PrintWriter w = new PrintWriter(new File(asciidocOutputDir, m.getUrlName() + ".adoc"), "UTF-8")) {
+                IOUtils.copy(new InputStreamReader(getClass().getResourceAsStream("component-preamble.txt"), "UTF-8"), w);


Ditto:

Suggested change

try (PrintWriter w = new PrintWriter(new File(asciidocOutputDir, m.getUrlName() + ".adoc"), "UTF-8")) {

IOUtils.copy(new InputStreamReader(getClass().getResourceAsStream("component-preamble.txt"), "UTF-8"), w);

try (PrintWriter w = new PrintWriter(new File(asciidocOutputDir, m.getUrlName() + ".adoc"), StandardCharsets.UTF_8)) {

IOUtils.copy(new InputStreamReader(getClass().getResourceAsStream("component-preamble.txt"), StandardCharsets.UTF_8), w);

Resolved in 48046e6

basil · 2022-09-09T15:36:48Z

src/main/java/org/jenkinsci/extension_indexer/SourceAndLibs.java

        final File tempDir = File.createTempFile("jenkins","extPoint");
-        tempDir.delete();
-        tempDir.mkdirs();
+        Files.delete(tempDir.toPath());
+        Files.createDirectories(tempDir.toPath());


If what you're looking for is a temporary directory, Files#createTempDirectory is a more straightforward path to get there and avoids a race condition that I think Jonathan Leitschuh has been working on fixing throughout the entire planet with automated PRs.

Thanks! Included in 4f8dd3b

We fixed the temporary directory creation issue, but I'm not sure whether this repository also contains a zip slip issue. @JLLeitschuh are you interested in running your tool against it and creating a PR if there is a zip slip issue?

I think Jonathan Leitschuh has been working on fixing throughout the entire planet with automated PRs.

I have been, this is a hilarious way of phrasing this one 😆 ❤️

We fixed the temporary directory creation issue, but I'm not sure whether this repository also contains a zip slip issue. @JLLeitschuh are you interested in running your tool against it and creating a PR if there is a zip slip issue?

If you could do me a kind favor and get any projects (ie probably this entire org) you do want me to generate PRs against indexed into this project, I'd be more than happy to PR generation! 🎉

https://github.com/moderneinc/jenkins-ingest

Sorry, I do not understand what you are asking me to do. I just thought you might be interested in contributing another helpful automated PR, as you have in the past. If you are not, no problem. We appreciate your past contributions!

Co-authored-by: Basil Crow <me@basilcrow.com>

…ension-indexer into modernize

MarkEWaite added 19 commits August 2, 2022 16:29

Use parent pom 1.84 instead of 1.37

433b35b

Add spotbugs exclusions Correct a deprecated execution goal

Use jsoup 1.15.2 instead of 1.14.2

6f7a509

Use args4j 2.33 instead of 2.0.16

1ac4c32

Use commons-io 2.11.0 instead of 2.7

599ac4e

Use commons-lang3 3.12.0 instead of 3.4

c64106a

Use maven-resolver-api 1.8.2 instead of 1.6.2

90f4521

Remove confluence api dependency

e3ad325

Use https protocol instead of git protocol

6fef325

Remove obsolete glassfish repository

89a4f34

Minor phrasing change in description

ece9de7

Remove unread field

cbb45ce

Fix default encoding spotbugs warnings

88ad5bb

Exclude EI_EXPOSE_REP2 spotbugs warning

f588034

Do not use same class name as super class

dd96163

FileUtilsExt is not much harder to read than FileUtils

Don't ignore return values from File methods

20347d8

Exclude command injection spotbugs warning

82337e0

Use Java 11 in Jenkinsfile

0cee6e9

Revert "Use Java 11 in Jenkinsfile"

87d135b

This reverts commit 0cee6e9.

Use JDK 11

92a322d

MarkEWaite mentioned this pull request Aug 8, 2022

Updating to Java 11 jenkins-infra/pipeline-steps-doc-generator#197

Merged

MarkEWaite marked this pull request as draft August 8, 2022 12:54

Merge branch 'master' into modernize

9d948c7

MarkEWaite mentioned this pull request Sep 7, 2022

Extension indexer fails to index Gradle-built plugins #42

Open

Merge branch 'master' into modernize

5f7b071

Merge branch 'master' into modernize

abb8030

basil approved these changes Sep 9, 2022

View reviewed changes

MarkEWaite and others added 6 commits September 9, 2022 17:59

Remove unused dependency

6296825

Co-authored-by: Basil Crow <me@basilcrow.com>

Remove version already provided by parent pom

25d4504

Co-authored-by: Basil Crow <me@basilcrow.com>

Fix spelling error

2b8bd9c

Co-authored-by: Basil Crow <me@basilcrow.com>

Merge branch 'modernize' of https://github.com/MarkEWaite/backend-ext…

b54120f

…ension-indexer into modernize

Use StandardCharsets.UTF_8 instead of "UTF-8"

48046e6

Use Files.createTempDirectory

4f8dd3b

MarkEWaite marked this pull request as ready for review September 10, 2022 00:13

basil approved these changes Sep 10, 2022

View reviewed changes

MarkEWaite merged commit 1953d69 into jenkins-infra:master Sep 10, 2022

MarkEWaite deleted the modernize branch September 10, 2022 02:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modernize the tool #39

Modernize the tool #39

MarkEWaite commented Aug 2, 2022 •

edited

Loading

MarkEWaite commented Aug 3, 2022

basil commented Sep 7, 2022

basil commented Sep 7, 2022

basil commented Sep 7, 2022

MarkEWaite commented Sep 8, 2022

basil commented Sep 8, 2022

MarkEWaite commented Sep 8, 2022

basil commented Sep 8, 2022

basil commented Sep 8, 2022

MarkEWaite commented Sep 8, 2022 •

edited

Loading

basil commented Sep 8, 2022

MarkEWaite commented Sep 8, 2022 •

edited

Loading

basil commented Sep 8, 2022

basil commented Sep 8, 2022

basil left a comment

basil Sep 9, 2022 •

edited

Loading

MarkEWaite Sep 10, 2022

basil Sep 9, 2022

MarkEWaite Sep 10, 2022

basil Sep 9, 2022

MarkEWaite Sep 10, 2022

basil Sep 9, 2022

MarkEWaite Sep 10, 2022

basil Sep 12, 2022

JLLeitschuh Sep 12, 2022 •

edited

Loading

basil Sep 12, 2022 •

edited

Loading

	InputStreamReader isr = new InputStreamReader(is, "UTF-8");
	InputStreamReader isr = new InputStreamReader(is, StandardCharsets.UTF_8);

		try (PrintWriter w = new PrintWriter(new File(asciidocOutputDir, "index.adoc"), "UTF-8")) {
		IOUtils.copy(new InputStreamReader(getClass().getResourceAsStream("index-preamble.txt"), "UTF-8"), w);

Modernize the tool #39

Modernize the tool #39

Conversation

MarkEWaite commented Aug 2, 2022 • edited Loading