-
Notifications
You must be signed in to change notification settings - Fork 37
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
Fix line-ending related errors on MacOS (ARM64) #482
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small initial nits for you to consider while you're sorting out the CI
Dismissing my review as I have now acted as an author
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #482 +/- ##
===========================================
+ Coverage 90.01% 90.85% +0.83%
===========================================
Files 60 60
Lines 3826 3826
===========================================
+ Hits 3444 3476 +32
+ Misses 382 350 -32 |
Adds MacOS/M1/ARM64 to test matrix as described by https://github.blog/changelog/2024-01-30-github-actions-introducing-the-new-m1-macos-runner-available-to-open-source/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM pending tests, thanks for coping with so many different options/variants with such a clean code.
For some reason, cloning Redis on Apple Silicon results in files within some of the Redis build scripts that have Windows-style line endings. This leads to errors because the interpreter for these scripts cannot be parsed correctly (e.g
/bin/sh^M
). To solve this, we now modify thegit clone
for both Redis and RedisAI to setcore.autoclrf=true
. This should only be done for this platform; on Linux and MacOS (x64), setting this to true ends up with the wrong line endings.