Skip to content

Commit aa3a824

Browse files
Jamesclaude
authored andcommitted
Verify the .bat scripts on Windows in CI, and make them actually work
The .bat scripts have been "repaired but unverified" since the package rename. Running them on a windows-latest runner turned out to require fixing two things first, neither of which was the path staleness already dealt with. 1. lib\ shadows the current parser. These scripts predate the parser being resolved from Maven. lib\ now holds only OLD parser jars that other things still pin (gsqlparser-3.1.1.0, gudusoft.gsqlparser-3.0.2.5), and setenv.bat put lib\ first on the classpath, so those win over anything newer: javac -cp ".;build;lib/*;external_lib/*" ... checksyntax.java -> error: cannot find symbol: class EOBTenantMode EOBTenantMode is OceanBase tenant-mode support, far newer than a 3.x jar. Same command with external_lib\ ahead of lib\ compiles and runs. setenv.bat now orders it that way, with the reason written down. The parser goes in external_lib\, which setenv.bat has always had on the classpath and which the repository never actually created. It is fetched, not vendored: mvn dependency:copy -Dartifact=com.gudusoft:gsqlparser:4.1.6 -DoutputDirectory=external_lib external_lib/ and /build/ are gitignored. 2. setenv.bat hardcoded JAVA_HOME to C:\Program Files\Java\jdk1.8.0_201, which overwrote whatever the runner had set. It now keeps an existing JAVA_HOME and only falls back to that path when none is set, so nobody has to edit this file on a machine that already knows where its JDK is. The new windows-bat job then runs the real thing: compile_checksyntax.bat, assert checksyntax.class appears, run_checksyntax.bat, assert it reports "syntax errors: 0". Each script ends with `pause`, which would block forever on a runner with no keyboard, so their stdin is fed from NUL. This covers one demo, not all 45, and that is a deliberate stopping point: the scripts are generated from one template and go stale as a set, which is exactly what happened when the demos moved directory. One canary catches that. Every step above was reproduced locally before being written into the workflow, including both failing classpath orders. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012qPRpoD8exYRrUmbfXXWXj
1 parent feb1290 commit aa3a824

4 files changed

Lines changed: 122 additions & 16 deletions

File tree

.github/workflows/build.yml

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,62 @@ jobs:
8989
-Dexec.args="/f q.sql /t oracle" -Dexec.classpathScope=compile)
9090
echo "$out"
9191
grep -q "syntax errors: 0" <<<"$out"
92+
93+
# The .bat scripts are the original Windows, no-Maven workflow: edit
94+
# setenv\setenv.bat, cd into a demo folder, run compile_<demo>.bat then
95+
# run_<demo>.bat. They had been stale for years -- compiling
96+
# src\main\java\demos\<demo>\ and cd-ing up five levels, both correct only
97+
# before the demos moved under gudusoft\gsqlparser\demos\ -- and nothing ever
98+
# noticed, because nothing ran them. This job runs them.
99+
windows-bat:
100+
runs-on: windows-latest
101+
102+
steps:
103+
- name: Checkout
104+
uses: actions/checkout@v4
105+
106+
# The .bat scripts want a JDK 8 era toolchain, and setenv.bat now keeps
107+
# whatever JAVA_HOME it is given rather than hardcoding one.
108+
- name: Set up JDK 8
109+
uses: actions/setup-java@v4
110+
with:
111+
distribution: temurin
112+
java-version: "8"
113+
cache: maven
114+
115+
# These scripts predate the parser being resolved from Maven. lib\ only
116+
# carries old parser jars now, so on its own compile_<demo>.bat fails on
117+
# symbols those jars predate. Drop the current parser into external_lib\,
118+
# which setenv.bat puts ahead of lib\ on the classpath for exactly this
119+
# reason. Not vendored: fetched here, and gitignored.
120+
- name: Fetch the parser into external_lib
121+
shell: bash
122+
run: |
123+
set -euo pipefail
124+
ver=$(mvn -q help:evaluate -Dexpression=gsp.core.version -DforceStdout)
125+
echo "parser version: $ver"
126+
mvn -q dependency:copy \
127+
-Dartifact=com.gudusoft:gsqlparser:"$ver" \
128+
-DoutputDirectory=external_lib
129+
ls external_lib
130+
131+
# `pause` at the end of each script would block forever on a runner with
132+
# no keyboard, so stdin is fed from NUL.
133+
- name: compile_checksyntax.bat then run_checksyntax.bat
134+
shell: cmd
135+
run: |
136+
cd src\main\java\gudusoft\gsqlparser\demos\checksyntax
137+
call compile_checksyntax.bat < NUL
138+
if not exist "%GITHUB_WORKSPACE%\build\gudusoft\gsqlparser\demos\checksyntax\checksyntax.class" (
139+
echo ::error::compile_checksyntax.bat did not produce checksyntax.class
140+
exit /b 1
141+
)
142+
cd /d "%GITHUB_WORKSPACE%"
143+
echo SELECT a.id FROM ta a; > q.sql
144+
cd src\main\java\gudusoft\gsqlparser\demos\checksyntax
145+
call run_checksyntax.bat /f "%GITHUB_WORKSPACE%\q.sql" /t oracle < NUL > "%GITHUB_WORKSPACE%\out.txt" 2>&1
146+
type "%GITHUB_WORKSPACE%\out.txt"
147+
findstr /c:"syntax errors: 0" "%GITHUB_WORKSPACE%\out.txt" >NUL || (
148+
echo ::error::run_checksyntax.bat did not report "syntax errors: 0"
149+
exit /b 1
150+
)

