Update Dependencies #146

gibson9583 · 2025-07-23T15:12:05Z

This seems like the most efficient way to handle updating these jars. Pulling from a known quantity rather than reviewing ourselves

jonbartels · 2025-07-28T15:47:19Z

Adding this to the "Next Release" miletone. Steering Committee discussed and this is valuable.

It catches up on library updates
It makes OIE consistent with Innovar and Nextgen

There are some risks, like testing this whole thing, but those risks are manageable.

jonbartels · 2025-07-28T15:52:26Z

Steering Committee also discussed this issue being a higher priority than Gradle (#146) since this gets us security updates sooner.

gibson9583 · 2025-09-09T00:56:30Z

@kayyagari @kpalang @mgaffigan - I may be a bit over my head here.... I thought this was going to be a simple cherry pick of a handful of commits, but our build processes have deviated a bit and I need to pull in some people a bit more familiar with the process than me.

mgaffigan · 2025-09-09T01:42:04Z

@gibson9583, see/pull 6a2459f

Note also the changes to server/src/com/mirth/connect/model/util/DefaultMetaData.java. That seems like an opinion needing to be pulled out of this change set.

mgaffigan · 2025-09-09T02:18:09Z

Updating the GH pipeline to Java 17 has it building successfully in CI. Still some test failures.

gibson9583 · 2025-09-09T13:49:25Z

@mgaffigan - thanks. Also pushed a commit to revert the change to DefaultMetaData

Signed-off-by: Chris Gibson <cgibson@outlook.com>

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net> Signed-off-by: Chris Gibson <cgibson@outlook.com>

Signed-off-by: gibson9583 <cgibson@outlook.com> Signed-off-by: Chris Gibson <cgibson@outlook.com>

mgaffigan

Compile failures:

[javadoc] error: invalid flag: -add-modules

New test-compile warnings:

[javac] warning: [options] --add-opens has no effect at compile time
[junit] OpenJDK 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended
[junit] WARNING: package sun.misc not in java.base

Test failures:

[junit] TEST com.mirth.connect.donkey.server.channel.ChannelTest FAILED
[junit] TEST com.mirth.connect.donkey.server.data.jdbc.JdbcDaoTest FAILED
[junit] Test com.mirth.connect.client.core.ConnectServiceUtilTest FAILED
[junit] Test com.mirth.connect.model.PublicServerSettingsTest FAILED
~~[junit] Test com.mirth.connect.util.JavaScriptSharedUtilTest FAILED~~ (#184 opened - unrelated to this PR)

General concerns:

gitignore needs updated or build needs updated to avoid un-ignored untracked files after build (307 generated)
~~If we're going to spend time testing a release, we should verify the versions of deps we're going to are best~~ (Getting this merged doesn't stop us from future updates).
A lot of these changes are formatting or whitespace. Not strictly a problem, but the PR can be substantially minimized.

server/build.xml

server/test/com/mirth/connect/model/PublicServerSettingsTest.java

Signed-off-by: gibson9583 <cgibson@outlook.com>

kpalang · 2025-09-09T17:21:41Z

Seems like a stupid suggestion and I haven't been following the previous conversation around this topic. But. What if we see what versions BL bumped and do the change manually on our side? We could also do appropriate unit-tests in the process.

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan · 2025-09-29T03:45:01Z

[junit] Test com.mirth.connect.util.JavaScriptSharedUtilTest FAILED

Not sure re: this one. I can't see the output from CI test failure. Running
on windows gives the attached, which I don't understand the assertion failure.

TEST-com.mirth.connect.util.JavaScriptSharedUtilTest.xml
87_JavaScriptSharedUtilTest.html
87_JavaScriptSharedUtilTest-fails.html

mgaffigan · 2025-09-29T03:57:12Z

@kpalang

Seems like a stupid suggestion and I haven't been following the previous conversation around this topic. But. What if we see what versions BL bumped and do the change manually on our side? We could also do appropriate unit-tests in the process.

Since this is also a move to Java 17, the version upgrades are tied to some of the other changes. I think it's in a fairly good state - unresolved issues notwithstanding.

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

tonygermano · 2025-09-29T13:30:04Z

Looks like this was introduced in #90. I didn't notice the test failure since all of the checks passed. The files Mitch shared on this ticket indicate it was related to pretty print functionality.

mgaffigan

Let's go! Seems to be a step in the right direction. Moves the build process to Java 17. Seems to open and run as usual in Ballista.

tonygermano

17 commits and a lot of unrelated changes makes this somewhat messy and difficult to review (I am aware that much of it was probably externally generated.) It's difficult to tell which changes are necessary for the dependency updates and which are not. There are a lot of which I marked in my review as potentially being unnecessary changes. There are many more that are additional comments and whitespace changes which I did not mark, but probably don't belong in this PR (not that I have anything against comments, but they just bloat this already large PR.)

I think while we are changing the minimum required java version, we should do it correctly and close out #18. I think this only entails modifying the ant javac tasks to include a release="17" attribute. It may be wise to store the release version in a build property as it will likely get updated from 17 to 21 in the not distant future now that version 25 is out.

See https://ant.apache.org/manual/Tasks/javac.html and https://stackoverflow.com/a/43103038

client/ant-build.xml

command/build.xml

donkey/build.xml

tonygermano · 2025-09-29T21:19:10Z

server/build.xml

-		<copy todir="${connectors.http}/lib" failonerror="false">
-			<fileset dir="${lib.extensions}/http" />
-		</copy>
 		<jar destfile="${connectors.http}/http-shared.jar" basedir="${classes}">


There are a lot of similar removals here. Technically the directories appear to not exist (I haven't checked all of them,) so there is likely no harm in removing them, but I'm sure the intent is for all of the plugin sections to be the same, whether they have additional libraries or not.

Are these changes we want to keep?

I agree they are unrelated to java 17 - but they add a bunch of warnings to the build output, making it hard to identify actual issues caused by Java 17.

I think these are beneficial changes, since there is not seemingly an ant option to avoid the warning otherwise.

@tonygermano, do you view this as a blocker?

server/build.xml

tonygermano · 2025-09-29T21:47:53Z

server/test/com/mirth/connect/connectors/dimse/DICOMDispatcherTest.java

        String responseData = response.getMessage();

-        String expectedResponseString = "<dicom>\n" + "<tag00000900 len=\"2\" tag=\"00000900\" vr=\"IS\">0</tag00000900>\n" + "</dicom>\n" + "";
+        String expectedResponseString = "<dicom><tag00000900 len=\"2\" tag=\"00000900\" vr=\"IS\">0</tag00000900></dicom>";


Do we know which change caused the newline character removal from the expected response?

@tonygermano, do you view this as a blocker?

server/test/com/mirth/connect/model/PublicServerSettingsTest.java

tonygermano · 2025-09-29T22:04:16Z

server/test/com/mirth/connect/server/controllers/TestUtils.java

+                    statement.execute("CREATE TABLE D_MESSAGE_SEQUENCES (LOCAL_CHANNEL_ID BIGINT NOT NULL PRIMARY KEY)");
+                }
+
                result = statement.executeQuery("SELECT MAX(id) FROM d_m" + localChannelId);


connection is not database dependent, but these new queries are.

Agree. Reverted. I'm also not clear why this was required in the first place.

server/test/com/mirth/connect/server/servlets/WebStartServletTest.java

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan · 2025-09-30T02:10:12Z

17 commits and a lot of unrelated changes makes this somewhat messy and difficult to review (I am aware that much of it was probably externally generated.) It's difficult to tell which changes are necessary for the dependency updates and which are not. There are a lot of which I marked in my review as potentially being unnecessary changes.

Agree. The BL comments are not clear on motivation. I think everything in the PR is based on building with Java 17. The PR could indeed be smaller, but some of these changes were submitted previously as fractional PR's - and were not merged.

There are many more that are additional comments and whitespace changes which I did not mark, but probably don't belong in this PR (not that I have anything against comments, but they just bloat this already large PR.)

The comments were added to avoid increasing the number of warnings in the build. Absent those changes, moving to 17 increases the number.

The whitespace changes I commented on too, but the original whitespace is mental. If you look at the file wrong, it will be reformatted and the whitespace will change. It was not worth my time to revert, since it is still a beneficial change if a bit unrelated. I'd be more willing to pull those into separate PR's if small PR's were to merge quickly.

I think while we are changing the minimum required java version, we should do it correctly and close out #18. I think this only entails modifying the ant javac tasks to include a release="17" attribute. It may be wise to store the release version in a build property as it will likely get updated from 17 to 21 in the not distant future now that version 25 is out.

See https://ant.apache.org/manual/Tasks/javac.html and https://stackoverflow.com/a/43103038

Specifying release gives the following errors:

    [javac] error: exporting a package from system module java.sql.rowset is not allowed with --release
    [javac] error: exporting a package from system module java.base is not allowed with --release
    [javac] error: exporting a package from system module java.base is not allowed with --release

jonbartels added this to the Next Release milestone Jul 28, 2025

jonbartels mentioned this pull request Jul 28, 2025

[IDEA] Introduce Dependency Management (e.g. Gradle or Maven) #52

Open

Innovarzweng and others added 8 commits September 9, 2025 10:23

update the dependency jars

48f6cbe

Signed-off-by: Chris Gibson <cgibson@outlook.com>

update unit tests/jacoco to java 17

2a5b2df

Signed-off-by: Chris Gibson <cgibson@outlook.com>

fix the path

df9070c

Signed-off-by: Chris Gibson <cgibson@outlook.com>

Fix paths

5bdee7d

Signed-off-by: Chris Gibson <cgibson@outlook.com>

[bugfix]update unit tests/jacoco to java 17

5ea883d

Signed-off-by: Chris Gibson <cgibson@outlook.com>

Fix build error

a18463a

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net> Signed-off-by: Chris Gibson <cgibson@outlook.com>

Bump CI build to Java 17

2efa916

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net> Signed-off-by: Chris Gibson <cgibson@outlook.com>

Revert Update DefaultMetaData.java

8db4aca

Signed-off-by: gibson9583 <cgibson@outlook.com> Signed-off-by: Chris Gibson <cgibson@outlook.com>

gibson9583 force-pushed the feature/bl-dependency-cherrypick branch from 165ab87 to 8db4aca Compare September 9, 2025 14:25

gibson9583 requested review from a team, jonbartels, kayyagari, kpalang, mgaffigan, pacmano1, ssrowe and tonygermano and removed request for a team September 9, 2025 15:22

mgaffigan requested changes Sep 9, 2025

View reviewed changes

server/build.xml Show resolved Hide resolved

server/build.xml Outdated Show resolved Hide resolved

server/build.xml Outdated Show resolved Hide resolved

server/test/com/mirth/connect/model/PublicServerSettingsTest.java Show resolved Hide resolved

Revert message source/type to mirth

cf06b23

Signed-off-by: gibson9583 <cgibson@outlook.com>

gibson9583 changed the title ~~Update Dependencies from Bridgelink via Cherry Pick~~ Update Dependencies Sep 24, 2025

mgaffigan added 4 commits September 28, 2025 17:25

Reverted extra changes, added gitignore for junit-html

fd2fd9d

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

Ignore directories created by test-run

c409861

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

Fix donkey build errors re: sun.misc.Unsafe not available

1f345f4

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

Fix warning re: sun.misc not in java.base

8de2837

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan added 3 commits September 28, 2025 23:27

Fix javadoc warning

6e94caa

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

Fix warnings for copy from non-exstent directory

9b98aae

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

Suppress warnings about JSM removal

ebf8e87

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan force-pushed the feature/bl-dependency-cherrypick branch from 4d29958 to 11aa84b Compare September 29, 2025 04:27

Avoid warning about 'Sharing is only supported for boot loader classes'

1647b0f

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan force-pushed the feature/bl-dependency-cherrypick branch from 11aa84b to 1647b0f Compare September 29, 2025 04:33

gibson9583 requested a review from mgaffigan September 29, 2025 12:14

mgaffigan previously approved these changes Sep 29, 2025

View reviewed changes

tonygermano requested changes Sep 29, 2025

View reviewed changes

mgaffigan added 2 commits September 29, 2025 20:46

Avoid duplication of normalization logic in jnlp unit test

8165e46

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

Revert changes with unclear motivation

f62a8bf

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>

mgaffigan dismissed their stale review via f62a8bf September 30, 2025 01:55

mgaffigan requested a review from tonygermano September 30, 2025 02:28

mgaffigan approved these changes Sep 30, 2025

View reviewed changes

Merge branch 'main' into feature/bl-dependency-cherrypick

7802e3a

gibson9583 marked this pull request as ready for review October 1, 2025 17:26

mgaffigan mentioned this pull request Nov 4, 2025

Fix warnings on build #200

Open

Update Dependencies #146

Are you sure you want to change the base?

Update Dependencies #146

Uh oh!

Conversation

gibson9583 commented Jul 23, 2025

Uh oh!

jonbartels commented Jul 28, 2025

Uh oh!

jonbartels commented Jul 28, 2025

Uh oh!

gibson9583 commented Sep 9, 2025

Uh oh!

mgaffigan commented Sep 9, 2025

Uh oh!

mgaffigan commented Sep 9, 2025

Uh oh!

gibson9583 commented Sep 9, 2025

Uh oh!

mgaffigan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kpalang commented Sep 9, 2025

Uh oh!

mgaffigan commented Sep 29, 2025

Uh oh!

mgaffigan commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonygermano commented Sep 29, 2025

Uh oh!

mgaffigan left a comment

Choose a reason for hiding this comment

Uh oh!

tonygermano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tonygermano Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

mgaffigan Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

mgaffigan Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tonygermano Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

mgaffigan Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

mgaffigan Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tonygermano Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

mgaffigan Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mgaffigan commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

mgaffigan left a comment •

edited

Loading

mgaffigan commented Sep 29, 2025 •

edited

Loading

mgaffigan Sep 30, 2025 •

edited

Loading

mgaffigan commented Sep 30, 2025 •

edited

Loading