Skip to content

Add compiler path to $PATH and $CRYSTAL_EXEC_PATH for subcommands#15186

Merged
straight-shoota merged 21 commits intocrystal-lang:masterfrom
straight-shoota:feat/compiler-subcommand-path
Mar 25, 2025
Merged

Add compiler path to $PATH and $CRYSTAL_EXEC_PATH for subcommands#15186
straight-shoota merged 21 commits intocrystal-lang:masterfrom
straight-shoota:feat/compiler-subcommand-path

Conversation

@straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Nov 12, 2024

Resolves #15145

This patch contains a breaking change of dropping the environment variable $CRYSTAL (introduced in #14953 1.14.0) in favour of the newly added ones.

Quoting from #15145:

I think it should be acceptable to replace the $CRYSTAL variable. We could consider keeping it along, but if we do that, we'll need to do that for a long time. I think it's better to drop it in order to avoid new subcommands ever depend on it.

@straight-shoota straight-shoota added kind:feature breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. topic:compiler:cli labels Nov 12, 2024
@straight-shoota straight-shoota self-assigned this Nov 12, 2024
@straight-shoota straight-shoota force-pushed the feat/compiler-subcommand-path branch from 13afe7e to c8d8a61 Compare November 13, 2024 21:00
@straight-shoota
Copy link
Member Author

I dropped the test code for exit_code because it was failing on Windows. It was unrelated to this change anyway. I'll investigate further and add it in a follow-up.

@bcardiff
Copy link
Member

An upside of the $CRYSTAL env variable is that if the compiler is not crystal but something else crystal-1.0, crystal-1.14, crystal-2 then the external commands still works.

I don't mind prepending to the $PATH and having a $CRYSTAL_EXEC_PATH, but I don't see how we enable binaries named differently.

$CRYSTAL is also used in shards as a convention, so I thought it was convenient to keep the same pattern. (which happens to be the same as in cabal with $CABAL)

@bcardiff
Copy link
Member

If no one thinks that supporting different names of the compiler binary is needed then 👍

@RX14
Copy link
Member

RX14 commented Nov 16, 2024

The interface between the compiler and extension scripts is likely to be a long-lived interface, so I think it's worth spending the time to get it working when the compiler isn't named just crystal. This PR is an improvement already, but I think using $CRYSTAL is a good thing to encourage.

@straight-shoota straight-shoota changed the title Add compiler path to $PATH for subcommands Add compiler path to $PATH and $CRYSTAL_EXEC_PATH for subcommands Feb 24, 2025
@straight-shoota
Copy link
Member Author

CI on mingw-w64 is failing due to #15516.
This is the first time we're adding a new test for a new behaviour of the compiler executable, so it's the first time we notice that the CI workflow doesn't work correctly.

@straight-shoota
Copy link
Member Author

straight-shoota commented Feb 26, 2025

The MinGW-W64 CI workflow appears to have even more issues: #15516 (comment)

I might try to skip this spec then so we can move forward here.

@straight-shoota straight-shoota added this to the 1.16.0 milestone Feb 28, 2025
@straight-shoota straight-shoota merged commit c5598e8 into crystal-lang:master Mar 25, 2025
40 of 41 checks passed
@straight-shoota straight-shoota deleted the feat/compiler-subcommand-path branch March 25, 2025 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change May have breaking effect in some edge cases. Mostly negligible, but still noteworthy. kind:feature topic:compiler:cli

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Propagating the target compiler to subcommands

4 participants