Skip to content
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

Dependency Conflict Resolution, Dependency Bump #1360

Merged
merged 11 commits into from
Oct 20, 2024

Conversation

synthfi
Copy link
Collaborator

@synthfi synthfi commented Aug 22, 2024

Changes

  • Bump javafx dependencies from 17.0.2 to 20.0.2
  • Bump commons-text from 1.10 to 1.12
  • Bump commons-lang3 from 3.12 to 3.14
  • Bump unit-jupiter dependencies from 5.6 to 5.9
  • Removes opera-driver dependency
  • Bump selenium-java from 4.3 to 4.14
  • Bump jsoup to 1.18.1
  • Removes jsr305 dependency
  • Removes stored jar files for jsoup, jsr305, and commons-lang3
  • Updates build_image.py so that jlink is able to find jsoup & commons-lang3's modules
  • Updates module-info.java & comments

Notes

  • removes dependency conflicts
  • it looks like opera-driver was replaced with selenium-opera-driver
  • removes javafx-media 17.0.2 cve

Tested so far:

  • kubuntu 24.04
  • mac os sonoma 14.6.1
  • windows 11 23H2 *

Notes:

  • I'm able to make the exe installer on windows and install it just fine. However, every time I run the full suite of tests on windows it hangs on testPTextInputDialog. I also see this behavior on the current master branch, so I don't believe it's a bug that arose as a result of the changes I've made here. Running the OpenScreensTest suite manually doesn't show this behavior

move to all being 5.9.0 to resolve conflicts
…ium.selenium-opera-driver 4.3.0

the operadriver dependency has been superseded by selenium-opera-driver
- selenium-java already declares this
- first instance that doesn't have internal dependency conflicts
- does not have vulnerabilities as reported by the Red Hat Depedency
  Analytics plugin
- also works on asahi
@synthfi
Copy link
Collaborator Author

synthfi commented Aug 29, 2024

There's an issue that I can't get quite past on windows in regards to the unit testing. If I run mvn test, the tests hang on the testPTextInputDialog test, where some polyglot windows open and then never close. It just hangs, and with jstack I can see some messages that look like

java.lang.Thread.State WAITING on (object monitor)

However, if I run the OpenScreensTest test suite specifically, I don't see this issue. No idea what's going on here.

I was able to generate an installer with windows and then install it on windows 11 23H2, so there's that??

edit: I see this behavior with master as well. So I don't think this is an issue that is raised by this PR

@synthfi synthfi changed the title Draft: Dependency conflict resolution Dependency Conflict Resolution, Dependency Bump Aug 29, 2024
@synthfi synthfi marked this pull request as ready for review August 29, 2024 07:23
@synthfi
Copy link
Collaborator Author

synthfi commented Sep 15, 2024

notes after investigating other dependencies

  • net.java.dev.swing-layout was last updated in 2007
  • newer versions of org.jsoup drop the depedency on jsr305, but I can't get it to build correctly after I update it on ubuntu. something to do with module injection I think
  • commons-lang was last updated in 2011. I suspect that this is for the java 8 bridge

@DraqueT
Copy link
Owner

DraqueT commented Sep 23, 2024

Heyo, thanks for the work here! I do have a few thoughts.

Were you able to get the injector to work properly for commons-csv, jsoup, and commons-lang3? The module injected jars will need to be placed into the module_injected_jars folder, as those are manually added at build time. Without the injected library versions put into that folder, PolyGlot will run at debug time with the newer versions, but be built with the older ones. Conversely, have those three libraries finally been made to be compatible with the module system? If so, that would eliminate the need for the module injection stuff entirely (which would be great, it is a not-great hack, and the reason that those libraries remained un-updated for so long).

Regarding the TestPTextInputDialog test... that one has been flakey for a while. I think that there is some odd race condition that comes up when it's tested in automation, but doesn't tend to appear in regular use. If it's causing you too much trouble, feel free to disable the test. I think it's causing more bother than it is doing good.

Regarding commons lang and the Java bridge... you're exactly right there. It's unfortunate, but the PDF generation library that I use declined to give me the same educational license for their newer versions which support the module system. The old, clunky version is fundamentally tied to Java 8, which it the only real remaining reason for the bridge's existence (and commons lang). I know that other options exist for PDF generation these days, but it would mean refactoring that portion of code entirely.

@synthfi
Copy link
Collaborator Author

synthfi commented Sep 23, 2024

haven't run the module injector yet, will do that next

jsoup became a proper module in 1.17.1 if I'm reading this right: jhy/jsoup#2025, so that one is good at least.

When I look inside the commons-lang3 jar, I see a module-info.class file in there, but the github repo doesn't have a module-info.java file so I am less sure about that one

Regarding the TestPTextInputDialog test...

sounds good. I'll just ignore that for now.

@DraqueT
Copy link
Owner

DraqueT commented Sep 23, 2024

Ok! If JSoup has been made into a module, then I think that the jar within module_injected_jars can be deleted entirely. You might need to update src/main/java/module-info.java if the module name that the official release chosen was different than the one I arbitrarily assigned with the injector, but that should be it!

- commons-lang3 3.14.0 is a multi-release jar
- update comment
- update build script for jlink to grab location of commons-lang3
- requires testing
@synthfi
Copy link
Collaborator Author

synthfi commented Sep 26, 2024

found the section in the release notes for commons-lang3 about how it is officially a multi-release jar now. Made several updates for this, still need to test on other systems besides ubuntu

@synthfi
Copy link
Collaborator Author

synthfi commented Sep 26, 2024

tested on windows (minus OpenScreensTest) & macos, both tests & installation of packages

@synthfi synthfi merged commit 5cb724e into DraqueT:master Oct 20, 2024
@synthfi synthfi deleted the dependency-conflict-resolution branch October 20, 2024 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants