Skip to content

antora: repackage using buildNpmPackage#229512

Merged
SuperSandro2000 merged 1 commit intoNixOS:masterfrom
ehllie:antora
May 6, 2023
Merged

antora: repackage using buildNpmPackage#229512
SuperSandro2000 merged 1 commit intoNixOS:masterfrom
ehllie:antora

Conversation

@ehllie
Copy link
Contributor

@ehllie ehllie commented May 2, 2023 •
edited
Loading

Description of changes

Repackages nodePackages.@antora/cli to be built using buildNpmPackage as per the suggestion in #229475
@winterqt

Things done

I've had to link the long $out/lib/node_modules/antora-build/packages/cli/bin/antora path to $out/bin/antora because the the base repository is a monorepo containing a bunch of tools. I tried setting an npm flag --package cli to only build the @antora/cli, but that doesn't seem to have changed anything. Is there a better way of doing this?

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@ehllie ehllie added the 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment label May 2, 2023
@ehllie
Copy link
Contributor Author

ehllie commented May 2, 2023 •
edited
Loading

@domenkozar I've overwritten your previous package derivation in pkgs/development/tools/documentation/antora/default.nix, which looked like it just linked the @antora/site-generator-default with the node2nix generated package, but the buildNpmPackage builder builds that one as well by default.

That also makes me realise there's still @antora/site-generator-default in node-packages.json, but it gets built here. @antora/site-generator-default itself does not provide any binaries, so it might be worth deleting it from node-packages.json as well?

@ehllie ehllie mentioned this pull request May 2, 2023
Open
@ehllie ehllie requested a review from winterqt May 2, 2023 19:00
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels May 2, 2023
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented May 3, 2023

That also makes me realise there's still @antora/site-generator-default in node-packages.json, but it gets built here. @antora/site-generator-default itself does not provide any binaries, so it might be worth deleting it from node-packages.json as well?

Good catch, does not sound useful to me.

SuperSandro2000
SuperSandro2000 reviewed May 3, 2023
View reviewed changes
SuperSandro2000
SuperSandro2000 reviewed May 3, 2023
View reviewed changes
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

I think it is fine to not regen the nodePackages lock file all the time to reduce merge conflicts. On the next regen the entries will just disappear and until then we have some dead code but the workflow is going to be so much nicer.

@ehllie
Copy link
Contributor Author

ehllie commented May 3, 2023

That also makes me realise there's still @antora/site-generator-default in node-packages.json, but it gets built here. @antora/site-generator-default itself does not provide any binaries, so it might be worth deleting it from node-packages.json as well?

Good catch, does not sound useful to me.

I'm wondering what to do with nodePackages.@antora/site-generator-default. If we just remove it from the package set, it will get removed the next time nodePackages gets regenerated, and that theoretically could break some people's code if they were depending on it directly. Should we just alias it to pkgs.antora like @antora/cli?

@lilyinstarlight
Copy link
Member

lilyinstarlight commented May 3, 2023

I've had to link the long $out/lib/node_modules/antora-build/packages/cli/bin/antora path to $out/bin/antora because the the base repository is a monorepo containing a bunch of tools. I tried setting an npm flag --package cli to only build the @antora/cli, but that doesn't seem to have changed anything. Is there a better way of doing this?

We might need the workspace logic from #218923. I may extract the workspace logic and put it in a newer PR with review comments addressed, so that we can get that in sooner and be able to package more npm monorepos

@ehllie
Copy link
Contributor Author

ehllie commented May 6, 2023

I have rebased with master and included antora in the overrides file

@SuperSandro2000 SuperSandro2000 merged commit 0a131c1 into NixOS:master May 6, 2023
@bjornfor bjornfor mentioned this pull request May 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 approved these changes

@winterqt winterqt Awaiting requested review from winterqt

+1 more reviewer

@lilyinstarlight lilyinstarlight lilyinstarlight left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

Repackage node2nix packages
Status: Re-packaged

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ehllie @SuperSandro2000 @lilyinstarlight