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

Plans for version 2 #14

Closed
8 tasks done
lovell opened this issue Dec 28, 2021 · 5 comments
Closed
8 tasks done

Plans for version 2 #14

lovell opened this issue Dec 28, 2021 · 5 comments

Comments

@lovell
Copy link
Owner

lovell commented Dec 28, 2021

This module is downloaded about a million times every day (!), and its logic is perhaps run hundreds of times every second.

Making changes to something this highly depended upon is a little frightening, but if we can reduce the time/energy cost then I think it will be worthwhile.

The logic hasn't changed in over four years and (rather amazingly) it all still works, but the ecosystem around it has improved so it's time for this module to do likewise.

Goals:

  • Expose functionality via both async and sync functions
  • Reduce require-time CPU and I/O cost, defer as much logic as possible until functions are called
  • Only spawn a child process where absolutely necessary, and consolidate into a single call
  • The requirements of both @mapbox/node-pre-gyp and prebuild become first-class concerns (perhaps others? Parcel? Electron?)
  • Support Node.js 8+, drop support for Node.js 6 and earlier
  • Migrate CI to GitHub Actions, add more recent OS and Node.js versions to integration tests
  • Use Node.js 12+ native report feature where available, allows family detection and glibc version (but not musl version)

Progress:

@coreprocess
Copy link
Contributor

@lovell a million times per day, crazy!

@styfle
Copy link
Contributor

styfle commented Jan 3, 2022

Use Node.js 12+ native report feature where available, allows family detection and glibc version (but not musl version)

Would it make sense to add musl detection to the report as well?

I started a related PR here: nodejs/node#41338

@lovell
Copy link
Owner Author

lovell commented Jan 12, 2022

@styfle Yes, the addition of process.report.getReport().header.muslVersionRuntime would be useful, but I'm unsure if musl exposes __libc_version via its external API - see https://git.musl-libc.org/cgit/musl/tree/src/internal/version.c

@lovell
Copy link
Owner Author

lovell commented Jan 17, 2022

The new main branch (which will become the default branch) has all the latest code and tests.

If anyone would like to help review this, add more containers for integration testing or submit further improvements, all these things are very welcome.

https://github.com/lovell/detect-libc/tree/main

I plan to publish this as v2.0.0, probably later this week.

@lovell
Copy link
Owner Author

lovell commented Jan 19, 2022

v2.0.0 now available. Please open a new issue if you discover any problems. PRs to add more containers for integration testing are welcome too.

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

No branches or pull requests

3 participants