Skip to content

Conversation

@abrahamwolk
Copy link
Collaborator

@abrahamwolk abrahamwolk commented Sep 4, 2025

This pull request fixes building of Phoebus for the the platform mac-aarch64 by using the Phoebus versions of javafx-base and javafx-controls for the dependency 'chartfx'. (The observed issue is described here: #3529). The fix is due to @georgweiss who found both the reason and the fix that is included in this PR.

The reason for the observed issue is that the oldest version of javafx-base and javafx-controls that exist in Maven for the platform mac-aarch64 is version 17. However, ChartFX depends on version 16 of these libraries.

ChartFX's pom.xml states the following reason for using version 16 of ChartFX:

<!-- starting with javafx 17, the chart will not be garbage collected as soon as a plugin is present -->

Indeed, if I build ChartFX with the version of the JavaFX-dependencies set to 17 and run ChartFX's tests, then I get an error about references not being garbage collected that should be garbage collected. (I had to override ObservableDeque.reversed() as described in fair-acc/chart-fx#655 (comment) in order to build ChartFX with Java version 21.)

When I set ChartFX's JavaFX version to 18, 19, or 20, all ChartFX-tests succeed for me.

However, when I set ChartFX's JavaFX to version 21.0.7 (the version we currently use in Phoebus), then I get ChartFX-tests failing with java.lang.reflect.InvocationTargetException. I think (but I am not sure) that these exceptions are thrown due to some change in JavaFX, and that the tests need updating: the tests don't fail, but rather exit with a runtime error.

If I test Phoebus built with the changes in this PR, then the Waterfall Plot widget seems to work.

…arch64' by using the Phoebus versions of javafx-base and javafx-controls for the dependency 'chartfx'.
Copy link
Collaborator

@georgweiss georgweiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see how this will work on upcoming JavaFX 25.

@kasemir
Copy link
Collaborator

kasemir commented Sep 4, 2025

Glad that you found a workaround

@abrahamwolk
Copy link
Collaborator Author

abrahamwolk commented Sep 4, 2025

There were duplicate dependencies in the pom.xml of the Waterfall Plot widget, the naming of the Waterfallplot widget by the build system was also not optimal, and the parent of the waterfallplot seems not to have been set correctly, as reported in #3531. I have improved these three issues in the commit 014e8b5 which I have added to this pull request.

Copy link
Collaborator

@kasemir kasemir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the "app-display-waterfallplot" renaming!! I was about to comment on the mismatch of app/display/waterfallplot for location vs app-waterfall-plot for name, but felt that'd be overly picky.

@abrahamwolk
Copy link
Collaborator Author

I don't think that'd be overly picky.

@abrahamwolk abrahamwolk merged commit 3586cfc into master Sep 4, 2025
3 checks passed
@abrahamwolk abrahamwolk deleted the CSSTUDIO-3427 branch September 4, 2025 13:40
@kasemir
Copy link
Collaborator

kasemir commented Sep 4, 2025

Well, ideally the location in the source tree, the jar/module file name, and package name would all match. So if you get an exception from org.phoebus.app.xxx you know that it's from the app-xxx.jar, the sources are under app/xxx/src and so on.
We have many sources that started with the Eclipse-based CSS. Like

  • Location: core/formula
  • Jar/module/id: core-formula
  • Package: org.csstudio.apputil.formula

It's "close enough", you can guess where an exception might originate, but in the fullness of time maybe the package name should become org.phoebus.app.formula??

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.

4 participants