Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow compiler executable under test to be overridden #11457

Merged
merged 2 commits into from
Nov 29, 2021

Conversation

HertzDevil
Copy link
Contributor

This PR adds an environment variable SPEC_COMPILER which, when set, overrides the Crystal compiler executable being used in specs.

This is mainly relevant for local development on Windows. Unlike on CI, bin\crystal.exe relative to the repository root is normally not expected to exist, because the local compiler is supposed to go into .build instead; also unlike on Unix, bin\crystal must resolve to an .exe, never some other script like bin\crystal.bat. With this PR, any installation of Crystal or wrapper script can be used in place of bin/crystal.

crystal spec automatically sets this environment variable to the compiler process itself if the variable is absent, before building the specs (not just before running, so that SPEC_COMPILER would always be available even in macros). If the specs are built and then run separately, like on CI, then SPEC_COMPILER should also be set beforehand (our workflows don't need this).

@straight-shoota
Copy link
Member

Sounds good! I think the env var name should be CRYSTAL_SPEC_COMPILER, though.

@oprypin oprypin self-requested a review November 16, 2021 19:26
@HertzDevil
Copy link
Contributor Author

CRYSTAL_SPEC_COMPILER_FLAGS and CRYSTAL_SPEC_COMPILER_THREADS are already used by the bin/check-compiler-flag, but these are only effective for the fresh compiler class required by the specs, not any existing Crystal executable, so there would be some potential confusion.

@HertzDevil
Copy link
Contributor Author

CRYSTAL_SPEC_COMPILER_EXECUTABLE might be better.

@Sija
Copy link
Contributor

Sija commented Nov 16, 2021

Perhaps a bit shorter CRYSTAL_SPEC_COMPILER_BIN?

@@ -13,7 +13,7 @@ input = ARGV[0]
bin = File.join(tmp_build_dir, "debug_test_case")
debugger_script = File.join(tmp_build_dir, "./debugger.script")

`#{repo_base_dir}/bin/crystal build --debug #{input} -o #{bin}`
Process.run(ENV["CRYSTAL_SPEC_COMPILER_BIN"]? || "#{repo_base_dir}/bin/crystal", ["build", "--debug", input, "-o", bin])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was already problematic before, so feel free to ignore, but we now have a good opportunity to check that this command actually succeeded.

@straight-shoota straight-shoota added this to the 1.3.0 milestone Nov 26, 2021
@straight-shoota straight-shoota merged commit 1f4d4e8 into crystal-lang:master Nov 29, 2021
@HertzDevil HertzDevil deleted the spec/spec-compiler branch November 30, 2021 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants