This repository has been archived by the owner on Jul 24, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add a platform variant for installing a musl binary #1836
Merged
Merged
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
fd0238a
Add a platform variant for installing a musl binary
lox 85193ea
Don't use glibc variant, default to linux
lox 5773e8d
Add missing index check for indexOf
lox a3e3e6d
Fix linting errors
lox 3cf4232
Drop glibc variant
lox 83e3095
More linting fixes: == -> ===
lox 78f4601
Handle readFileSync exceptions and 0.x Buffer quirks
lox b0a63a0
Only use .toString() when needed for speed
lox 7e32a94
Fix linting errors
lox 4647e02
fix getPlatformVariant, getHumanPlatform and getBinaryPath
df1803d
fix extra / argument
5892a85
Merge pull request #3 from gmaliar/add-musl-platform-variant
lox 73199fa
Cleanup
xzyfer 209b1a6
Always toString
xzyfer d38985a
Delay tostring
xzyfer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -375,10 +375,14 @@ function getPlatformVariant() { | |
if (process.platform !== 'linux') { | ||
return ''; | ||
} | ||
if (fs.readFileSync(process.execPath).indexOf('libc.musl-x86_64.so.1') !== -1) { | ||
return 'musl'; | ||
} | ||
return ''; | ||
var contents = ''; | ||
try { | ||
contents = fs.readFileSync('foo.bar').toString(); | ||
if (contents.indexOf('libc.musl-x86_64.so.1') !== -1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ping @ncopa can you confirm it's ok to cover all Alpine releases or should we look for a wider pattern? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you asking whether this test will work for all Alpine releases? If so, yes. |
||
return 'musl'; | ||
} | ||
} catch (err) { } | ||
return '' | ||
} | ||
|
||
module.exports.hasBinary = hasBinary; | ||
|
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.
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.
sorry I sent comment while you are updating this.
toString is slow on where Buffer.indexOf is available. so this would be longer version.
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.
Starts to get a bit micro-optimization-y for my tastes, but sure.