-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
build(deps): bump org.jsoup:jsoup from 1.21.2 to 1.22.1 #19985
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build(deps): bump org.jsoup:jsoup from 1.21.2 to 1.22.1 #19985
Conversation
|
c575df1 to
1f3ac44
Compare
|
So glad we have release-mode testing. This does not reproduce with debug builds, only when running release builds.
I added the transitive dependency the new jsoup feature depends on, and I also added a link to the upstream release notes for jsoup works for me locally now |
|
Do you feel the dependency is required for our use case? It should be optional, and I'd hope we can ignore it
|
|
running APK size check to see if it has an impact worth caring about... |
|
38659667: Old APK size |
|
🤔 70kbytes for nothing, yeah, should be optional based on release notes and should be something we can get by without here. Will dig a bit more deeply. Tagging as "upstream issue" based on current hypothesis that upstream isn't correctly making their transitive optional in the way they load it or not based on initial evidence that the shrinker is finding hard references to the classes while it does it's work |
|
Opened upstream issue An analysis of the As such, it should be safe to tell the shrinker to simply ignore the re2j classes causing the error, as they will never be used in practice in runtime if re2j is not on the classpath |
Bumps [org.jsoup:jsoup](https://github.com/jhy/jsoup) from 1.21.2 to 1.22.1. - [Release notes](https://github.com/jhy/jsoup/releases) - [Changelog](https://github.com/jhy/jsoup/blob/master/CHANGES.md) - [Commits](jhy/jsoup@jsoup-1.21.2...jsoup-1.22.1) --- updated-dependencies: - dependency-name: org.jsoup:jsoup dependency-version: 1.22.1 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> build(deps): ignore optional re2j class dependency that the r8 shrinker finds in jsoup - all access of the concrete references to the optional re2j library are gated via reflective inspection of the classpath to see if it is available, so ignoring warnings should be safe - testing in note editor adding an image (which should pass through the code that uses jsoup) survived with no crash
1f3ac44 to
f2cb359
Compare
|
testing with It appears that there is no crash if you simply do not have re2j on classpath and instruct r8 to ignore the warning as I did in the commit I just posted. |
|
|
||
| # Ignore intended-to-be-optional re2j classes - only needed if using re2j for jsoup regex | ||
| # jsoup safely falls back to JDK regex if re2j not on classpath, but has concrete re2j refs | ||
| # See https://github.com/jhy/jsoup/issues/2459 - may be resolved in future, then this may be removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added to my watchlist. Cheers!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I historically skipped adding the repo link as a comment if dependabot was working and linking changelogs.
Undetermined if we want to start linking everything, maybe an hour of busywork
|
I figure it's easy to add a link while adding new deps. I'm not fussed about adding all the ones historically |
Bumps org.jsoup:jsoup from 1.21.2 to 1.22.1.
Release notes
Sourced from org.jsoup:jsoup's releases.
Changelog
Sourced from org.jsoup:jsoup's changelog.
Commits
8dd66fe[maven-release-plugin] prepare release jsoup-1.22.1d924385Changelog prep for v1.22.10f3100cBump actions/upload-artifact from 5 to 6 (#2457)cf6ac20Bump org.apache.maven.plugins:maven-release-plugin from 3.3.0 to 3.3.1 (#2455)6bef938Fix parsing of SVG foreignObject in paragraphs9b1c0fcBump org.apache.maven.plugins:maven-release-plugin from 3.2.0 to 3.3.0 (#2450)1415e64Bump actions/checkout from 5 to 6 (#2451)0e99fd9Isolate TagSet copies to prevent shared mutation (#2453)90019cbBump com.github.siom79.japicmp:japicmp-maven-plugin from 0.24.2 to 0.25.0 (#2...9395269Don't preemptively closeDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)