[CI] mingw-w64 workflow testing a fresh compiler

[straight-shoota]

Primitive specs test the behaviour of the compiler build. They need a fresh .build/crystal to succeed.

This doesn't work as expected in the x86_64-mingw-w64-test-compiler job because we never build the compiler there. If we change the compiler's behaviour, it'll fail (see #15186 (comment)).

The command in the workflow suggests that we incorrectly assume a) make primitives_spec to have .build/crystal.exe as a prerequisite and b) that .build/crystal.exe exists.
UPDATE: .build/crystal.exe is actually a prerequisite of primitives_spec. The -o flag just disabled building it (but the specs still run with the system compiler).

crystal/.github/workflows/mingw-w64.yml

Line 140 in 62c92a0

make -o .build/crystal.exe primitives_spec # we know the compiler is fresh; do not rebuild it here

I believe we probably should make both premises true:

• primitives_spec should require .build/crystal to exist as a very primitive measure to detect incorrect use (UPDATE: Maybe this is already fine. We used explicitly used the -o flag incorrectly).
• Copy the compiler artifact from the build job into the test-compiler job

For primitive specs it's essential that they run on a fresh compiler. But we usually run other tests on the fresh compiler as well to ensure it behaves correctly.

In other workflows (which typically use the bin/ci tool) the process is usually something like this:

# test std_spec on previous compiler
make std_spec clean

# build fresh compiler
make crystal

# run tests on fresh compiler
make primitives_spec std_spec compiler_spec
make samples
CRYSTAL_OPTS=--debug make samples

We should probably have a similar setup for mingw64 as well. For organizational purposes it would be nice to keep the split into separate jobs, but have the compiler test job depend on the build job.

I'm also wondering if we need to build shards everytime in the build job. We added that because the artifact is used for releases, but that should really only be necessary for release builds.

[ysbaddaden]

I believe we probably should make both premises true:

Sounds good.

I'm also wondering if we need to build shards everytime in the build job.

Agreed. CI should only build shards during a release.

[straight-shoota]

In win.yml we're also building a full release build with shards on every run.

I suppose in both workflows we can split the build into two jobs.

• First one only builds the compiler. This serves as a prerequsite for the test job.
• Second one builds shards and packages it with the compiler from the build job. This job only runs when we actually need a release package (tag, nightly schedule, workflow dispatch).

[straight-shoota]

Seems like the MinGW-W64 workflow is even more fundamentally broken.

It sets CRYSTAL_SPEC_COMPILER_BIN everywhere to the value of which crystal.exe. That's the bootstrap compiler. We want to test the freshly built compiler, not the bootstrap. This obviously doesn't work (see https://github.com/crystal-lang/crystal/actions/runs/13550872918/job/37874558782?pr=15186).

Removing these explicit env var assignments breaks even more things because now the environment can't find crystal:
https://github.com/crystal-lang/crystal/actions/runs/13554249031/job/37884922140?pr=15186
I presume that's the reason why this environment variable was used in the first place. But that still doesn't make it right.

[straight-shoota]

Oh that's wild.

$(pwd)\build\crystal.exe is at least the correct executable which was downloaded as build artifact.

But that location is still unexpected for a locally-built compiler and unknown to the wrapper script. Unless I'm missing something, running bin/crystal (which the make recipes do) will not actually use this fresh compiler. It'll use the system compiler. 🤦

EDIT: All right, I was missing something: We're prepending ./build to PATH, so the wrapper script will pick up the fresh compiler. We're not even installing a system compiler in the test jobs.
This is alll very convoluted, though. We should try to keep a simple, common setup without any extra hoops. The wrapper script expects the fresh compiler in .build/crystal, so that's where we should put it. Or, alternatively, set the CRYSTAL environment variable to point to an alternate location. But the local compiler shouldn't need to go into PATH.

---

In general, CRYSTAL_SPEC_COMPILER_BIN should only ever be necessary if you're doing something unconventional. For example if you want to test the same spec binary using different compiler configurations for its compiling tests.

In a regular development environment there should be no need to set CRYSTAL_SPEC_COMPILER_BIN. It's all supposed to work without config.
If it does not, that's an indicator that the setup is wrong.

[HertzDevil]

CRYSTAL_SPEC_COMPILER_BIN is required on MSVC when the spec executable is run individually after crystal build and not from crystal spec, because the default bin/crystal doesn't resolve to a valid Win32 process. That was the original purpose and most certainly holds on MinGW-w64 as well; an MSYS2 shell won't magically make LibC.CreateProcessW understand that it can run bin/crystal using sh.exe.

Though at this rate maybe we should just encode the Windows-specific defaults in the spec helpers directly?

[straight-shoota]

because the default bin/crystal doesn't resolve to a valid Win32 process.

The spec runner sets CRYSTAL_SPEC_COMPILER_BIN as the current compiler executable.
So bin/crystal should only be a fallback when the spec file is built and executed without crystal spec (which we do not do in CI).

But yes, we should fix this fallback to work on Windows.