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

Needed: New maintainership of node-pre-gyp #657

Closed
springmeyer opened this issue May 19, 2022 · 25 comments
Closed

Needed: New maintainership of node-pre-gyp #657

springmeyer opened this issue May 19, 2022 · 25 comments

Comments

@springmeyer
Copy link
Contributor

springmeyer commented May 19, 2022

I'm stepping down from maintaining node-pre-gyp. For the next month and a half (until the end of June) I can be available to a) facilitate moving maintainer-ship to someone else and b) be available to answer any questions on the transition from that new maintainer or the wider community (on this ticket).

However I will no longer be reviewing PRs, making releases, or responding to bug reports.

As far as a new maintainer, that maintainer could be:

  • A single person or group/team
  • Inside or outside of Mapbox
  • Could be given access to this repo or (ideally in my opinion) could take ownership of this module under a new organization
  • Either someone who has a vision for what good long-term maintenance looks like or someone who has a vision for how to potentially sunset/deprecate node-pre-gyp without causing undue turbulence for the node.js community

In my mind, all are good options and I will consider any proposals and try to decide promptly (by the end of June) to find a good solution that ensures a) node-pre-gyp is well maintained going forward for the benefit of the entire node.js community and b) also satisfies Mapbox needs.

So, please comment on this ticket if you or your org are interested. Or reach out to me to discuss by emailing me at dane@mapbox.com.

Thanks!


For those curious for extra context and history, here is a quick summary.

Since 2013 I (@springmeyer) have been the primary maintainer of node-pre-gyp. Over this time I've tended to be the single point of contact both for internal Mapbox customers of node-pre-gyp as well as for the wider node.js community (fielding requests from companies using node-pre-gyp for mission critical modules as well as tending to the public github issue queue).

I've recruited help from various Mapbox colleagues over the years (thank you especially @mapsam, and @wilhelmberg) and from countless generous open source contributors (like @Mithgol, @murgatroid99, @brianreavis, @jschlight, and @bmacnaughton). I'm especially grateful to these external folks for helping to keep things chugging during times when I've been in management roles without any time for coding.

Recently I've been away on parental leave and coming back I can see how important it is to find a new maintainer for this module.

