Upgrade to htmlunit 3.x #589
Conversation
pom.xml
Outdated
| @@ -121,7 +121,7 @@ THE SOFTWARE. | |||
| <dependency> | |||
| <groupId>${project.groupId}</groupId> | |||
| <artifactId>jenkins-test-harness-htmlunit</artifactId> | |||
| <version>141.v740721146265</version> | |||
| <version>142.ve874f871a_6e3</version> | |||
There was a problem hiding this comment.
TODO update to release version
| @@ -201,17 +201,6 @@ THE SOFTWARE. | |||
| <artifactId>junit-vintage-engine</artifactId> | |||
| <scope>test</scope> | |||
| </dependency> | |||
| <dependency> | |||
| <groupId>org.jvnet.hudson</groupId> | |||
| <artifactId>embedded-rhino-debugger</artifactId> | |||
There was a problem hiding this comment.
some abandoned project of kohsuke's which could popup a swing dialog to do something with JavaScript errors I think we can drop this.
https://github.com/jenkinsci/lib-embedded-rhino-debugger/
Otherwise it needs to be refreshed, updated to new APIs and released
timja
force-pushed
the
3.x
branch
2 times, most recently
from
6c184d7
to
c943a26
Compare
|
(requires jenkinsci/jenkins-test-harness-htmlunit#110 first) |
There was a problem hiding this comment.
This is an incompatible change that will require at least 125 plugins to be adapted, including about 70 plugins with over 10,000 installations. While it may be fair to place the burden of adapting to a breaking change on plugin maintainers for breaking changes that are smaller in scope, I feel that this is too significant of a breaking change for the burden to be placed on plugin maintainers. By and large, we need to bring plugins along with this breaking change rather than leaving them to deal with the fallout by themselves.
|
Do you have any suggestions? So far I haven’t hit any functional issues it’s just a find and replace that can go in release notes although may complicate bom testing till those are released |
|
I can do a bulk migration once it's released it's a very simple find and replace |
I do not think it would because BOM/PCT does not update the plugin POM/JTH to the latest version but rather uses the plugin's version, and we should have any JTH method signatures in Jenkins core. Might be worth checking with a BOM PR that adds
OK, my goal is simply to ensure that this happens with us (it does not have to be just you!) taking on this burden rather than leaving the burden on plugin maintainers. The gap between the work done in this PR and the work promised above still seems a bit too large for me to feel comfortable approving this. I would like to understand in more detail how we plan to identify affected plugins, how we plan on migrating them (manually? an automated script?), who plans to be involved in the effort (I can help, but cannot take on the whole burden), how we plan to track progress, etc. In other words, seeing a more thorough migration plan (with some examples of it being followed successfully) would go a long way toward convincing me that this migration will be successful once it has begun and it is too late to turn back. |
|
just planning on running multi-gitter over the jenkinsci org replacing the imports and methods and bumping the plugin pom to the version with the latest test harness Shouldn't need any manual work once the script has been written. |
|
OK, that sounds like a promising start. I would like to see the script written and applied to at least one repository successfully to have full confidence in the migration plan. |
|
Migration script mostly finished and works on stapler: #!/usr/bin/env bash
set -e
if [ "$(uname)" == "Darwin" ]; then
sed=gsed
fi
find . -name 'pom.xml' -type f -exec $sed -i'' -e '
/<artifactId>jenkins-test-harness-htmlunit<\/artifactId>/{
N
/<version>[^<]*<\/version>/{
s/<version>[^<]*<\/version>/<version>143.v8979204b_fc0c<\/version>/
}
}
/<artifactId>jenkins-test-harness<\/artifactId>/{
N
/<version>[^<]*<\/version>/{
s/<version>[^<]*<\/version>/<version>1995.v96366f69785f<\/version>/
}
}
' {} +
# Iterate over all files in the current directory recursively
find . -type f -name '*.java' -exec $sed -i'' -e '
/net\.sourceforge\.htmlunit/{
s/net\.sourceforge\.htmlunit/org.htmlunit/g
}
/com\.gargoylesoftware\.htmlunit/{
s/com\.gargoylesoftware\.htmlunit/org.htmlunit/g
}
/getValueAttribute/{
s/getValueAttribute/getValue/g
}
/setAttribute("value", /{
s/setAttribute("value", /setValue(/g
}
/setAttribute("value",/{
s/setAttribute("value",/setValue(/g
}
/setValueAttribute/{
s/setValueAttribute/setValue/g
}
/<artifactId>jenkins-test-harness<\/artifactId>.*<version>[^<]*<\/version>/{
s/<version>[^<]*<\/version>/<version>1995.v96366f69785f<\/version>/
}
' {} +
if git diff --exit-code > /dev/null
then
echo "No replacements required"
exit
fi
if grep -q '<artifactId>plugin</artifactId>' pom.xml
then
mvn versions:update-parent -DparentVersion=4.64
fi
spotless_disabled=$(mvn help:evaluate -Dexpression=spotless.check.skip -q -DforceStdout)
if ! $spotless_disabled; then
mvn spotless:apply
fi
git status
echo "Replacement completed."
|
|
For future migrations it might be nice to take a look at https://docs.openrewrite.org/ |
Worth a look although chatgpt wrote this for me trivially with only minor changes needed 😄. |
|
I think this script is fairly incomplete. For one thing, it didn't work in spotless_disabled=$(mvn help:evaluate -Dexpression=spotless.check.skip -q -DforceStdout)
if ! $spotless_disabled; then
mvn spotless:apply
fiThe other question I have is, what is going to be done if these bot-filed PRs fail to get a clean build? |
|
Script updated now, should be fairly good now can always re-run it if more is learnt after applying to a bunch of repositories.
I think it would be good to try fix / apply enhance the script for more cases if the plugin is maintained. I'm planning on running it from a bot account so I don't get notification spam from it. What do you think about that? |
The criteria for maintained/unmaintained are fairly subjective. If a plugin is in the top 250 and not obviously deprecated/useless (e.g., For example, recent changes to the archetype's While covering every plugin in the whole ecosystem is surely impractical, covering too few is an all too common scenario in the project and creates problems for other people when incomplete migrations delay unrelated efforts in the future. Note that in many cases I already have build migration PRs open (e.g. jenkinsci/active-directory-plugin#168) that this migration will need to build on top of. Why don't we start by identifying the plugins that need to be updated and create a spreadsheet sorted by # of installs to track the migration effort. This sheet is one example. I want to get a clear understanding of what is in scope and what isn't. If we disagree about what should be in scope I am willing to help out to some degree. My goal is not to create obstacles but rather to be part of the solution to ensure the overall health of the plugin ecosystem. |
|
Identifies 540 results. The UI only shows about 100 though if I click through. It also reports multiple matches per repository. I would really like to get started on this and clean up low hanging fruit and then look for stragglers. |
|
I have a local clone of the top 250 which I supplement with results from the GitHub UI. By all means, get started, but I want to be clear that looking for stragglers is only one part of this. The other part is bringing those stragglers back into the fold by filing PRs and making sure those PRs build for the majority of the actively used parts of the ecosystem, regardless of whether or not there is somebody currently actively maintaining that plugin. People who adopt a plugin aren't likely to quickly come up to speed on arcane build trivia, so we owe it to them to make the onboarding process as straightforward as possible by providing ready-to-merge PRs. |
FWIW I ran a similar search on all @jenkinsci repos already cloned locally & up to date, found 310 of them containing ListFTR, 5 archived repositories also match the search: |
|
Thanks @lemeurherve! I started a spreadsheet for this migration here: https://docs.google.com/spreadsheets/d/1ih_gVd9uhxLw4BZ6IeJGrYsvu-DvIF0xovu4wZbaXy8/edit As you can see, there is a huge amount of scope to this migration. Limiting the scope to the top 100, there are about 40 plugins that need migration. Limiting the scope to the top 250, there are about 80 plugins that need migration. Limiting the scope to anything > 10,000 installs there are about 90 plugins that need migration. Limiting the scope to anything > 1,000 installs there are about 150 plugins that need migration. From my perspective filing PRs against the top 250 and making sure they have a clean build is critical, regardless of whether or not there is an active maintainer. Beyond that, I think it is important to file PRs for anything with > 10,000 installations (and desirable even for anything with > 1,000 installations), although I could be OK if the plugin is not maintained and therefore the PRs don't have a clean build. If something has less than 1,000 installations I don't think it is worth our time to focus on it. This isn't a request for one person to do all of this work singlehandedly. We can surely discuss ways of sharing this burden. But I think it is important that we do take this burden on, as a group, rather than simply starting a migration and never finishing it. |
|
Script created here: https://github.com/timja/htmlunit-2.x-to-3.x Tested here: jenkinsci/junit-plugin#535 Out of time now so won't release and run. |
|
The merge of this PR forestalled my intention to have the migration work completed before this PR was merged and released. |
Includes updates for * JGit 6.5.0 to 6.6.0 https://projects.eclipse.org/projects/technology.jgit/releases/6.6.0 jenkinsci/git-client-plugin#1016 * Require Jenkins 2.387.3 or newer jenkinsci/git-client-plugin#1014 jenkinsci/git-plugin#1459 * HTMLUnit 2.x to 3.x jenkinsci/jenkins-test-harness#589 jenkinsci/git-client-plugin#1015 jenkinsci/git-plugin#1460
|
+1 to @StefanSpieker's suggestion of using OpenRewrite. Here's what a recipe to change package names looks like: type: specs.openrewrite.org/v1beta/recipe
name: net.sghill.rewrite.HtmlUnit3
displayName: Upgrade to HtmlUnit 3.x
description: Update packages to org.htmlunit
recipeList:
- org.openrewrite.java.ChangePackage:
oldPackageName: net.sourceforge.htmlunit
newPackageName: org.htmlunit
recursive: "False"
- org.openrewrite.java.ChangePackage:
oldPackageName: com.gargoylesoftware.htmlunit
newPackageName: org.htmlunit
recursive: "False"For fun I ran this on https://public.moderne.io and it found a lot of changes that could be issued as PRs from the service. To completely automate this it'd be worth adding a few more recipes to ensure HtmlUnit 3 is being used, but as the yaml shows it's easy to compose recipes.
|
|
I worked on the spreadsheet today and made some progress, but there are still a lot of long-tail plugins without a passing build with the new version of HtmlUnit. I would like to see a bit more effort spent on the long tail before we declare this project a success. |
Part of #568
Testing done
Ran tests in the project
Submitter checklist