Fix remove trailing whitespace from CRYSTAL definition #15131
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
approved these changes
Oct 28, 2024
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.
#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 theinstallrecpie resolves to the path.build/crystalwhich doesn't exist. It looks like$(EXE)resolves to a single whitespace, but the error is actually in the definition ofCRYSTALwhich contains a trailing whitespace.This is only an issue in the
installrecipe 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.CRYSTALdescribes the base compiler, not the output path.This patch partially reverts #15123 and reintroduces
$(CRYSTAL_BIN), but it's now based on$(EXE).