Mit Java 17 kompilieren #663

tobidope · 2025-02-12T20:16:57Z

Mit dem maven build hat es ausgereicht, auf jdk 17 zu wechseln. Alles andere habe ich auf 11 geleasen (maven.compiler.release)

fixes #659

tobidope · 2025-02-12T20:20:36Z

Was muss ich denn tun, damit es auf Github Actions baut?

tolot27

Das funktioniert alles für 3.x+. Bei backports/Bugfixes sollte dieser commit nicht gemerged werden.

tobidope · 2025-02-12T20:59:56Z

Das funktioniert alles für 3.x+. Bei backports/Bugfixes sollte dieser commit nicht gemerged werden.

Kannst du einmal erklären, warum das so ist? Da fehlt mit die Übersicht. Das target bleibt unverändert bei 11 im ant build.

dippeal · 2025-02-13T07:09:08Z

.github/workflows/buildcheck.yml

@@ -12,6 +12,7 @@ on:
      - 'lib/**'
      - 'lib.src/**'
      - 'src/**'
+      - '.github/**'


Erkläre bitte warum der .github Ordner geprüft werden soll.

Erkläre bitte warum der .github Ordner geprüft werden soll.

Nur so funktioniert der buildcheck auch bereits bei diesem PR.

Eigentlich würde .github/workflows reichen.

So richtig wohl ist mir dabei auch nicht und es erfordert einen sorgfältigen Review-Prozess, dass nicht irgend ein PR auch mal was an den Actions ändert. Irgendwo hatte ich da mal was mit Abfluss von Informationen (z. B. Tokens), insbesondere im Zusammenhang mit pull_request und pull_request_target gelesen. Ich glaub das war in Accessing contextual information about workflow runs.

Will einer anpassen oder soll ich später?

Ach du hast das nicht in deinem fork master getestet oder wie? Ich habe die buildchecks immer in meinem repo getestet bevor ich den PR erstellt habe.

Da hat es natürlich auch gebaut, weil ich den workflow angepasst habe. Aber dann baut es natürlich auch hier. Oder wie ist das gemeint? Anders gesagt, ich habe es auch im maven build getestet und ich baue lokal mit Java 21. Da baut und läuft es auch.

Das Problem ist, dass nun jeder Änderung (oder vollständige Ersetzung) an der buildcheck.yml von irgend einem neuen PR automatisch ausgeführt wird. Damit kann man schon einiges anstellen. Vor deinem letzten Commit 46dd5d8 war es noch krasser, da damit neue Actions einfach durch Erstellen eines PRs hätten "untergeschoben" werden können. Alles natürlich erst nachdem dieser PR gemerged wird.
Das ist natürlich nicht unbedingt gewünscht. Daher nehmen wir es bisher in Kauf, dass die PR buildchecks fehlschlagen, bis wir die Workflows "kontrolliert" angepasst haben.

Das konnte man doch schon vorher. Die Actions laufen grundsätzlich. Ich kann auch ein bisschen white space in einer Java Datei ändern und einen workflow anpassen oder hinzufügen. Oder übersehe ich was? Das wichtige secret, der GITHUB_TOKEN ist nur für den run gültig, aber damit kann man natürlich schon was anfangen. Daher sollte der prinzipiell in den meisten Läufen so angepasst sein, dass er nur lesend auf das repo zugreifen kann und nicht mehr.

permissions: contents: read

Das konnte man doch schon vorher. Die Actions laufen grundsätzlich. Ich kann auch ein bisschen white space in einer Java Datei ändern und einen workflow anpassen oder hinzufügen. Oder übersehe ich was?

Ne, ich hatte bisher ein anderes Verhalten bei meinen Tests bzw. der Implementierung des buildchecks beobachtet, die jedoch vermutlich falsch waren. Ich meinte, beobachtet zu haben, dass immer die Workflow-Konfiguration (.yml) der base branch verwendet wird. Auf dieser Annahme basierten meine Kommentare. Wie ich mit #671 jedoch geprüft habe, wird doch das yml des PR verwendet. Daher könnten wir durchaus in den Actions den Pfad .github/workflows mit aufnehmen. Kannst du das bitte so setzen?

Worauf ich eigentlich hinaus will, ist GitHub Actions Pinning. Das sollten wir in einem anderen PR machen und dazu unsere Tags nutzen (statt die commit SHAs). Damit können wir auch das in #663 (comment) angesprochene Problem lösen, indem wir das in ein Bugfix release für die älteren Versionen so mit aufnehmen.

So habe ich wieder einiges über die GitHub Actions gelernt. 😄

Das wichtige secret, der GITHUB_TOKEN ist nur für den run gültig, aber damit kann man natürlich schon was anfangen. Daher sollte der prinzipiell in den meisten Läufen so angepasst sein, dass er nur lesend auf das repo zugreifen kann und nicht mehr.

permissions: contents: read

Es ist sicher eine gute Idee, die notwendigen Permissions nochmal zu validieren und ggf. zu setzen. Das können wir gern in einem weiteren PR machen oder zunächst in einer Discussion. Allerdings kann es sein, dass die Read Permission bereits in den repository/organization settings gesetzt ist. Das kann ich jedoch nicht prüfen, da ich dazu nicht die nötige Berechtigung habe. Vielleicht kann das @dippeal ?

tolot27 · 2025-02-13T08:54:49Z

Das funktioniert alles für 3.x+. Bei backports/Bugfixes sollte dieser commit nicht gemerged werden.

Kannst du einmal erklären, warum das so ist? Da fehlt mit die Übersicht. Das target bleibt unverändert bei 11 im ant build.

Weil wir sonst alle früheren Version, für die wir Bugfixes rausgeben/releasen nicht mehr gegen die damals verwendeten Jameica- und Hibiscus-Versionen "linken" würden, sondern gegen die neuen. Das könnte wieder zu Build- und Execute-Problemen führen.

tolot27 · 2025-02-13T08:57:35Z

@tobidope Hattest du schon mal geprüft, ob unsere master bzw. feature branch jars mit den 17er .class-Dateien in den libs klarkommen und unser Java11-Plugin weiterhin funktioniert?

tobidope · 2025-02-13T09:13:28Z

Nein. Aber zur Laufzeit muss mit der neuen Jameica Version Java 17 genutzt werden. Swt kommt in Java 17. Also alles darunter kann per definition nicht genutzt werden. Vielleicht der headless mode, aber da habe ich keine Ahnung.

tobidope · 2025-02-13T09:24:13Z

Das funktioniert alles für 3.x+. Bei backports/Bugfixes sollte dieser commit nicht gemerged werden.

Kannst du einmal erklären, warum das so ist? Da fehlt mit die Übersicht. Das target bleibt unverändert bei 11 im ant build.

Weil wir sonst alle früheren Version, für die wir Bugfixes rausgeben/releasen nicht mehr gegen die damals verwendeten Jameica- und Hibiscus-Versionen "linken" würden, sondern gegen die neuen. Das könnte wieder zu Build- und Execute-Problemen führen.

Ein Grund mehr hart gegen bestimmte Versionen von Jameica und Hibiscus zu bauen. Das aktuelle nightly sollte nicht der default sein, sondern ein Sonderlauf oder renovate branch.

dippeal · 2025-02-13T12:37:14Z

Das funktioniert alles für 3.x+. Bei backports/Bugfixes sollte dieser commit nicht gemerged werden.

Kannst du einmal erklären, warum das so ist? Da fehlt mit die Übersicht. Das target bleibt unverändert bei 11 im ant build.

Weil wir sonst alle früheren Version, für die wir Bugfixes rausgeben/releasen nicht mehr gegen die damals verwendeten Jameica- und Hibiscus-Versionen "linken" würden, sondern gegen die neuen. Das könnte wieder zu Build- und Execute-Problemen führen.

Ein Grund mehr hart gegen bestimmte Versionen von Jameica und Hibiscus zu bauen. Das aktuelle nightly sollte nicht der default sein, sondern ein Sonderlauf oder renovate branch.

Sehe ich ähnlich. Wollten einige hier nicht und lieber die neuesten tags geladen haben.

tolot27 · 2025-02-13T13:21:25Z

Sehe ich ähnlich. Wollten einige hier nicht und lieber die neuesten tags geladen haben.

Man könnte das ja auftrennen zwischen den verschiedenen branch/tag Kategorien (master/dev, feature, bugfix) bzw. bei dem Buildcheck auch als Matrix-Check implementieren.

JohannMaierhofer · 2025-02-13T13:58:40Z

Also, in unserem plugin.xml steht, dass wir Jameica 2.8+ voraussetzen. Das bedeutet doch, dass wir selbst mit der ersten 2.8.x funktionieren müssen.
Dann müssten wir eigentlich diese Version als Grundlage nehmen, oder wir ziehen das schon mal auf 2.10+ hoch.
Bei Jameica 2.8.6 müssten wir laut Kompatibilitätsmatrix nämlich noch mit Java 8 kompilieren.

tobidope · 2025-02-15T14:45:53Z

Was hindert uns daran, den pr zu mergen? Im build.xml wird weiterhin kompatibel zu Java 11 gebaut, da der Parameter release weiterhin auf 11 steht.

 <target name="compile" depends="init, clean">
    <mkdir dir="${class.dir}" />
    <javac debug="true" includeantruntime="false" debuglevel="lines,vars,source" release="${define.java.version}" encoding="${define.encoding}" deprecation="true" destdir="${class.dir}" srcdir="${src.dir}">
			<classpath refid="compilepath" />
		</javac>
  </target>

Es ist kein breaking change, es führt nur dazu, dass im aktuellen System der build wieder funktioniert.

Mit Java 17 kompilieren

1400516

tobidope marked this pull request as ready for review February 12, 2025 20:17

Auch bei Anpassung der Github Actions bauen

5519902

tobidope requested review from tolot27 and dippeal February 12, 2025 20:27

tolot27 previously approved these changes Feb 12, 2025

View reviewed changes

dippeal reviewed Feb 13, 2025

View reviewed changes

tolot27 mentioned this pull request Feb 13, 2025

Fix Spendenbescheinigung Mailversand und Konto Buchungsartauswahl #666

Open

Eingeschränkt auf buildcheck.yml

46dd5d8

tobidope dismissed tolot27’s stale review via 46dd5d8 February 13, 2025 17:05

tolot27 mentioned this pull request Feb 14, 2025

demonstrate base workflow action #671

Closed

tobidope requested review from tolot27 and dippeal February 15, 2025 14:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mit Java 17 kompilieren #663

Mit Java 17 kompilieren #663

tobidope commented Feb 12, 2025 •

edited

Loading

tobidope commented Feb 12, 2025

tolot27 left a comment

tobidope commented Feb 12, 2025

dippeal Feb 13, 2025

tolot27 Feb 13, 2025

tolot27 Feb 13, 2025

tolot27 Feb 13, 2025

tobidope Feb 13, 2025

dippeal Feb 13, 2025

tobidope Feb 13, 2025

tolot27 Feb 13, 2025

tobidope Feb 13, 2025

tolot27 Feb 14, 2025

tolot27 commented Feb 13, 2025

tolot27 commented Feb 13, 2025

tobidope commented Feb 13, 2025

tobidope commented Feb 13, 2025

dippeal commented Feb 13, 2025

tolot27 commented Feb 13, 2025

JohannMaierhofer commented Feb 13, 2025

tobidope commented Feb 15, 2025

Mit Java 17 kompilieren #663

Are you sure you want to change the base?

Mit Java 17 kompilieren #663

Conversation

tobidope commented Feb 12, 2025 • edited Loading

tobidope commented Feb 12, 2025

tolot27 left a comment

Choose a reason for hiding this comment

tobidope commented Feb 12, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tolot27 commented Feb 13, 2025

tolot27 commented Feb 13, 2025

tobidope commented Feb 13, 2025

tobidope commented Feb 13, 2025

dippeal commented Feb 13, 2025

tolot27 commented Feb 13, 2025

JohannMaierhofer commented Feb 13, 2025

tobidope commented Feb 15, 2025

tobidope commented Feb 12, 2025 •

edited

Loading