.gitignore

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,10 @@ CLAUDE.md
7171
# generated by src/main/java/.../dlineage/buildJar.sh
7272
src/main/java/gudusoft/gsqlparser/demos/dlineage/class/
7373
src/main/java/gudusoft/gsqlparser/demos/dlineage/data_flow_analyzer.jar
74+
75+
# populated by the .bat workflow, not vendored:
76+
# mvn dependency:copy -Dartifact=com.gudusoft:gsqlparser:<ver> -DoutputDirectory=external_lib
77+
external_lib/
78+
79+
# javac output dir used by compile_<demo>.bat
80+
/build/

README.md

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -411,19 +411,44 @@ recompile.
411411
## The .bat scripts (Windows, no Maven)
412412

413413
Each demo directory also ships `compile_<demo>.bat` and `run_<demo>.bat`, with
414-
`setenv/setenv.bat` holding `JAVA_HOME`. The original workflow was:
414+
`setenv/setenv.bat` holding the shared environment. The workflow:
415415

416-
1. edit `setenv/setenv.bat` and set `JAVA_HOME` to your JDK
417-
2. `cd` into a demo directory, for example `src/main/java/gudusoft/gsqlparser/demos/checksyntax`
416+
1. **Put the parser in `external_lib/`.** From the repository root:
417+
```
418+
mvn dependency:copy -Dartifact=com.gudusoft:gsqlparser:4.1.6 -DoutputDirectory=external_lib
419+
```
420+
2. `cd` into a demo directory, e.g. `src\main\java\gudusoft\gsqlparser\demos\checksyntax`
418421
3. run `compile_checksyntax.bat`, then `run_checksyntax.bat`
419422

