Skip to content
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

Add support for official libc formatting #22

Merged
merged 5 commits into from
Mar 19, 2024

Conversation

dstaley
Copy link
Contributor

@dstaley dstaley commented Mar 16, 2024

This PR updates the ldd content checks for family and version to account for distributions that are more closely aligned with the official formatting of ldd, which does not contain the string "GLIBC".

This allows faster checks on distros like Amazon Linux, openSUSE, Fedora, RHEL, Arch, and more without falling back to process.report.getReport().

@lovell
Copy link
Owner

lovell commented Mar 17, 2024

Thank you for the PR Dylan.

Could checking for "libc" on its own, which feels rather generic, potentially lead to false positives? What does Bionic libc report? What if musl starts using the string "MUSL libc"?

Would a check for the explicit string "GNU libc" or similar be more robust?

@dstaley
Copy link
Contributor Author

dstaley commented Mar 17, 2024

Great question! I definitely see how being a tad bit more generic could potentially lead to false positives (although I think, at present, those situations are theoretical rather than actually possible given the current file content of ldd for musl and bionic).

GNU libc isn't present, but GNU C Library or even This file is part of the GNU C Library could work, with the latter being highly unlikely to appear in any implementation that isn't at least derived from libc.

@lovell
Copy link
Owner

lovell commented Mar 17, 2024

Thanks, if searching for "GNU C Library" improves things then that seems like a good option here.

$ docker run --rm -it amazonlinux:2 grep GNU /usr/bin/ldd
# This file is part of the GNU C Library.
# The GNU C Library is free software; you can redistribute it and/or
# modify it under the terms of the GNU Lesser General Public
# The GNU C Library is distributed in the hope that it will be useful,
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
# You should have received a copy of the GNU Lesser General Public
# License along with the GNU C Library; if not, see
    echo 'ldd (GNU libc) 2.26'

Copy link
Owner

@lovell lovell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the updates, I've left one comment inline about the failing musl-based Void test. (The failing glibc-based Void test is now fixed on the main branch so a rebase will get this passing.)

lib/detect-libc.js Outdated Show resolved Hide resolved
@dstaley dstaley requested a review from lovell March 19, 2024 00:36
test/unit.js Outdated Show resolved Hide resolved
test/unit.js Outdated Show resolved Hide resolved
@lovell lovell merged commit 2642d96 into lovell:main Mar 19, 2024
34 checks passed
@lovell
Copy link
Owner

lovell commented Mar 19, 2024

Thank you very much Dylan.

@lovell
Copy link
Owner

lovell commented Mar 19, 2024

This change was included in v2.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants