ci: build and verify the Windows binary on every pull request - #206
Merged
Merged
Conversation
The previous attempt put the fix straight after `spc download` and it never ran, because `download` only fetches archives — extraction into source/ happens during `build`. So the check found nothing and failed the job: ::error::libpq.lib not found under source\postgresql-win\pgsql\lib which was the guard doing its job, on a directory that did not exist yet. `spc extract postgresql-win` runs first, giving somewhere to apply the fix. The later `build` does not undo it: SourceManager skips extraction when the source directory exists and its .spc-hash matches the archive hash, and that hash is over the downloaded archive, so adding a directory does not invalidate it. The rest is unchanged — junctions rather than copies, so the workaround costs nothing and disappears if SPC fixes the stripped path, and the build still fails loudly with a directory listing if libpq.lib is missing. Co-Authored-By: Claude <noreply@anthropic.com>
Windows had no per-PR coverage at all. The binary was only ever built by the release and dry-run workflows, which do not run on pull requests, so two Windows-specific fixes in a row shipped unverified: the missing NASM assembler, and the libpq headers SPC looks for one directory too deep. Both were only disproved by a release dry run twenty minutes later, and the second attempt at the libpq fix was wrong as well. The new job builds the binary, smoke-tests it, and checks its database drivers — which is exactly what went wrong both times. A missing NASM fails the build; a missing libpq is caught by the driver check, because --version passes whether or not a single driver made it in. That is how a driverless binary reached a release dry run in the first place. Deliberately narrower than the Linux job: GitHub does not run service containers on Windows runners, so there is no MySQL or Postgres to diff against and the live-database half stays Linux-only. Claiming parity would be worse than stating the gap. The driver check does not need a database — PDO reports a missing driver as "could not find driver" and an unreachable server as a connection error, so a closed port tells them apart. SPC_WINDOWS_LIBS is added to this workflow's env; it was defined only in release.yml, so the download command would have been built with an empty library list. Also stops the Dependabot auto-merge job failing a green pull request. On a branch that requires a review, GITHUB_TOKEN is refused: GraphQL: Pull request User is not authorized for this protected branch That is a repository policy decision rather than a problem with the update, so it is now reported as a notice and the pull request stays green. Co-Authored-By: Claude <noreply@anthropic.com>
…t code The new Windows job failed on its first run while reporting exactly what it was built to prove: postgresql driver present mysql driver present ##[error]Process completed with exit code 1. Both drivers are there — the NASM and libpq fixes work — but the step still failed. The check points each driver at a closed port and reads the error, so every dbdiff.exe call is *expected* to fail. GitHub's pwsh shell appends `exit $LASTEXITCODE`, so the step inherited that deliberate failure and reported a passing check as a failing job. The bash version never hit this because it uses `|| true`. An explicit `exit 0` after the loop, in all three workflows that carry this check. The failure paths above it still exit 1, so a genuinely missing driver still fails. release.yml and release-dry-run.yml had the same latent bug: it only stayed hidden there because the check had never yet reached the end — the Windows binary was genuinely missing its Postgres driver, so it exited 1 for the real reason first. Co-Authored-By: Claude <noreply@anthropic.com>
Splitting these was a mistake. The Windows job added here carries the libpq workaround inline so it can build at all, while the release workflows got it only from the other branch — so neither pull request was complete on its own, and merging them in the wrong order would have left the release workflows broken while CI looked fine. Folding them together means the job that builds a Windows binary on every pull request is testing the same fix the release uses.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Windows had no per-pull-request coverage. The binary was only ever built by the release workflows, which do not run on pull requests — so a Windows-specific regression could only be found by cutting a release.
This adds a
win32-x64job to the existing binary check.Coverage
--version/--helpWhy the driver check matters
--versionsucceeds whether or not a database driver was compiled in, so a binary can look healthy while being unable to connect to anything. The check points each driver at a closed port and reads the error: PDO reports a missing driver ascould not find driver, and a present driver as a connection failure — which distinguishes them without needing a database.This also fixes the Windows build so
pdo_pgsqlis linked correctly. The upstream builder looks for the PostgreSQL client libraries one directory below where the archive extracts them, and the resulting copy failures are warnings rather than errors — so the build completes and produces a binary with no PostgreSQL support. The libraries are now put where the builder expects them, and the build fails loudly, with a directory listing, if they are missing.Why it is narrower than the Linux job
GitHub does not run service containers on Windows runners, so there is no MySQL or Postgres to diff against and the live-database half stays Linux-only. Stating that is better than implying parity.
Also in this PR
dbdiff.execall in the driver check is expected to fail, and PowerShell propagates the last exit code, so a passing check reported a failing job. The failure paths still exit non-zero, so a genuinely missing driver is still caught.SPC_WINDOWS_LIBSis defined in this workflow. It was referenced by the download command but only set in the release workflow, so the library list would have been empty.🤖 Generated with Claude Code