Jens-G opened a new pull request, #3800:
URL: https://github.com/apache/thrift/pull/3800

   `lib/dart/test` holds nine test files — transport, protocol, serializer, 
`t_application_error` — and nothing in the build system ever ran them. 
`lib/dart/Makefile.am` had `check-local: all` with an empty recipe, and 
`all-local` is `dart pub get`, so `make check` resolved dependencies and 
reported success without executing a single test. No CI job built the binding 
either: both `build.yml` and `sca.yml` pass `--without-dart`.
   
   Wiring up a job meant fixing two things underneath it first.
   
   ### configure could not detect Dart at all
   
   Not on any SDK the library supports. It probed for `pub` as a standalone 
program:
   
   ```
   AC_PATH_PROG([DART], [dart])
   AC_PATH_PROG([DARTPUB], [pub])
   ```
   
   `pub` was deprecated in 2.15 and **removed in Dart 3** — `dart pub` is the 
only spelling left. On anything newer than the 2.7.2 the images pinned, 
`AC_PATH_PROG` found no `pub`, `have_dart` stayed `no`, and `lib/dart` was 
skipped:
   
   ```
   checking for dart... /usr/lib/dart/bin/dart
   checking for pub... no
   Building Dart Library ........ : no
   ```
   
   So probe the subcommand instead, and let configure hand `DART` and `DARTPUB` 
to the two `Makefile.am` files that had been hardcoding `dart pub` themselves.
   
   ### The SDK pin, and the floor it was measured against
   
   The images pinned `DART_VERSION=2.7.2-1`, older than the `">=2.12.0"` the 
library's own pubspec asked for — `dart pub get` could not have resolved in the 
project's own images.
   
   That floor was itself wrong. The `http` 1.x dependency does not resolve 
below 3.2.0, so `pub get` already fails on 3.1, with a message about `http` 
rather than about thrift:
   
   ```
   Because thrift depends on http >=1.1.1 which requires SDK version >=3.2.0 
<4.0.0,
   version solving failed.
   ```
   
   Both now say 3.2.0, as do the two dependent pubspecs under `test/dart`. The 
images pin `3.13.3-1` — verified to install on focal, jammy and noble alike.
   
   ### With that, the tests run
   
   `check-local` runs `dart test`: **87 tests**.
   
   `test/dart`'s recursion-depth test (THRIFT-6056) was excluded from `check` 
with a comment saying it needs a null-safe SDK. That is no longer a distinction 
worth drawing — the floor is 3.2.0 — so it joins `check` as well: **27 more 
tests** that had never run.
   
   ### The job
   
   Matrix is the declared floor and the pinned image version, 3.2.0 and 3.13.3. 
`setup-dart` has to come before configure. `LANGUAGES.md` now says the same 
pair instead of 2.0.0 to 2.4.0.
   
   The configure step checks that Dart was actually found before the test steps 
run. That is not defensive habit: while writing this I had a job go green end 
to end against an unpatched configure that had disabled Dart — precisely the 
failure this ticket is about — and it is worth making impossible rather than 
unlikely.
   
   ### Verification
   
   End to end on clean `ubuntu:24.04` containers with the job's own steps:
   
   ```
   Dart 3.2.0    lib/dart  87 tests passed    test/dart  27 tests passed
   Dart 3.13.3   lib/dart  87 tests passed    test/dart  27 tests passed
   ```
   
   With the `dart` binary removed the job fails at configure instead of passing.
   
   ### Not in this PR
   
   - `tutorial/dart`'s three pubspecs still declare `>=2.12.0`. Harmless — they 
depend on `lib/dart` by path, so pub intersects the constraints and the 
effective floor is already 3.2.0 — and no CI job builds them.
   - Adding `dart` to the `cross-test` matrix.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to