Miscellaneous Windows-related fixes in the compiler #9054
Merged
Conversation
This file contains hidden or 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
11 tasks
* Temporary file needs .exe extension to be executable * Replacing '/' is not enough to sanitize a filename, use Path splitting and replace the drive ':' manually * Split PATH by the correct delimiter * Don't try to fork (n_threads=1) * Don't try to set a directory's mtime on Windows, it errors * Don't rely on an external `date` command to get current date, use macro run instead * Remove custom abs-path code that is already handled by expand_path * Correct string interpolation in ECR * Allow overriding `llvm-config --targets-built` because the host LLVM doesn't necessarily match target LLVM * Skip signal code Unrelated: handle non-exit non-signal exit statuses correctly on POSIX.
Closed
RX14
reviewed
Apr 13, 2020
Member
Author
|
This PR is ready, it's not waiting on any macro additions. |
…se macro run instead"
straight-shoota
approved these changes
Apr 15, 2020
Member
straight-shoota
left a comment
straight-shoota
left a comment
There was a problem hiding this comment.
Yes, I'd prefer to keep the change to SOURCE_DATE_EPOCH out of this. As long as the env var is set, the command won't be executed, so there's nothing stopping this to work on windows.
RX14
approved these changes
Apr 16, 2020
Member
|
Thank you @oprypin |
straight-shoota
added
kind:bug
Sija
reviewed
Apr 16, 2020
carlhoerberg
pushed a commit
to carlhoerberg/crystal
that referenced
this pull request
Apr 29, 2020
* Temporary file needs .exe extension to be executable * Replacing '/' is not enough to sanitize a filename, use Path splitting and replace the drive ':' manually * Split PATH by the correct delimiter * Don't try to fork (n_threads=1) * Don't try to set a directory's mtime on Windows, it errors * Remove custom abs-path code that is already handled by expand_path * Correct string interpolation in ECR * Allow overriding `llvm-config --targets-built` because the host LLVM doesn't necessarily match target LLVM * Skip signal code Unrelated: handle non-exit non-signal exit statuses correctly on POSIX.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Don't rely on an externaldatecommand to get current date, use macro run insteadllvm-config --targets-builtbecause the host LLVM doesn't necessarily match target LLVMUnrelated: handle non-exit non-signal exit statuses correctly on POSIX.