fix: handle null libc version on Android/Termux#305
fix: handle null libc version on Android/Termux#305zerone0x wants to merge 1 commit intogoogleworkspace:mainfrom
Conversation
On Android/Termux with bionic libc, detect-libc returns null for the
libc version. The npm installer's binary.js crashes with
"Cannot read properties of null (reading 'split')" because it calls
.split(".") on the null return value without a null check.
This adds a null check for libcVersion in binary.js and falls back to
the static musl binary when the libc version cannot be detected. The
patched binary.js is applied to the npm package during the release
build via a new step in the release workflow.
Fixes googleworkspace#271
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the robustness of the npm installer by addressing a critical issue on Android/Termux systems where the libc version could not be reliably determined. The changes prevent application crashes by introducing a null-safe check for libc version detection and implementing a fallback mechanism to a static musl binary, ensuring broader compatibility and stability for users on diverse Linux-based platforms. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a crash that occurs on Android/Termux when the libc version cannot be determined. The fix involves adding a null check and falling back to a static musl binary, which is a sound approach. I have provided a few comments on the newly added npm/binary.js file to enhance its robustness and reliability.
| } = require("./package.json"); | ||
|
|
||
| // FIXME: implement NPM installer handling of fallback download URLs | ||
| const artifactDownloadUrl = artifactDownloadUrls[0]; |
There was a problem hiding this comment.
The installer only uses the first URL from artifactDownloadUrls, which makes it vulnerable to failures if that specific URL is down. The FIXME comment acknowledges this. To improve reliability, the installer should be updated to try all available fallback URLs in artifactDownloadUrls until a binary is successfully downloaded.
| } | ||
|
|
||
| if (rawOsType === "Linux") { | ||
| if (libc.familySync() == "musl") { |
There was a problem hiding this comment.
It's best practice to use the strict equality operator (===) for comparisons to avoid unexpected type coercion. This prevents potential bugs and makes the code's intent clearer.
| if (libc.familySync() == "musl") { | |
| if (libc.familySync() === "musl") { |
| let splitLibcVersion = libcVersion.split("."); | ||
| let libcMajorVersion = splitLibcVersion[0]; | ||
| let libcMinorVersion = splitLibcVersion[1]; | ||
| if ( | ||
| libcMajorVersion != builderGlibcMajorVersion || | ||
| libcMinorVersion < builderGlibcMinorVersion |
There was a problem hiding this comment.
The version components obtained from split('.') are strings. Comparing them directly with numbers relies on implicit type coercion, which can be fragile and lead to bugs (e.g., lexicographical string comparison like '10' < '2' is true). It's safer and clearer to explicitly parse the version components into integers before performing numerical comparisons. This makes the logic more robust.
const [majorStr, minorStr] = libcVersion.split(".");
const libcMajorVersion = parseInt(majorStr, 10);
const libcMinorVersion = parseInt(minorStr, 10);
if (
libcMajorVersion !== builderGlibcMajorVersion ||
libcMinorVersion < builderGlibcMinorVersion
)2 participants
Summary
detect-libcreturnsnullfor the libc version (bionic libc), causingbinary.jsto throwTypeError: Cannot read properties of null (reading 'split')libcVersionin the npm installer'sbinary.js, falling back to the static musl binary when the libc version cannot be detectedDetails
On Android/Termux with bionic libc:
libc.familySync()does not return"musl"libc.isNonGlibcLinuxSync()returnsfalselibc.versionSync()returnsnull.split(".")onnullcrashes the CLIThe fix adds a null guard before the
.split()call. WhenlibcVersionisnull, the installer logs a warning and falls back to the static musl binary, matching the existing behavior for other incompatible libc scenarios.Since
binary.jsis generated by cargo-dist, the patched version is stored innpm/binary.jsand applied to the npm tarball during thebuild-global-artifactsstep of the release workflow.Fixes #271
Test plan
binary.jspasses Node.js syntax check (node --check)gwsno longer crashes with null libc version🤖 Generated with Claude Code