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

Node2nix: switch to community-maintained fork #82228

Closed
wants to merge 3 commits into from
Closed

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Mar 10, 2020

Motivation for this change

node2nix had some outstanding pull requests that had not been merged in months, but are required for nixpkgs. Therefor we forked the repository to https://github.com/nix-community/node2nix

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Mic92
Copy link
Member Author

Mic92 commented Mar 10, 2020

Oh, Looks like the new update.sh script is still buggy: https://gist.github.com/GrahamcOfBorg/e1113e8e467703b2115b7e9c182ec2eb#file-output-path-comparison-L10

@Mic92 Mic92 force-pushed the node2nix branch 2 times, most recently from ac9456e to 7477814 Compare March 10, 2020 11:06
Previous version was total broken since paths were computed wrong.
Since we rely on relatives paths we cannot run node2nix outside of nixpkgs.
Since there is no response from upstream, we decided to move
it for the time being to Nix community.
@svanderburg
Copy link
Member

I just integrated most of the open pull requests into node2nix. That includes all the pull requests that got merged into the fork already.

I still have some small documentation improvements that I want to make, some dependency updates and few QA tests with projects that I cannot release publicly.

Unfortunately, I have not been able to work much on node2nix or any open source project in the last few months due to personal issues, as you might be able to observe by checking my Github activity or blog.

Anyway, my schedule is now mostly cleared up and I can have a 1.8.0 version released by the end of this week.

I can also do the update in Nixpkgs to use provide newly generated expressions with 1.8.0, if it still desired to use that.

Anyway, I leave that decision open to the community. For me personally, it does not matter much what is used in Nixpkgs because I mostly use node2nix on project level and that is my main motivation to develop it.

@flokli
Copy link
Contributor

flokli commented Mar 10, 2020

I tested this PR and was able to successfully (re)-generate the packageset. Thanks!

@svanderburg thanks for the feedback, and all the work you put into node2nix. It's important to sometimes take a step back, especially if more important private stuff comes up.

For maintainability, and to decrease the bus-factor I still think moving over node2nix to the nix-community project sounds like a good idea, especially as it's used for the nixpkgs tooling.

Do you have any strong reservations against doing so, and moving development to there?

@svanderburg
Copy link
Member

I don't have any strong objections.

What I basically care about is that I want node2nix to offer a high degree of compatibility and be based around as many "solid principles" as possible to ensure that we can derive and automate as much as possible without too many ad-hoc patches/workarounds and hacks.

I still remember when I was contributing to npm2nix years ago, is that I sort of lost how the whole thing worked and the only thing I did was adding patch layers on top of it so that I could get my use-case supported. With node2nix I'm basically trying to prevent that from happening.

Opening it up to other people and thinking about a better way to organize and coordinate changes in the future is definetely something that I consider a good thing. How that should be done should probably be discussed in a future thread.

Another question to all the folks here: are there any other changes in the fork that I missed? Not that I will blindly accept them, but I can at least look at them and see how we can get the underlying requirement implemented/covered.

Also slightly off-topic: I have some future plans on the drawing board to improve certain aspects, but they take a bit of time to materialize. For example, node-env.nix has evolved into a unmaintainable beast that is way too complicated than it should be. I already have some ideas how we can make the implementation a lot simpler and better customizable.

And another I'm considering is extracting certain pieces from node2nix into separate modules so that this functionality can be used independently. The advantage of doing this is that we can, for example, reuse certain concepts in other JavaScript deployment solutions, e.g. yarn integration, or to use them independently.

@svanderburg
Copy link
Member

I also realize that there is discrepancy between what people typically want for projects and what people want for packages in Nixpkgs.

For the former, we probably want more accuracy (at least I do, because I have people whose production deployments will break if I don't simulate NPM correctly), but for the latter we probably want to be more pragmatic -- in Nixpkgs, we typically deploy end-user software and we want that to work. To accomplish that, temporary hacks/fixes and overrides/hack, and slight mismatches between dependencies deployed by node2nix and NPM are more acceptable.

To address the latter requirement, maybe having a more hackable implementation for Nixpkgs could (with a common infrastructure shared by both implementations) could help. Maybe that's also something we want to discuss to make these kind of changes less painful in the future.

@Mic92
Copy link
Member Author

Mic92 commented Mar 11, 2020

@svanderburg I would suggest to mark you as the maintainer in https://github.com/nix-community/node2nix just as we do for other Nix community projects i.e. https://github.com/nix-community/nixops-libvirtd
From there we might also find new people under your supervision to help looking at pull requests. What do you think? If that is the case I would delete the fork so that you can move it over to the organization.

@svanderburg
Copy link
Member

Aside from the discussions, and all the recent news about the CORVID-19 virus (that now forces me to work from home), I have been updating the testcases and processed the issues list. This resulted in a number of additional fixes and a couple of small documentation improvements.

The 1.8.0 release is now nearly finished. Actually: I don't expect to make any more changes for this release, but I want to have my usual validation process finished first. The testcases should all turn green very soon: https://hydra.nixos.org/jobset/node2nix/master#tabs-jobs

Tomorrow, I will do some tests with my own projects and a few non-public projects. This also gives some of the people who have reported issues time to respond to my fixes. (I typically ask people who reported problems to test their problematic case again with the development version)

If this finishes, I will publish version 1.8.0 and then we're ready to update Nixpkgs again (if it's still desired by the community to use the vanilla version). If that's desired, I can take care of doing the integration this upcoming weekend, and then this pull request can be closed.

@svanderburg
Copy link
Member

It's a good as ready now. I also took opportunity to get some the related projects updated. If nobody objects, I'll update Nixpkgs tomorrow.

@flokli
Copy link
Contributor

flokli commented Mar 14, 2020

@svanderburg thanks for taking the time merging outstanding PRs, documenting and testing things, much appreciated :-)

I'm not sure if there has been any communication with @Mic92 outside this PR, but do you still plan to move the repo to the nix-community organization?

@Mic92
Copy link
Member Author

Mic92 commented Mar 15, 2020

I'm not sure if there has been any communication with @Mic92 outside this PR, but do you still plan to move the repo to the nix-community organization?

None yet.

@svanderburg
Copy link
Member

There has not yet been any communication, but I'll get back to this soon and we should probably have another discussion about it in another thread.

To make node2nix a true community project, I'd like to discuss a few things first, because there are a variety of reasons/choices why it was developed in the first place (e.g. things I have explained earlier) but also a few others, and I'm also deviating with many of my choices in comparison with most other language-specific Nix generators.

This also partially explains why there was a strong reason in the past to deliberately call it node2nix and not npm2nix. First, the idea was to replace the old npm2nix, but the two ended up co-existing for quite some time, because some of my choices were not acceptable to the majority at that time.

Eventually, we still ended up using node2nix in Nixpkgs, because npm2nix became too problematic to handle cyclic dependencies, as more and more packages ended up having es5-ext in their dependency tree that relies on cyclic dependencies. It was not that I really want node2nix to become the new standard -- there was basically no alternative.

For me it is important that node2nix's original values are maintained and aligned, otherwise I could never have brought in a state that provides all the features that it currently supports. If these are clear, then I would not have any problems opening up the repository up, give people commit rights and transfer it to the community group, or some other place.

There may also still be strong reasons why it is still desired to have a community fork, a different tool, or a tool that shares parts with the node2nix implementation that is more suitable for the needs to manage NPM packages in Nixpkgs, that can be more progressively developed as well.

Anyway, my main goal now is to get all NPM stuff up to date in Nixpkgs and I'm already quite far with that. The master branch was successfully updated.

Furthermore, if people want to help me improving NPM integration even further, I have created the following issue:

#82662

I think addressing the above issue is pretty important to keep the NPM packages in Nixpkgs maintainable. It's quite a bit of work, but really helpful.

I'm also working on a solution to considerably simplify node-env.nix. I think this file is now the biggest reason why things are hard and require many changes/tweaks/overrides. I can actually do this in a very interesting way that provides you all kinds of new possibilities. More about this later.

@svanderburg
Copy link
Member

I have closed this ticket, because it is no longer mergeable, and the current master branch was updated with the recently released 1.8.0 version that has all required changes to work with it.

@Mic92 Mic92 deleted the node2nix branch March 16, 2020 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants