From f9b264312c7c1bf6d607cabe940e670b63ad4d53 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:21:29 +0200 Subject: [PATCH 01/13] First try --- .github/workflows/tests-code.yml | 55 ++++++++++++++++- ...g-script-modification-for-testing-in-ci.md | 61 +++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 docs/decisions/0048-jbang-script-modification-for-testing-in-ci.md diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index bbf592ce7b2..ae9cea6f380 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -393,10 +393,10 @@ jobs: CI: "true" DBMS: "postgresql" + jbang: name: JBang runs-on: ubuntu-latest - # JBang scripts depend on main-SNAPSHOT; thus, we can only test fully when main is updated if: github.ref == 'refs/heads/main' steps: - name: Checkout source @@ -428,6 +428,59 @@ jobs: - run: jbang build --fresh .jbang/JabKitLauncher.java - run: jbang build --fresh .jbang/JabSrvLauncher.java + jbang-pr: + name: JBang + runs-on: ubuntu-latest + if: github.ref != 'refs/heads/main' + strategy: + matrix: + launcher: [JabKitLauncher, JabLsLauncher, JabServLauncher] + steps: + - name: Checkout source + uses: actions/checkout@v5 + with: + submodules: 'false' + show-progress: 'false' + - name: Set up JDK + uses: actions/setup-java@v5 + with: + java-version: 24 + distribution: 'corretto' + - name: Generate JBang cache key + id: cache-key + shell: bash + run: | + echo "cache_key=jbang-$(date +%F)" >> $GITHUB_OUTPUT + - name: Use cache + uses: actions/cache@v4 + with: + path: ~/.jbang + key: ${{ steps.cache-key.outputs.cache_key }} + restore-keys: + jbang- + - name: Setup JBang + uses: jbangdev/setup-jbang@main + - run: jbang build .jbang/CheckoutPR.java + - run: jbang build .jbang/CloneJabRef.java + + - name: Detect changed jablib classes + id: changed-jablib-files + uses: tj-actions/changed-files@v45 + with: + files: | + jablib/src/main/java/**/*.java + + - name: Build ${{ matrix.launcher }} launcher including changed classes + shell: bash + if: steps.changed-jablib-files.outputs.any_changed == 'true' + run: | + WRAPPER="$(mktemp --suffix=.java)" + cp .jbang/${{ matrix.launcher }}.java "$WRAPPER" + for f in ${{ steps.changed-jablib-files.outputs.all_changed_files }}; do + echo "//SOURCES $f" >> "$WRAPPER" + done + jbang build --fresh "$WRAPPER" + codecoverage: if: false name: Code coverage diff --git a/docs/decisions/0048-jbang-script-modification-for-testing-in-ci.md b/docs/decisions/0048-jbang-script-modification-for-testing-in-ci.md new file mode 100644 index 00000000000..73da7c25794 --- /dev/null +++ b/docs/decisions/0048-jbang-script-modification-for-testing-in-ci.md @@ -0,0 +1,61 @@ +--- +nav_order: 48 +parent: Decision Records +--- + +# JBang script modification for testing in CI + +## Context and Problem Statement + +All JBang scripts on `main` should always compile. + +Example: + +```terminal +gg.cmd https://github.com/JabRef/jabref/blob/main/.jbang/JabLsLauncher.java +``` + +- JBang scripts link to `org.jabref:jablib:6.0-SNAPSHOT`. +- JBang scripts include java files from the respective `-cli` repository and the sources of the respective server project. E.g., `JabLsLauncher.java` includes `ServerCli.java` from `jabls-cli` and all sources from `jabls`. +- Code changes might change a) things inside the server project and b) things in JabRef's logic. + +As a consequence, the JBang script might break. + +Case a) can be detected when running the JBang check in the respective PR. +Case b) can be detected when running the JBang check in the main branch, because `org.jabref:jablib:6.0-SNAPSHOT` is updated there. + +We aim for checking JBang scripts in PRs and on `main`. + +## Decision Drivers + +* Fast detection of issues at all JBang files +* Easy CI pipeline + +## Considered Options + +* Have JBang script have all changed classes of `jablib` included directly. +* Use jitpack +* Temporary `-SNAPSHOT.jar` + +## Decision Outcome + +Chosen option: "Have JBang script have all changed classes of `jablib` included directly.", because comes out best (see below). + +## Pros and Cons of the Options + +### Have JBang script have all changed classes of `jablib` included directly. + +- `org.jabref:jablib:6.0-SNAPSHOT` provides non-modified classes +- modified classes are included using JBang's `//SOURCES` directive. + +[tj-actions/changed-files](https://github.com/marketplace/actions/changed-files) can be used. + +* Good, because least effort + +### Use jitpack + +- Bad, because jitpack is unreliable + +### Temporary `-SNAPSHOT.jar` + +- Bad, because setting up a temporary maven repository is effort. From 2c8c034aa4fd49a747799030f332fdf4352ae71c Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:24:20 +0200 Subject: [PATCH 02/13] Remove empty line --- .github/workflows/tests-code.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index ae9cea6f380..ae76b793772 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -393,7 +393,6 @@ jobs: CI: "true" DBMS: "postgresql" - jbang: name: JBang runs-on: ubuntu-latest From 1d88c37670f5839307c13c13bb3b0d79c47ae7dd Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:25:47 +0200 Subject: [PATCH 03/13] Add check for JabLSLauncher Co-authored-by: Philip <37398281+palukku@users.noreply.github.com> --- .github/workflows/tests-code.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index ae76b793772..ed97efdb1ef 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -425,6 +425,7 @@ jobs: - run: jbang build .jbang/CheckoutPR.java - run: jbang build .jbang/CloneJabRef.java - run: jbang build --fresh .jbang/JabKitLauncher.java + - run: jbang build --fresh .jbang/JabLsLauncher.java - run: jbang build --fresh .jbang/JabSrvLauncher.java jbang-pr: From b5c2e52834ea37ef14094a0a760967a963bd0f39 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:26:18 +0200 Subject: [PATCH 04/13] Run if the workflow itself changed --- .github/workflows/tests-code.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index ed97efdb1ef..3518df5041c 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -468,6 +468,7 @@ jobs: uses: tj-actions/changed-files@v45 with: files: | + .github/workflows/tests-code.yml jablib/src/main/java/**/*.java - name: Build ${{ matrix.launcher }} launcher including changed classes From 598cc4b751c10507f0ff1250542a031b17adf911 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:27:37 +0200 Subject: [PATCH 05/13] Fix generation --- .github/workflows/tests-code.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index 3518df5041c..f1ce3bf1372 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -478,7 +478,9 @@ jobs: WRAPPER="$(mktemp --suffix=.java)" cp .jbang/${{ matrix.launcher }}.java "$WRAPPER" for f in ${{ steps.changed-jablib-files.outputs.all_changed_files }}; do - echo "//SOURCES $f" >> "$WRAPPER" + if [[ $f == *.java ]]; then + echo "//SOURCES $f" >> "$WRAPPER" + fi done jbang build --fresh "$WRAPPER" From 9e0e9184a517ece8f9b9bdf54de66c300f56695e Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:28:51 +0200 Subject: [PATCH 06/13] Fix markdown lint issues --- ...-jbang-script-modification-for-testing-in-ci.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/decisions/0048-jbang-script-modification-for-testing-in-ci.md b/docs/decisions/0048-jbang-script-modification-for-testing-in-ci.md index 73da7c25794..3272bc7ed06 100644 --- a/docs/decisions/0048-jbang-script-modification-for-testing-in-ci.md +++ b/docs/decisions/0048-jbang-script-modification-for-testing-in-ci.md @@ -28,14 +28,14 @@ We aim for checking JBang scripts in PRs and on `main`. ## Decision Drivers -* Fast detection of issues at all JBang files -* Easy CI pipeline +- Fast detection of issues at all JBang files +- Easy CI pipeline ## Considered Options -* Have JBang script have all changed classes of `jablib` included directly. -* Use jitpack -* Temporary `-SNAPSHOT.jar` +- Have JBang script have all changed classes of `jablib` included directly +- Use jitpack +-Temporary `-SNAPSHOT.jar` ## Decision Outcome @@ -43,14 +43,14 @@ Chosen option: "Have JBang script have all changed classes of `jablib` included ## Pros and Cons of the Options -### Have JBang script have all changed classes of `jablib` included directly. +### Have JBang script have all changed classes of `jablib` included directly - `org.jabref:jablib:6.0-SNAPSHOT` provides non-modified classes - modified classes are included using JBang's `//SOURCES` directive. [tj-actions/changed-files](https://github.com/marketplace/actions/changed-files) can be used. -* Good, because least effort +- Good, because least effort ### Use jitpack From 8cfe322d2226438eca644ab9fedeabd2b3f2ebd1 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:30:42 +0200 Subject: [PATCH 07/13] Remove CheckoutPR and CloneJabRef from the PR check --- .github/workflows/tests-code.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index f1ce3bf1372..18ee9ad9de0 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -460,8 +460,6 @@ jobs: jbang- - name: Setup JBang uses: jbangdev/setup-jbang@main - - run: jbang build .jbang/CheckoutPR.java - - run: jbang build .jbang/CloneJabRef.java - name: Detect changed jablib classes id: changed-jablib-files From 917fdd5bdc1d5f513b595b9ddf88f797cba06f38 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:32:23 +0200 Subject: [PATCH 08/13] Fix typo --- .github/workflows/tests-code.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index 18ee9ad9de0..0d57b7414a3 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -434,7 +434,7 @@ jobs: if: github.ref != 'refs/heads/main' strategy: matrix: - launcher: [JabKitLauncher, JabLsLauncher, JabServLauncher] + launcher: [JabKitLauncher, JabLsLauncher, JabSrvLauncher] steps: - name: Checkout source uses: actions/checkout@v5 From e5afc2656e1571b8949e66ddb42514786adc09ff Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:33:11 +0200 Subject: [PATCH 09/13] No fail fast on JBang tests --- .github/workflows/tests-code.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index 0d57b7414a3..78eee28a64e 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -433,6 +433,7 @@ jobs: runs-on: ubuntu-latest if: github.ref != 'refs/heads/main' strategy: + fail-fast: false matrix: launcher: [JabKitLauncher, JabLsLauncher, JabSrvLauncher] steps: From 476b073e0fd0e56821eee0d293e14a177fd0173d Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:34:09 +0200 Subject: [PATCH 10/13] Just run --- .github/workflows/tests-code.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index 78eee28a64e..6af91776e59 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -467,12 +467,10 @@ jobs: uses: tj-actions/changed-files@v45 with: files: | - .github/workflows/tests-code.yml jablib/src/main/java/**/*.java - name: Build ${{ matrix.launcher }} launcher including changed classes shell: bash - if: steps.changed-jablib-files.outputs.any_changed == 'true' run: | WRAPPER="$(mktemp --suffix=.java)" cp .jbang/${{ matrix.launcher }}.java "$WRAPPER" From 85ad0cd4a7af3356488b4c300ffb008c2c7b2b31 Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:36:27 +0200 Subject: [PATCH 11/13] Patch real path --- .github/workflows/tests-code.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index 6af91776e59..6c32d0fb4a3 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -472,14 +472,12 @@ jobs: - name: Build ${{ matrix.launcher }} launcher including changed classes shell: bash run: | - WRAPPER="$(mktemp --suffix=.java)" - cp .jbang/${{ matrix.launcher }}.java "$WRAPPER" for f in ${{ steps.changed-jablib-files.outputs.all_changed_files }}; do if [[ $f == *.java ]]; then - echo "//SOURCES $f" >> "$WRAPPER" + echo "//SOURCES $f" >> ".jbang/${{ matrix.launcher }}.java" fi done - jbang build --fresh "$WRAPPER" + jbang build --fresh ".jbang/${{ matrix.launcher }}.java" codecoverage: if: false From 7a2b4bc3355fd85c5dec0c1f478d21657ac0841a Mon Sep 17 00:00:00 2001 From: Oliver Kopp Date: Wed, 27 Aug 2025 13:38:39 +0200 Subject: [PATCH 12/13] Refine comments --- .github/workflows/tests-code.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/tests-code.yml b/.github/workflows/tests-code.yml index 6c32d0fb4a3..1fa4355d242 100644 --- a/.github/workflows/tests-code.yml +++ b/.github/workflows/tests-code.yml @@ -394,7 +394,7 @@ jobs: DBMS: "postgresql" jbang: - name: JBang + name: JBang (main) runs-on: ubuntu-latest if: github.ref == 'refs/heads/main' steps: @@ -429,7 +429,7 @@ jobs: - run: jbang build --fresh .jbang/JabSrvLauncher.java jbang-pr: - name: JBang + name: JBang (PR) runs-on: ubuntu-latest if: github.ref != 'refs/heads/main' strategy: @@ -471,11 +471,11 @@ jobs: - name: Build ${{ matrix.launcher }} launcher including changed classes shell: bash + # We always run, because changes in jabls, jabsrv also could cause JBang to fail run: | + # We modify the JBang scripts directly to avoid issues with relative paths for f in ${{ steps.changed-jablib-files.outputs.all_changed_files }}; do - if [[ $f == *.java ]]; then - echo "//SOURCES $f" >> ".jbang/${{ matrix.launcher }}.java" - fi + echo "//SOURCES $f" >> ".jbang/${{ matrix.launcher }}.java" done jbang build --fresh ".jbang/${{ matrix.launcher }}.java" From fcc57ab7ffe6b0b431371281fd35909bc34b79f8 Mon Sep 17 00:00:00 2001 From: Philip <37398281+palukku@users.noreply.github.com> Date: Wed, 27 Aug 2025 13:43:38 +0200 Subject: [PATCH 13/13] fix JabLsLauncher --- .jbang/JabLsLauncher.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.jbang/JabLsLauncher.java b/.jbang/JabLsLauncher.java index cfb9e1ecc18..ea64b2320e7 100644 --- a/.jbang/JabLsLauncher.java +++ b/.jbang/JabLsLauncher.java @@ -10,8 +10,14 @@ //SOURCES ../jabls/src/main/java/org/jabref/languageserver/BibtexTextDocumentService.java //SOURCES ../jabls/src/main/java/org/jabref/languageserver/BibtexWorkspaceService.java -//SOURCES ../jabls/src/main/java/org/jabref/languageserver/LSPLauncher.java -//SOURCES ../jabls/src/main/java/org/jabref/languageserver/LSPServer.java +//SOURCES ../jabls/src/main/java/org/jabref/languageserver/controller/LanguageServerController.java +//SOURCES ../jabls/src/main/java/org/jabref/languageserver/ExtensionSettings.java +//SOURCES ../jabls/src/main/java/org/jabref/languageserver/LspClientHandler.java +//SOURCES ../jabls/src/main/java/org/jabref/languageserver/LspLauncher.java +//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspConsistencyCheck.java +//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspDiagnosticBuilder.java +//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspDiagnosticHandler.java +//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspIntegrityCheck.java // REPOS mavencentral,snapshots=https://central.sonatype.com/repository/maven-snapshots/ // REPOS mavencentral,mavencentralsnapshots=https://central.sonatype.com/repository/maven-snapshots/,s01oss=https://s01.oss.sonatype.org/content/repositories/snapshots/,oss=https://oss.sonatype.org/content/repositories,jitpack=https://jitpack.io,oss2=https://oss.sonatype.org/content/groups/public,ossrh=https://oss.sonatype.org/content/repositories/snapshots