Skip to content

Commit d2dfc6e

Browse files
kopporpalukku
andauthored
Refine JBang check (#13765)
* First try * Remove empty line * Add check for JabLSLauncher Co-authored-by: Philip <37398281+palukku@users.noreply.github.com> * Run if the workflow itself changed * Fix generation * Fix markdown lint issues * Remove CheckoutPR and CloneJabRef from the PR check * Fix typo * No fail fast on JBang tests * Just run * Patch real path * Refine comments * fix JabLsLauncher --------- Co-authored-by: Philip <37398281+palukku@users.noreply.github.com>
1 parent 1e7053a commit d2dfc6e

File tree

3 files changed

+122
-4
lines changed

3 files changed

+122
-4
lines changed

.github/workflows/tests-code.yml

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -394,9 +394,8 @@ jobs:
394394
DBMS: "postgresql"
395395

396396
jbang:
397-
name: JBang
397+
name: JBang (main)
398398
runs-on: ubuntu-latest
399-
# JBang scripts depend on main-SNAPSHOT; thus, we can only test fully when main is updated
400399
if: github.ref == 'refs/heads/main'
401400
steps:
402401
- name: Checkout source
@@ -426,8 +425,60 @@ jobs:
426425
- run: jbang build .jbang/CheckoutPR.java
427426
- run: jbang build .jbang/CloneJabRef.java
428427
- run: jbang build --fresh .jbang/JabKitLauncher.java
428+
- run: jbang build --fresh .jbang/JabLsLauncher.java
429429
- run: jbang build --fresh .jbang/JabSrvLauncher.java
430430

431+
jbang-pr:
432+
name: JBang (PR)
433+
runs-on: ubuntu-latest
434+
if: github.ref != 'refs/heads/main'
435+
strategy:
436+
fail-fast: false
437+
matrix:
438+
launcher: [JabKitLauncher, JabLsLauncher, JabSrvLauncher]
439+
steps:
440+
- name: Checkout source
441+
uses: actions/checkout@v5
442+
with:
443+
submodules: 'false'
444+
show-progress: 'false'
445+
- name: Set up JDK
446+
uses: actions/setup-java@v5
447+
with:
448+
java-version: 24
449+
distribution: 'corretto'
450+
- name: Generate JBang cache key
451+
id: cache-key
452+
shell: bash
453+
run: |
454+
echo "cache_key=jbang-$(date +%F)" >> $GITHUB_OUTPUT
455+
- name: Use cache
456+
uses: actions/cache@v4
457+
with:
458+
path: ~/.jbang
459+
key: ${{ steps.cache-key.outputs.cache_key }}
460+
restore-keys:
461+
jbang-
462+
- name: Setup JBang
463+
uses: jbangdev/setup-jbang@main
464+
465+
- name: Detect changed jablib classes
466+
id: changed-jablib-files
467+
uses: tj-actions/changed-files@v45
468+
with:
469+
files: |
470+
jablib/src/main/java/**/*.java
471+
472+
- name: Build ${{ matrix.launcher }} launcher including changed classes
473+
shell: bash
474+
# We always run, because changes in jabls, jabsrv also could cause JBang to fail
475+
run: |
476+
# We modify the JBang scripts directly to avoid issues with relative paths
477+
for f in ${{ steps.changed-jablib-files.outputs.all_changed_files }}; do
478+
echo "//SOURCES $f" >> ".jbang/${{ matrix.launcher }}.java"
479+
done
480+
jbang build --fresh ".jbang/${{ matrix.launcher }}.java"
481+
431482
codecoverage:
432483
if: false
433484
name: Code coverage

.jbang/JabLsLauncher.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,14 @@
1010

1111
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/BibtexTextDocumentService.java
1212
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/BibtexWorkspaceService.java
13-
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/LSPLauncher.java
14-
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/LSPServer.java
13+
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/controller/LanguageServerController.java
14+
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/ExtensionSettings.java
15+
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/LspClientHandler.java
16+
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/LspLauncher.java
17+
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspConsistencyCheck.java
18+
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspDiagnosticBuilder.java
19+
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspDiagnosticHandler.java
20+
//SOURCES ../jabls/src/main/java/org/jabref/languageserver/util/LspIntegrityCheck.java
1521

1622
// REPOS mavencentral,snapshots=https://central.sonatype.com/repository/maven-snapshots/
1723
// 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
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
---
2+
nav_order: 48
3+
parent: Decision Records
4+
---
5+
6+
# JBang script modification for testing in CI
7+
8+
## Context and Problem Statement
9+
10+
All JBang scripts on `main` should always compile.
11+
12+
Example:
13+
14+
```terminal
15+
gg.cmd https://github.com/JabRef/jabref/blob/main/.jbang/JabLsLauncher.java
16+
```
17+
18+
- JBang scripts link to `org.jabref:jablib:6.0-SNAPSHOT`.
19+
- 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`.
20+
- Code changes might change a) things inside the server project and b) things in JabRef's logic.
21+
22+
As a consequence, the JBang script might break.
23+
24+
Case a) can be detected when running the JBang check in the respective PR.
25+
Case b) can be detected when running the JBang check in the main branch, because `org.jabref:jablib:6.0-SNAPSHOT` is updated there.
26+
27+
We aim for checking JBang scripts in PRs and on `main`.
28+
29+
## Decision Drivers
30+
31+
- Fast detection of issues at all JBang files
32+
- Easy CI pipeline
33+
34+
## Considered Options
35+
36+
- Have JBang script have all changed classes of `jablib` included directly
37+
- Use jitpack
38+
-Temporary `-SNAPSHOT.jar`
39+
40+
## Decision Outcome
41+
42+
Chosen option: "Have JBang script have all changed classes of `jablib` included directly.", because comes out best (see below).
43+
44+
## Pros and Cons of the Options
45+
46+
### Have JBang script have all changed classes of `jablib` included directly
47+
48+
- `org.jabref:jablib:6.0-SNAPSHOT` provides non-modified classes
49+
- modified classes are included using JBang's `//SOURCES` directive.
50+
51+
[tj-actions/changed-files](https://github.com/marketplace/actions/changed-files) can be used.
52+
53+
- Good, because least effort
54+
55+
### Use jitpack
56+
57+
- Bad, because jitpack is unreliable
58+
59+
### Temporary `-SNAPSHOT.jar`
60+
61+
- Bad, because setting up a temporary maven repository is effort.

0 commit comments

Comments
 (0)