420-
These were stale for a long time: they compiled `src\main\java\demos\<demo>\`
421-
and `cd`-ed up five levels, both correct only before the demos moved under
422-
`src/main/java/gudusoft/gsqlparser/demos/`. The package rename had to rewrite
423-
their class names anyway, so their paths and `cd` depths were corrected at the
424-
same time — 7 levels for most, 8 for the two nested under another demo. They
425-
have not been run on Windows since, so treat them as repaired-but-unverified;
426-
Maven remains the tested path.
423+
You no longer have to edit `setenv.bat` for step 1 of the old instructions:
424+
it keeps whatever `JAVA_HOME` is already set and only falls back to a fixed path
425+
when there is none.
426+
427+
**`external_lib/` is not optional, and it must come before `lib/`.** These
428+
scripts predate the parser being resolved from Maven, and `lib/` now holds only
429+
*old* parser jars that other things still pin (`gsqlparser-3.1.1.0`,
430+
`gudusoft.gsqlparser-3.0.2.5`). With `lib/` first on the classpath those shadow
431+
the current parser and the demos fail to compile on symbols the old jars predate
432+
`checksyntax` dies on `EOBTenantMode`. `setenv.bat` therefore puts
433+
`external_lib\*` ahead of `lib\*`, and `external_lib/` is gitignored so the
434+
parser is fetched rather than vendored.
435+
436+
### These are now tested on Windows
437+
438+
They had been stale for years — compiling `src\main\java\demos\<demo>\` and
439+
`cd`-ing up five levels, both correct only before the demos moved under
440+
`gudusoft/gsqlparser/demos/`. Nothing noticed, because nothing ran them.
441+
442+
The `windows-bat` job in `.github/workflows/build.yml` now does, on
443+
`windows-latest`: it fetches the parser into `external_lib/`, runs
444+
`compile_checksyntax.bat`, asserts the `.class` file appears, runs
445+
`run_checksyntax.bat`, and asserts it reports `syntax errors: 0`. Each script
446+
ends with `pause`, so CI feeds their stdin from `NUL` to keep them from blocking
447+
on a runner with no keyboard.
448+
449+
That covers one demo, not all 45. It is enough to catch the failure mode that
450+
actually happened here — the whole family going stale together after a
451+
directory move — since they are generated from one template and break as a set.
427452

428453

429454
## Building the dlineage demo on its own

setenv/setenv.bat

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
REM # This script/batch file sets all the envrironment variables required by other batch files and
2-
REM # scripts inorder to run the demos.
1+
REM # This script/batch file sets all the envrironment variables required by other batch files and
2+
REM # scripts inorder to run the demos.
33
REM # This script/batch file will be invoked by every other script in the demos directory so that
44
REM # envrionment variables are properly set before running an application. This ensures that users
55
REM # have to change the envrironment settings in only one location.
@@ -8,9 +8,12 @@ REM # have to change the envrironment settings in only one location.
88
REM # SET PATH FOR Native Libraries
99
set PATH=%PATH%;lib\;external_lib\
1010

11-
REM # set the Java home directory
12-
rem set JAVA_HOME=C:\Program Files\Java\jdk1.7.0_80
13-
set JAVA_HOME=C:\Program Files\Java\jdk1.8.0_201
11+
REM # set the Java home directory.
12+
REM # If JAVA_HOME is already set -- by CI, or by a developer who configured it
13+
REM # once -- keep it. Only fall back to a fixed path when it is not set, so this
14+
REM # file does not have to be edited on a machine that already knows where its
15+
REM # JDK is.
16+
if not defined JAVA_HOME set JAVA_HOME=C:\Program Files\Java\jdk1.8.0_201
1417

1518

1619
set JAVA_CMD="%JAVA_HOME%\bin\java.exe"
@@ -22,4 +25,16 @@ REM #Set the home directory of the GSP library
2225
set gspDemoHome=.
2326

2427
REM # set classpath to the GSP library Jar files and the database JDBC drivers.
25-
set CLASSPATH=.;%gspDemoHome%\build;%gspDemoHome%\lib\*;%gspDemoHome%\external_lib\*
28+
REM #
29+
REM # external_lib comes BEFORE lib, and that order matters. lib\ still holds old
30+
REM # parser jars (gsqlparser-3.1.1.0, gudusoft.gsqlparser-3.0.2.5) that other
31+
REM # things depend on, and with lib\ first those shadow the current parser: the
32+
REM # demos then fail to compile on symbols the old jars predate, for example
33+
REM # EOBTenantMode in checksyntax. Put the parser you actually want to build
34+
REM # against in external_lib\ and it wins.
35+
REM #
36+
REM # To fetch the current parser into external_lib\, from the repository root:
37+
REM #
38+
REM # mvn dependency:copy -Dartifact=com.gudusoft:gsqlparser:4.1.6 -DoutputDirectory=external_lib
39+
REM #
40+
set CLASSPATH=.;%gspDemoHome%\build;%gspDemoHome%\external_lib\*;%gspDemoHome%\lib\*

0 commit comments

Comments
 (0)