Support .exe file extension in Makefile on MSYS2
#15123
Merged
Conversation
This file contains 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
HertzDevil
added
kind:feature
topic:infrastructure
kind:specs
platform:windows-gnu
ysbaddaden
approved these changes
Oct 24, 2024
straight-shoota
approved these changes
Oct 24, 2024
|
This patch broke the Makefile due to an unrelated error. But this failure also showed that replacing Fix is in #15123 |
straight-shoota
added a commit
that referenced
this pull request
Oct 28, 2024
#15123 broke the Makefile (and in turn CI: https://app.circleci.com/pipelines/github/crystal-lang/crystal/16310/workflows/f2194e36-a31e-4df1-87ab-64fa2ced45e2/jobs/86740) Since this commit, `"$(O)/$(CRYSTAL)$(EXE)"` in the `install` recpie resolves to the path `.build/crystal ` which doesn't exist. It looks like `$(EXE)` resolves to a single whitespace, but the error is actually in the definition of `CRYSTAL` which contains a trailing whitespace. This is only an issue in the `install` recipe because it's the only place where we put the path in quotes. So it would be simple to fix this by removing the quotes. The introduction of `$(EXE)` replaced `$(CRYSTAL_BIN)` with `$(CRYSTAL)$(EXE)`. But this is wrong. `CRYSTAL` describes the base compiler, not the output path. This patch partially reverts #15123 and reintroduces `$(CRYSTAL_BIN)`, but it's now based on `$(EXE)`.
CTC97
pushed a commit
to CTC97/crystal
that referenced
this pull request
Nov 9, 2024
…15131) crystal-lang#15123 broke the Makefile (and in turn CI: https://app.circleci.com/pipelines/github/crystal-lang/crystal/16310/workflows/f2194e36-a31e-4df1-87ab-64fa2ced45e2/jobs/86740) Since this commit, `"$(O)/$(CRYSTAL)$(EXE)"` in the `install` recpie resolves to the path `.build/crystal ` which doesn't exist. It looks like `$(EXE)` resolves to a single whitespace, but the error is actually in the definition of `CRYSTAL` which contains a trailing whitespace. This is only an issue in the `install` recipe because it's the only place where we put the path in quotes. So it would be simple to fix this by removing the quotes. The introduction of `$(EXE)` replaced `$(CRYSTAL_BIN)` with `$(CRYSTAL)$(EXE)`. But this is wrong. `CRYSTAL` describes the base compiler, not the output path. This patch partially reverts crystal-lang#15123 and reintroduces `$(CRYSTAL_BIN)`, but it's now based on `$(EXE)`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
.build/crystalalready adds the.exefile extension inMakefilewhen run on MSYS2. This PR does the same to the spec executables so that runningmake std_spectwice will only build the executable once.