-
Notifications
You must be signed in to change notification settings - Fork 18
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
Maven build ermöglichen und automatisieren #614
base: feature-3.1.0
Are you sure you want to change the base?
Conversation
Geht der maven build ohne Änderung der Verzeichnisstruktur? Das würde den PR deutlich übersichtlicher machen. Es ist ohnehin ein Refactoring geplant. Das wollen wir jedoch schrittweise angehen. |
Kann ich probieren. Macht den build initial komplex, da in src/ Code und Ressourcen liegen. Das würde dann erstmal ein bisschen zaubern müssen. |
d83dc52
to
cbc0a17
Compare
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.
Kannst du die file permission changes (100755 → 100644) bitte in einen separaten PR auslagern? Ich halte sie schon für sinnvoll und wir sollten sie unabhängig von diesem PR mergen. Ich hatte schon geschaut, ob es dafür einen Commit gibt. Leider konnte ich keinen identifizieren, zumindest nicht anhand der commit message.
Dann sind auch noch ein paar Dateien geändert, die glaub ich nichts mit maven zu tun haben (z. B. MitgliedLastschriftAction.java
). Ich nehme an, dass du da EOL von Windows zu Unix geändert hast, oder?
Bitte das auch in einen separaten Commit packen und gern auch in vorgenannten PR auslagern.
Diese Anpassungen sind ungewollt passiert, weil ich unter Windows mit Intellij und vermutlich mit einer unpassenden gitconfig arbeite. Ich schaue mal, ob ich es entfernen kann. |
So besser? |
8647850
to
41fa2b5
Compare
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.
Könntest du bitte in der PR-Beschreibung (erster Kommentar) noch ergänzen, wie die Verzeichnisstruktur dann aussieht und was sich am GitHub Actions Workflow ändert (z. B. Caching)?
Ich sehe als Hauptvorteil von Maven, dass wir die Libs aus dem Git Repository entfernen können. Die liegen dann nur noch im local repository. Allerdings entfernt dieser PR sie noch nicht. Das könntest du noch ergänzen.
Oder ist erstmal gedacht, maven nur side-by-side zu verwenden? Dann dürfte der GitHub Actions Workflow jedoch nicht so stark abgeändert werden. Wie ist hier die Intention?
jameica_tag=$(git ls-remote --refs --tags --sort="-v:refname" https://github.com/willuhn/jameica.git V_\* | head -1 | cut -f 2 | cut -d / -f 3) | ||
echo "jameica_tag=${jameica_tag}" >> $GITHUB_ENV | ||
hibiscus_tag=$(git ls-remote --refs --tags --sort="-v:refname" https://github.com/willuhn/hibiscus.git V_\* | head -1 | cut -f 2 | cut -d / -f 3) |
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.
Hier bestimmen wir die aktuellen release tags dynamisch. In der POM hast du sie hart kodiert. Kann man das auch dynamisieren? Sonst verlieren wir wieder Flexibilität.
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.
Das macht den build aber nicht wiederholbar, da er vom Zustand externer Quellen abhängt. Ich würde vorschlagen renovate einzubauen und sehr bewusst die genutzte Version zu aktualisieren.
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.
Das macht den build aber nicht wiederholbar, da er vom Zustand externer Quellen abhängt.
Da hast du Recht. Das ist jedoch für die nightly builds absolut relevant. Nicht, dass wir von den Quellen abhängig wären. Wir bräuchten die Libs von Jameica und Hibiscus. Wenn die in einem maven repository lägen, wäre das ideal. Da könnten zumindest die Releases liegen. Ich hatte auch schon versucht, über Repology die aktuelle Version zu bekommen. Da habe ich jedoch die Rest-API nicht richtig nutzen können. In #516 (comment) hatte ich zumindest mal das Fetching der Jars mit remotezip dokumentiert. Vielleicht ist das für diesen PR eine Möglichkeit? Zumindest braucht es für unsere Releases kein compile der nightlies von Jameica und Hibiscus. Das wäre nur für unser future-proof notwendig.
Ich würde vorschlagen renovate einzubauen.
Meinst du https://github.com/renovatebot/renovate?
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.
Ich meine genau das renovate. Das hat github-tags als data source. D.h. man kann es konfigurieren, automatisch einen pull reuqest zu stellen, wenn Jameica oder Hibiscus ein neue Version bereitstellen. Die baut dann natürlich auch und du siehst sofort, ob es noch baut.
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.
Ich habe übrigens auch schon mal getestet, die benötigten jars in das GitHub maven repository zu installieren, das funktioniert auch, und würde den tatsächlichen build step völlig unabhängig vom reusable-Build machen. Zum lokalen Test braucht man immer noch die Plugin.zip entpackt und es ist wirklich nett, direkt in den source von Jamaica zu schauen.
pom.xml
Outdated
|
||
<groupId>io.github.openjverein</groupId> | ||
<artifactId>jverein</artifactId> | ||
<version>2.8.24-SNAPSHOT</version> |
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.
Das müsste auch irgendwie dynamisiert werden. Wir sind ja gerade bei 2.9.0-nightly
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.
Gibt das maven-release-plugin dafür. Habe ich noch nicht eingebaut. Wie wird jetzt hochgezählt?
<plugin> | ||
<groupId>org.apache.maven.plugins</groupId> | ||
<artifactId>maven-install-plugin</artifactId> | ||
<version>3.1.3</version> |
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.
Ich glaub diese Versionen lassen sich auch zentraler konfigurieren. Zumindest geht das bei Gradle.
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.
Ja, ich würde aber vorschlagen renovate einzubauen. Das stellt einem automatisch prs mit neuen Versionen zu den plugins.
# Eclipse Core | ||
.project | ||
# JDT-specific (Eclipse Java Development Tools) | ||
.classpath |
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.
Diese Dateien würde ich noch nicht gleich ignorieren. Nicht jeder wird sofort Maven verwenden.
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.
Dies sind bereits drin, aber ich kann es gerne wieder rausschmeißen. Mit Maven Projekten ist aber eher schlechter Stil den .classpath im workspace drin zu lassen, da man dann zwei Wahrheiten hat.
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.
Mit Maven und dem m2e plugin kann man sie eigentlich nicht miteinchecken.
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.
Und warum müssen die IDEA-Verzeichnisse/Dateien so detailliert exkludiert werden? Reicht da nicht einfach auch das .idea/
was schon drin ist?
Das gilt natürlich eigentlich auch für die Eclipse prefs. 😏
740979c
to
adb2834
Compare
Werde ich noch ergänzen.
Ich würde es komplett ändern wollen, würde mir aber wünschen, dass es jemand mal lokal bei sich ausprobiert. Dann würde ich die libs rausschmeißen. Alle können nicht raus, da z.B. nc.jar nicht via Maven herunterladbar ist und diese Version nicht im Internet existier. |
1b17de5
to
de9c8bc
Compare
@@ -1,7 +1,7 @@ | |||
<plugin xmlns="https://www.willuhn.de/schema/jameica-plugin" | |||
xmlns:xsi="https://www.w3.org/2001/XMLSchema-instance" | |||
xsi:schemaLocation="https://www.willuhn.de/schema/jameica-plugin https://www.willuhn.de/schema/jameica-plugin-1.5.xsd" | |||
name="jverein" version="2.9.0" class="de.jost_net.JVerein.JVereinPlugin"> | |||
name="jverein" version="${project.version}" class="de.jost_net.JVerein.JVereinPlugin"> |
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.
Dann funktioniert aber die "AboutView" Ausgabe der Version wie in #325 angepasst und damals gefordert nicht mehr.
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.
Wenn man es mit maven package
baut, wird mit dem ressource plugin die plugin.xml gefiltert und enthält dann im jverein-<version>.zip
die korrekte Version. In Maven wird man die Version nur einer Stelle pflegen, in der pom.xml. Alles andere wird beim build erzeugt und nutzt die Daten der pom.xml. So wird sichergestellt, dass es keine Inkonsistenzen gibt. Aber das Starten aus dem workspace passt dann nicht, man müsste das plugin.zip entpacken und als plugin verzeichnis für jameica nutzen. Es separat zu verwalten, fände ich schwierig. Du kannst in meinem repo mal ein release anschauen https://github.com/tobidope/jverein/releases/tag/2.9.0-SNAPSHOT. Dort müsste alles passen.
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.
Es wäre eine Option ein profil lokal zu aktivieren z.B. local-test
, dass die plugin.xml für den Test generiert. Die Basis dafür wäre ein plugin.xml aus den src/main/resources
.
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.
Aber das Starten aus dem workspace passt dann nicht, man müsste das plugin.zip entpacken und als plugin verzeichnis für jameica nutzen.
Kann man nicht ein inject über die run configuration machen?
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.
Ich habe eingebaut, dass mvn verify das plugin.zip im Verzeichnis target entpackt. Wenn dies als plugin quelle genutzt wird, funktioniert alles so wie es soll. Ich muss die Doku vom PR und CONTRIBUTING.md noch anpassen.
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.
Dann funktioniert aber die "AboutView" Ausgabe der Version wie in #325 angepasst und damals gefordert nicht mehr.
Ich glaube @dippeal meinte, dass man dann kein live debugging und live code changes mehr machen kann, wenn das Plugin nicht mehr vom source Verzeichnis geladen werden kann (wegen fehlender Version), wie es unter Wie richte ich mir das Beispiel-Plugin in Eclipse ein? beschrieben ist.
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.
Das Problem war, dass die Plugin Version nicht geparst werden konnte und beim starten aus der IDE immer ein Exception kam. Außerdem sind abhängige Plugins dann nicht geladen. Siehe #324
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.
Das Problem gibt es nicht. Man nutzt einfach die Version im Target Ordner. Bei Änderungen muss ein mvn verify durchgeführt werden, damit der Test funktioniert.
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.
Die Exception kommt mit diesem PR auch nicht.
Soll ich hier weitermachen, oder ist eine Aufnahme eher unwahrscheinlich? |
7f7815b
to
0d2421e
Compare
4088907
to
0cb1d62
Compare
Ich glaub vor dem 3.0er Release kommt es nicht rein. Danach kann ich mir das vorstellen. Dann haben wir alle genug Zeit zum Testen und dran gewöhnen. Ich werde es demnächst lokal ausprobieren. |
9ec2b4e
to
3ff6449
Compare
Kannst du bitte das force-push immer nur dann machen, wenn es wirklich notwendig ist? Solange wie es niemand ausgechecked hat, ist es noch unproblematisch. Das wird jedoch irgendwann zum Problem. |
Ich versuche immer nah am master branch zu bleiben. Solange keiner mit mir am branch arbeiten wollte, sollte es unproblematisch sein. Wenn man länger nicht rebased kann das ganz schön fummelig sein. Ist mir diese Woche passiert. |
afdf882
to
04f2311
Compare
Damit kann der lokale Test durch Einbinden von target/jverein als plugin in Jameica durchgeführt werden
03ff6b3
to
cb941c3
Compare
Ich mache das erstmal zu. Ich fange kleiner an. Erstmal den ant build sinnvoll verbessern und wenn das alles läuft. Können wir vielleicht auf Maven oder Gradle wechseln. |
Oh, dass finde ich jetzt überraschend. Ich hätte es begrüßt, wenn wir auf Maven (oder Gradle) gewechselt wären. |
Ich hatte das Gefühl, das ist zu groß um in endlicher Zeit gemerged zu werden. Den Branch habe ich noch. |
Wir hatten schon einige PRs, die eine Reviewzeit von mehreren Monaten hatten. Ich halte den PR für ziemlich wichtig. Daher öffne ich ihn wieder. |
Mit diesem PR wird der build von JVerein von ant auf Maven umgestellt.
Um den Build mit Maven durchzuführen, sind folgende Schritte notwendig:
mvn -f setup-build.xml clean install
ausführenIn diesem Schritt werden die Quelltexte von Jameica und Hibiscus aus GitHub heruntergeladen, entpackt und gebaut. Die für JVerein benötigten Bibliotheken aus Jameica, Hibiscus und JVerein werden anschließend in das lokale Maven-Repository installiert und stehen damit im eigentlichen Build zur Verfügung. Außerdem wird die Abhängigkeit zu NumericalChameleon von Sourceforge gezogen und verfügbar gemacht.
mvn clean verify
ausführenIm Maven-Build werden die Abhängigkeiten nicht mehr aus den lib Ordnern bezogen, sondern ausschließlich aus dem lokalen oder dem zentralen Maven-Repository. Das Ergebnis ist eine Plugin-ZIP, die analog zum früheren Ant-Build erstellt wird. Sourcen, Tests und Ressourcen sind noch an alter Stelle. Das plugin.zip ist im target Verzeichnis entpackt unter dem Ordner target/jverein. Dieses Verzeichnis kann als plugin in Jameica eingebunden werden.
Der größte Vorteil besteht für mich darin, dass neue Entwickler:innen, die an JVerein mitarbeiten möchten, weniger manuelle Schritte durchführen müssen und mit einem zeitgemäßen Build-Tool arbeiten können. Dinge wie das Encoding oder die JDK-Version werden automatisch geprüft. In Zukunft könnten wir weitere Maven-Plugins verwenden, um den Entwicklungsprozess weiter zu vereinfachen.
Der ganze build Prozess wird wesentlich vereinfacht und enthält keine dynamischen Abhängigkeiten. Ein neues Release von Hibiscus oder Jameica führt nicht dazu, dass alte Versionen auf einmal nicht mehr bauen. Zudem ist der build schneller, da das caching nur noch die benötigten Abhängigkeiten nutzt. Der Cache schrumpft von 200 MB auf 90 MB.
Bitte testet den Ablauf und gebt eine Rückmeldung, ob dieser PR gemergt werden kann. Die Anbindung an GitHub Actions bringe ich gerade zuende.