The main reasons are:

  • I no longer have the time or ability to prioritize node-pre-gyp maintenance at the level the node.js community needs (for example, look at all the great work from https://github.com/mapbox/node-pre-gyp/pulls/ronilan that needs attention)
  • Mapbox internal needs have shifted towards needing better support for private C++ addons, which node-pre-gyp does not yet support, and therefore it seems to me that someone needs to lead the effort to address this (either by adding robust private module support to node-pre-gyp or moving off node-pre-gyp internally at Mapbox).
  • CI systems and testing for the project need special love and attention. A great step forward would be to move node-pre-gyp CI to use github actions. But Mapbox's github org does not permit github actions, so short of Mapbox changing their policy (not something I have time to address) the only way I can see to get github actions unblocked would be to move node-pre-gyp outside the Mapbox org.

The quick timeline of node-pre-gyp is that:

  • In ~2011 Mapbox started using node.js during the Node.js 0.1.x series for all our services
  • Soon after we started porting over various C++ modules which we'd been using via Python to node.js (mapnik) and writing others from scratch (node-sqlite3, node-srs, node-zipfile, etc)
  • Also in 2011 we launched the TileMill project (https://tilemill-project.github.io/tilemill/) which featured many node C++ addons compiled into it in a way that was portable across OS X, Linux, and Windows such that users could download a desktop app and have it work without any additional configuration or knowledge that node.js was involved.
  • In 2012 we started work on cloud based mapping systems, and found ways to distribute those same Node C++ addons powering the TileMill desktop app to various linux distros (ubuntu, RHEL, solaris) on AWS.
    • Since we wanted complete control over dependencies for Mapnik (like libjpeg, boost, libpng, etc) and we wanted a system that would work across any linux version and macs we chose not to use standard linux package managers (apt, rpm, homebrew, etc) to handle binary deployment
    • Because several of our key (at the time) node modules (like node-mapnik and node-gdal) included binaries over 1-200 MB in size (per platform) we knew that a system that stored the binaries inside the package published to https://npmjs.com/ would not scale well (instead it would be more scalable to have a system to store the binaries separately, per platform)
    • So we leveraged platform agnostic static linking method to create binaries and archived them on s3. We therefore needed a node.js mechanism to reliably download the statically linked C++ binaries from s3 during npm install (this became node-pre-gyp).
  • Also in 2012 @ErisDS from the https://github.com/TryGhost reached out about how to make node-sqlite3 easier to install for windows users
  • So in 2013 I launched node-pre-gyp publicly such that the internal tool that helped us scale our AWS cloud deployments could also be used to handle binary deployment for windows users of node-sqlite3
  • At the time (and for many years after) I hoped that node-pre-gyp would become nothing more than the "art of the possible" and would inspire the Node.js community to find a solution inside of node core that would avoid anyone needing node-pre-gyp.
  • Instead what happened is very widespread adoption of node-pre-gyp and over time also additional projects like https://github.com/prebuild/prebuild.
  • When prebuildify was launched I hoped that Mapbox could move our modules to it and deprecate node-pre-gyp. But two things caused friction: a) prebuildify tooling works by storing the binaries inside the npm package which works for most use-cases but not for the Mapbox use-cases that involve really large binaries for many different platforms (https://github.com/prebuild/prebuild-install#note) and b) by the time prebuildify was created I was working on totally different things at Mapbox than node.js c++ addon development so I never found time to address a) with any creative solutions.
  • So, to this day node-pre-gyp only supports storing binaries on s3 (which was all Mapbox needed in ~2013) and does not support a) private modules, b) posting to github releases (like the third-party https://github.com/bchr02/node-pre-gyp-github allows), or c) allowing binaries inside of the npm package. All of these things seem quite reasonable to support to me, but are not something I will be able to help with. Hence this issue. Thanks for reading all the way to here :)

/cc @flippmoke @aswamina @danpat @yhahn @mapbox/security-and-compliance

@springmeyer
Copy link
Contributor Author

springmeyer commented Sep 26, 2022

In my mind, all are good options and I will consider any proposals and try to decide promptly (by the end of June) to find a good solution that ensures a) node-pre-gyp is well maintained going forward for the benefit of the entire node.js community and b) also satisfies Mapbox needs.

So, please comment on this ticket if you or your org are interested. Or reach out to me to discuss by emailing me at dane@mapbox.com.

Following up here that I did not receive any comments or questions of interest. Also dane@mapbox.com is no longer a valid email as I am no longer an employee at Mapbox and therefore I no longer have maintenance access to this repo. Therefore the time has passed that I would have been able to easily and formally pass on responsibility to others. If anyone finds this issue in the future, and has an interest in taking on maintenance responsibility, I recommend just commenting on this issue (instead of contacting me directly as I previously stated).

@ronilan
Copy link
Contributor

ronilan commented Feb 2, 2023

If there is interest from corporate users of the package for more active maintenance and/or tacking some of the more prominent open issues, I may be able to put time/work on this starting spring. Feel free to reach out if relevant.

@fcanela
Copy link

fcanela commented Oct 8, 2023

Can this unmaintained status be announced in the README.md?

That may help to find new maintainers and quickly understand that the recent issues can not be resolved meanwhile.

daniellockyer added a commit to TryGhost/node-sqlite3 that referenced this issue Dec 24, 2023
…stall`

fixes #1641
fixes #1721
fixes #1714
fixes #1713
fixes #1700
fixes #1704

- `@mapbox/node-pre-gyp` is effectively unmaintained [1] as has a few bugs
  which our users keep running into
- it seems the prebuilt binary world has moved in favor of prebuild +
  it's various other forms
- one option would be to use prebuildify to bundle all binaries into the
  package, but that's a step too far removed from the current situation
  for now
- instead, we can use prebuild-install to download the binaries, and
  `prebuild` to build + upload the binaries
- this means we can remove node-pre-gyp and fix a bunch of issues!

[1]: mapbox/node-pre-gyp#657
daniellockyer added a commit to TryGhost/node-sqlite3 that referenced this issue Dec 24, 2023
…stall`

fixes #1641
fixes #1721
fixes #1714
fixes #1713
fixes #1700
fixes #1704

- `@mapbox/node-pre-gyp` is effectively unmaintained [1] as has a few bugs
  which our users keep running into
- it seems the prebuilt binary world has moved in favor of prebuild +
  it's various other forms
- one option would be to use prebuildify to bundle all binaries into the
  package, but that's a step too far removed from the current situation
  for now
- instead, we can use prebuild-install to download the binaries, and
  `prebuild` to build + upload the binaries
- this means we can remove node-pre-gyp and fix a bunch of issues!
- eventually, we could start providing electron prebuilt binaries too

[1]: mapbox/node-pre-gyp#657
daniellockyer added a commit to TryGhost/node-sqlite3 that referenced this issue Dec 24, 2023
…stall`

fixes #1641
fixes #1721
fixes #1714
fixes #1713
fixes #1700
fixes #1704

- `@mapbox/node-pre-gyp` is effectively unmaintained [1] as has a few bugs
  which our users keep running into
- it seems the prebuilt binary world has moved in favor of prebuild +
  it's various other forms
- one option would be to use prebuildify to bundle all binaries into the
  package, but that's a step too far removed from the current situation
  for now
- instead, we can use prebuild-install to download the binaries, and
  `prebuild` to build + upload the binaries
- this means we can remove node-pre-gyp and fix a bunch of issues!
- eventually, we could start providing electron prebuilt binaries too

[1]: mapbox/node-pre-gyp#657
@cclauss
Copy link
Collaborator

cclauss commented Apr 11, 2024

Is there anyone who still has maintenance access to this repo?

I am an active maintainer of node-gyp and would be interested to work with @ronilan (as discussed above) to keep this repository up to date.

At 6.5 million weekly downloads and >1000 dependents (across @mapbox/node-pre-gyp and node-pre-gyp) it's clear this package is well used in the ecosystem...

@petersirota, @yhahn, @willwhite, @mapsam

https://github.com/nodejs/node-gyp/issues?q=is%3Aissue+label%3A%22node-pre-gyp+is+unmaintained%22

@Trott
Copy link

Trott commented Apr 11, 2024

As well as being a node-gyp maintainer, @cclauss is a maintainer of Node.js itself. It's probably not necessary, but if it would help, a lot of us on the Node.js team can vouch for his knowledge, diligence, and trustworthiness. I believe there are also people on the Node.js team who are (or at least were at one time) co-workers of @cclauss.

@ronilan
Copy link
Contributor

ronilan commented Apr 12, 2024

It's funny waking up to a flurry of activity from a comment made more than a year ago... anyway... I wrote the pull requests mentioned by @springmeyer in the original post (which apparently have rotted a little in the past two years) while on @solarwinds paycheque and using two weeks of their time. I'm (thankfully) not employed there anymore, and while I am way busier than I was when I made the offer @cclauss refers to, if there is a funding, I'd consider.

@styfle
Copy link

styfle commented May 13, 2024

I am no longer an employee at Mapbox and therefore I no longer have maintenance access to this repo

@springmeyer Seems like this can be solved by forking the repo, however the real issue is npm publish permission.

While you might not have access to publish @mapbox/node-pre-gyp npm package, it looks like you still have access to publish to the original node-pre-gyp npm package.

Perhaps you could grant npm publish permission for node-pre-gyp to @cclauss and we can move forward on a github fork after that? (I'm not sure the origin of the scopeless package if that was actually mapbox or not).

image

@mapsam
Copy link
Contributor

mapsam commented May 13, 2024

Hi folks, @mapsam here from Mapbox, thanks for engaging here and we really appreciate the eagerness to help maintain node-pre-gyp! The project has sorely missed regular updates from us at Mapbox as the teams' priorities have shifted. I'm putting together a plan to help grant the proper access to those who are interested in maintenance and will respond back when I have more details. We're considering all options right now, from granting NPM access and GitHub access, or even transferring the repository entirely to a new maintainer.

If you'd like to reach out to me directly with any ideas feel free to ping me here in this ticket or email me at sam@mapbox.com in the meantime.

@mapsam mapsam pinned this issue May 20, 2024
@benmccann
Copy link
Contributor

@mapsam is there any update on transferring maintainership to @cclauss? I see that he has already reviewed and approved several pending PRs, but I'm guessing he doesn't have merge rights since they're still in the queue. Thanks for your help!

@mapsam
Copy link
Contributor

mapsam commented Jun 28, 2024

Hey folks, thanks for checking back in. I'd like to get moving on a plan so @cclauss and others can make necessary changes without waiting for a larger plan to go into effect. Very much appreciate the input and response so far! Here's what I'm planning:

Phase 1 - "just get moving again"

  • I can add @cclauss and others who are interested to this repo as "Maintainers". This will allow review, approval, and merging of PRs into the primary branch (currently master)
  • We'll keep the NPM @mapbox/node-pre-gyp module namespace for now, I'll be here to help with releases. Our NPM setup is a little unusual at Mapbox, where most of our releases are automated by bot accounts, so we don't have the luxury of adding individual NPM accounts to a module at the moment. I recognize this is not ideal, I want to improve this in Phase 2.
  • Keep the repo within the mapbox/node-pre-gyp GitHub organization until we can find a more appropriate home. Given the potential blast radius with node-pre-gyp, I want to be mindful of any supply-chain NPM attacks and keeping the module within the mapbox org feels best for now (opposed to moving the repo to an individual account) (h/t to @ronilan for the input here)

Phase 2 - "future home"

  • Find a future organization/home for node-pre-gyp that we can transfer away Mapbox so we are no longer blockers to the project
  • Move the module back to the unscoped node-pre-gyp and grant access to that NPM module (I still have access here and can help shepherd this along)

Open to feedback here. Cheers!

@cclauss
Copy link
Collaborator

cclauss commented Jun 29, 2024

Thanks massively @mapsam

I have merged a handful of pull requests that:

  1. remove Travis CI
  2. get Dependabot updating our GitHub Actions and npm dependencies
  3. get AppVeyor testing on modern versions of Node.js
  4. add GitHub Actions tests on Linux, macOS, and Windows

The AppVeyor tests fail so now I think we should only merge PRs that:

  1. Make changes that fix failing AppVeyor and GitHub Actions tests

I am happy to review and merge those pull requests but I would kindly ask that they be as simple to review as possible.

If I create my own pull requests (to add GitHub Actions) who can approve them (I cannot approve my own PRs)?

@benmccann
Copy link
Contributor

If you have a branch with the GitHub Actions setup, I could send it as a PR so you can approve and merge it as a workaround for you not being able to approve and merge your own PRs. (I'm assuming you have only write access to this repo and there are permission restrictions you're facing if I understood your comment correctly)

@cclauss
Copy link
Collaborator

cclauss commented Jun 29, 2024

Yes. Permissions issue. All pull requests must be approved by a repo maintainer before they can be merged. I can approve and merge your PRs but I cannot approve my own PRs.

@cclauss cclauss unpinned this issue Jun 29, 2024
@benmccann
Copy link
Contributor

@mapsam a minor suggestion to pass along since I cannot file issues against @mapbox/eslint-config-mapbox. It would be nice if that library switched from the unmaintained eslint-plugin-node to eslint-plugin-n

@mapsam
Copy link
Contributor

mapsam commented Jul 1, 2024

Thanks @cclauss! I looked at the branch protection settings for master - here's what I'm seeing. Happy to update so you can move quickly. They don't seem very strict, but given we require reviews I suspect GitHub just makes assumptions that you can't approve your own.

I'll be here for reviews for the moment but will be good to get another maintainer to help you out in the meantime. Happy to include @benmccann if that's helpful, but don't want to put pressure on anyone without their consent and explicit interest! We could always work on feature branches before merging into master as well, that way you can segregate changes of work and not require reviews for every single change.

Screen Shot 2024-07-01 at 10 52 57 AM Screen Shot 2024-07-01 at 10 53 18 AM

@mapsam
Copy link
Contributor

mapsam commented Jul 1, 2024

@benmccann I'll get a PR and release out for @mapbox/eslint-config-mapbox in the meantime. I'm not against using an entirely different eslint configuration as well, if that's preferred.

@mapsam
Copy link
Contributor

mapsam commented Jul 1, 2024

@benmccann @cclauss @mapbox/eslint-config-mapbox@5.0.1 has been released which uses eslint-plugin-n. Thanks for the tip! mapbox/eslint-config-mapbox#16

@benmccann
Copy link
Contributor

I'm happy to act as a maintainer in the short-term to get a release out. I'm probably not the best maintainer in the long-term as I don't directly use this package. It just happens to be in our dependency tree and so I'm trying to fix some deprecation warnings (mostly by removing packages when possible so that we don't risk the warnings re-occuring), but I'm not very familiar with the package and would be less able to review actual code changes

Thanks for the new release of the eslint config Sam! I sent a PR here to use it

@lukekarrys
Copy link
Collaborator

I'd be interested in becoming a long-term maintainer for node-pre-gyp. I'm currently a maintainer of node-gyp and a previous maintainer of npm so I have experience in a similar area to node-pre-gyp.

@benmccann @cclauss @mapsam I am making my way through the recent issues and PRs in this repo to gather context. If there are any specifics that would be helpful for me to focus on initially please tag me on them and I will take a look.

@mapsam
Copy link
Contributor

mapsam commented Jul 19, 2024

@lukekarrys lovely, thank you for offering your time and expertise! I've sent you an invitation to be a maintainer of the repo.

@cclauss
Copy link
Collaborator

cclauss commented Jul 20, 2024

@lukekarrys Welcome to the pre-gyp maintainers team.

First priority is to use the new GitHub Action for releasing to create a pre-release on NPM. That should not be the default release but should allow those who want to ask for it by version number to download it and try it out. After a week or two of listening to that feedback, please repeat the process to make a real production release.

It would be best to do this BEFORE merging other changes because we have done a ton of Dependabot changes without any real world feedback. Small moves….

AFTER the real release is out we need to placate Dependabot and then create another production release. My expertise is Python, not JS so I am ecstatic to have your steadier hands on the wheel. Thanks!

@benmccann
Copy link
Contributor

Thank you @lukekarrys both for volunteering and for all the reviews you've already given.

1.1.0-dev.1 was manually released on July 3. There has been just two small PRs with user visible changes committed since that time. There was one PR to replace rimraf with native node.js APIs. And then one to bump the minor range of the semver dependency. It would be worth giving a second look at this latter PR. It makes the semver range more restrictive without a communicated reason for doing so and has left the project in an inconsistent state because it didn't update the lockfile. We should probably either revert the PR or update the lockfile as first order of business.

I agree setting up the release process is fairly high priority. I think we could make some other changes in parallel since there's already a pre-release version out on npm available for testing containing the majority of prior changes. The other PRs I think are highest priority are replacing npmlog, which would address multiple deprecation warnings being caused by the project's transitive dependencies, and updating dependabot to silence noise in the PR queue. Unfortunately I only have experience with rennovate, so don't have a ton of experience with dependabot. I just returned from vacation last night, so can try to test out the dependabot config changes in my fork this week if you'd prefer to avoid the churn of experimenting with it a bit here. I also have PRs out to upgrade or remove the other direct dependencies that are behind.

@cclauss
Copy link
Collaborator

cclauss commented Jul 20, 2024

Was there any feedback on 1.1.0-dev.1?

@benmccann
Copy link
Contributor

No complaints so far. I think it'd be nice to merge another batch of PRs and do another pre-release.

@cclauss
Copy link
Collaborator

cclauss commented Jul 22, 2024

Let’s let @lukekarrys decide.

@cclauss cclauss closed this as completed Jul 22, 2024
@mapbox mapbox locked as resolved and limited conversation to collaborators Jul 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants