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

Provide pnpm via nix (again)? #541

Closed
brprice opened this issue Oct 5, 2022 · 7 comments
Closed

Provide pnpm via nix (again)? #541

brprice opened this issue Oct 5, 2022 · 7 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request Nix Nix- or nixpkgs-related nodejs Related to the Node.js ecosystem

Comments

@brprice
Copy link
Contributor

brprice commented Oct 5, 2022

Main issue

Now that upstream supports new versions of nodejs (NixOS/nixpkgs#132456 NixOS/nixpkgs#145432 NixOS/nixpkgs#193337 (this last is a PR which is included in our nixpkgs pin)), can we provide pnpm via nix, so one does not have to bootstrap manually?

I have tested that adding pnpm to the inputs does provide the binary in the shell, and seems to work as well as the manual bootstrap (more below).

A related issue is #138, about prettier in nixpkgs.

Alternatively, it may be nice to change the shell hook to (either auto-install pnpm via npx or) detect if pnpm is available and if not halt and print a useful message, so one can nicely use this repo's devshell to bootstrap.

Bootstrapping woes (ignore this -- it is a red herring caused by me running (for unrelated reasons) an incompatible fork of nix)

My testing of this idea has been somewhat curtailed by the fact I don't know how to get a successful devshell currently. The README states "To develop interactively, enter the Nix shell via nix develop ... the first time you check out the primer-app repo, you'll need to run npx pnpm install ... After the initial bootstrap, you'll be able to run just nix develop and then all the standard pnpm command should work". My procedure (on a clean checkout of main) is

  • I have no npx in my normal shell (outside of any nix shell)
  • I need to use npx to install pnpm
  • Hopefully nix develop in this repo can provide the correct version of npx
  • Do a nix develop and see
    pnpm: command not found                 
    error (ignored): error: end of string reached
    error: getting status of '/bin/Setup': No such file or directory
    (use '--show-trace' to show detailed location information)
    pnpm: command not found  
    
  • It does land me in a devshell (but in a weird directory), with npx
  • Manually npx pnpm install
  • exit the devshell
  • Hopefully everything is now set up, so try to re-enter the devshell, but see
    Scope: all 4 workspace projects
    Lockfile is up to date, resolution step is skipped
    Already up to date
    error (ignored): error: end of string reached
    error: getting status of '/bin/Setup': No such file or directory
    (use '--show-trace' to show detailed location information)
    ln: failed to create symbolic link './primer-api.json': File exists
    
    > @hackworthltd/primer-app@0.2.0 generate /home/hackworth/primer-app/packages/primer-app
    > orval --config ./orval.config.js
    
    🍻 Start orval v6.10.2 - A swagger client generator for typescript
    ⚠️  ResolverError: Error opening file "/home/hackworth/primer-app/packages/primer-app/primer-api.json" 
    ENOENT: no such file or directory, open '/home/hackworth/primer-app/packages/primer-app/primer-api.json'
    🛑  primer-api - ResolverError: Error opening file "/home/hackworth/primer-app/packages/primer-app/primer-api.json" 
    ENOENT: no such file or directory, open '/home/hackworth/primer-app/packages/primer-app/primer-api.json'
    /home/hackworth/primer-app
    

I am not sure what is wrong with this, but I see the same thing when (in a clean checkout) entering a devshell after adding pnpm to the dependencies (this time I don't have to do the manual install pnpm step)

@brprice brprice added documentation Improvements or additions to documentation Nix Nix- or nixpkgs-related enhancement New feature or request nodejs Related to the Node.js ecosystem labels Oct 5, 2022
@georgefst
Copy link
Contributor

I agree it would be nice to provide pnpm through Nix. Especially as our shell hook actually calls pnpm generate.

It does land me in a devshell (but in a weird directory),

What directory? That could be relevant (see #531).

@brprice
Copy link
Contributor Author

brprice commented Oct 5, 2022

It does land me in a devshell (but in a weird directory),

What directory? That could be relevant (see #531).

That was a fast reply! I land in packages/primer-app, because the shell hook has cd packages/primer-app && pnpm generate && cd -, but pnpm generate fails.

@brprice
Copy link
Contributor Author

brprice commented Oct 5, 2022

In general, I think all the issues with the shellhook stem from the fact that nix-build -A packages.${system}.primer-openapi-spec fails. This means that OPENAPI_SPEC is empty, so ln -s $OPENAPI_SPEC ${local-spec} fails, and then pnpm generate fails as it does not have a spec to work from.

I wonder if it is worth being a bit more defensive: either

  • set -e (maybe in a subshell, so we don't pollute the devshell)
  • catching problems with the nix-build primer-openapi-spec and pnpm lines, and aborting with a helpful message.

@brprice
Copy link
Contributor Author

brprice commented Oct 5, 2022

nix-build -A packages.${system}.primer-openapi-spec fails

(this is getting a bit off topic, but just to close the book on why this happens) This is because I am running an incompatible fork of nix. If I use nix 2.8, then this works ok.

@dhess
Copy link
Member

dhess commented Oct 5, 2022

I'm not opposed to making the bootstrap process more bulletproof, or adding better error handling in the event that a nix command in the shellHook breaks (set -e would suffice IMO — I don't think it makes sense to add defensive programming to accommodate scenarios like broken Nix forks), but I don't think it's a high priority since this is something we typically only need to do once (or at least, extremely rarely), and nobody outside the company will use this repo anytime soon.

We've been burned several times, in several different ways, when trying to use nixpkgs for Node-related functionality. I don't think we can rely on nixpkgs to keep anything but nodejs up-to-date: the nixpkgs issue that was preventing us from using a nixpkgs-provided pnpm binary on a recent nodejs release was unresolved for over a year! I'd rather do a simple one-time bootstrap step and then use pnpm to maintain itself.

The dream2nix project looks promising as a holy grail solution for the Node ecosystem in Nix, but it's still very early days.

@brprice
Copy link
Contributor Author

brprice commented Oct 6, 2022

I'm not opposed to making the bootstrap process more bulletproof, or adding better error handling in the event that a nix command in the shellHook breaks

I'll get a pr up shortly (edit: #544)

I don't think it makes sense to add defensive programming to accommodate scenarios like broken Nix forks

Of course not, I didn't mean to suggest that

We've been burned several times, in several different ways, when trying to use nixpkgs for Node-related functionality. I don't think we can rely on nixpkgs to keep anything but nodejs up-to-date: the nixpkgs issue that was preventing us from using a nixpkgs-provided pnpm binary on a recent nodejs release was unresolved for over a year! I'd rather do a simple one-time bootstrap step and then use pnpm to maintain itself.

Fair enough

@brprice
Copy link
Contributor Author

brprice commented Oct 6, 2022

We decided that we would prefer to keep the one-time bootstrap step than rely on nixpkgs providing pnpm

@brprice brprice closed this as completed Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request Nix Nix- or nixpkgs-related nodejs Related to the Node.js ecosystem
Projects
None yet
Development

No branches or pull requests

3 participants