Skip to content

feat(nix): Add development shell and package export#231

Open
m4r1vs wants to merge 13 commits intomodem-dev:mainfrom
m4r1vs:main
Open

feat(nix): Add development shell and package export#231
m4r1vs wants to merge 13 commits intomodem-dev:mainfrom
m4r1vs:main

Conversation

@m4r1vs
Copy link
Copy Markdown

@m4r1vs m4r1vs commented May 7, 2026

I use nix and try to avoid globally installing npm packages.
There is a pull request to add Hunk to nixpkgs upstream, however it is stalled by support for direct bun packaging.

We use bun2nix which I have had good experience with in the past.

This would close #215

I've tested this on x86 NixOS, arm64 Fedora with Nix installed and MacOS Tahoe with nix-darwin. I ran into a couple of issues which I solved by removing bun as a dependency in package.json. Not sure why it's there -- I've never seen it installed like this before.

I tried to keep the documentation in README and CONTRIBUTING minimal and elaborated more in a new README in the nix directory.

Happy to answer any questions and to maintain the nix flake if maintanance is needed. Cheers :)

Ps. I also added a home-manager module so Hunk can be configured using nix. It also allows setting it as the default git pager so git diff works when using hunk through home-manager without any extra configuration needed.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 7, 2026

Greptile Summary

This PR adds Nix flake support to the repository, providing a default package derivation (built with bun2nix) and a development shell, along with direnv integration, updated documentation, and removal of bun as a runtime dependency.

  • flake.nix exports the package correctly under packages.${system}.default, but uses the deprecated devShell attribute instead of the required devShells.${system}.default, causing nix develop and use flake in .envrc to fail on typical Nix installations.
  • package.json wires bun2nix into the postinstall hook, so every contributor running bun install will silently regenerate nix/bun.lock.nix with what appears to be an incorrect -c ../ path; this side effect should be an opt-in script instead.
  • nix/package.nix hardcodes the version string that will drift from package.json, and the nix/README.md usage example references stdenv without a pkgs. prefix, which will fail to evaluate in typical NixOS/home-manager configs.

Confidence Score: 3/5

The Nix infrastructure changes are non-breaking for existing npm/bun workflows, but the devShell typo means the primary new feature (dev shell / direnv integration) does not work, and the postinstall hook silently runs a Nix-specific tool on every install for all developers.

Two issues block the stated goals: devShell (deprecated singular) means nix develop and .envrc's use flake won't activate the shell, and the postinstall hook runs a Nix-only regeneration step with an incorrect path flag on every bun install for all contributors. These need to be resolved before the flake is usable as advertised.

flake.nix and package.json need the most attention before merging.

Important Files Changed

Filename Overview
flake.nix New Nix flake entry point; uses deprecated devShell instead of devShells, which will prevent nix develop and direnv use flake from working correctly
package.json Adds bun2nix devDependency and a postinstall hook that regenerates the Nix lockfile on every bun install for all developers, with a likely-incorrect -c ../ path flag
nix/package.nix Nix derivation for building the hunk binary; version is hardcoded and will drift from package.json when bumped
nix/devShell.nix Minimal development shell providing bun and git; straightforward and correct
nix/README.md Installation guide for Nix users; the packages example uses stdenv.hostPlatform.system which is out of scope in typical NixOS/home-manager configs
.envrc Adds use flake for direnv integration; will not activate the dev shell until devShell is corrected to devShells in flake.nix
flake.lock Auto-generated Nix flake lock file pinning nixpkgs and bun2nix to specific revisions
nix/bun.lock.nix Auto-generated bun2nix dependency lockfile; no manual review needed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User runs `bun install`] --> B[postinstall hook fires]
    B --> C[bun2nix -o nix/bun.lock.nix -c ../]
    C -->|Wrong path flag| D[⚠️ Context = parent of repo root]
    C -->|Overwrites for all devs| E[nix/bun.lock.nix regenerated]

    F[User runs `nix develop`] --> G{Look for devShells.sys.default}
    G -->|Not found - deprecated devShell.sys| H[⚠️ May fail on newer Nix]

    I[.envrc use flake] --> G

    J[User runs `nix build .#packages.sys.default`] --> K[packages.sys.default]
    K --> L[nix/package.nix bun2nix.mkDerivation]
    L --> M[bun build --compile src/main.tsx]
    M --> N[result/bin/hunk OK]
Loading
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
flake.nix:42-51
**`devShell` should be `devShells`**

The modern Nix flake schema uses `devShells.${system}.default`, not `devShell.${system}`. With the current structure, `nix develop` and `use flake` in `.envrc` will fail to resolve the dev shell because they look for `devShells.${system}.default` — the top-level `devShell` attribute is the deprecated singular form. The `packages` output already uses the correct nested format; `devShells` needs the same treatment.

```suggestion
    devShells = forAllSystems (
      system: let
        pkgs = import nixpkgs {
          inherit system;
        };
      in {
        default = pkgs.callPackage ./nix/devShell.nix {
          inherit pkgs;
        };
      }
    );
```

### Issue 2 of 4
package.json:73
**`postinstall` regenerates Nix lockfile for every developer**

The `postinstall` script runs `bun2nix` on every `bun install`, meaning every contributor will silently overwrite `nix/bun.lock.nix` even if they have no interest in Nix. Additionally, the `-c ../` flag points one directory above the repo root when `postinstall` runs from the package root, which is almost certainly wrong. Consider moving this to a standalone script or an explicit `npm run nix:lock` command instead.

```suggestion
    "nix:lock": "bun2nix -o nix/bun.lock.nix"
```

### Issue 3 of 4
nix/package.nix:1-4
**Version hardcoded and will drift from `package.json`**

`version = "0.10.0"` is duplicated from `package.json`. When the project version is bumped, this derivation will silently report the wrong version. Consider reading it from the source tree via `lib.importJSON ../package.json`.

```suggestion
{ bun2nix, lib, ... }:
let
  packageJson = lib.importJSON ../package.json;
in
bun2nix.mkDerivation {
  pname = "hunkdiff";
  version = packageJson.version;
```

### Issue 4 of 4
nix/README.md:20-23
**`stdenv` is likely out of scope in this snippet**

The example uses `${stdenv.hostPlatform.system}` without a `pkgs.` prefix. In a typical NixOS or home-manager module, `stdenv` is not automatically in scope — the reader would need `${pkgs.stdenv.hostPlatform.system}` or the simpler `${pkgs.system}`. As written, copy-pasting this example will result in an "undefined variable" evaluation error.

Reviews (1): Last reviewed commit: "fix: remove bun as npm dependency" | Re-trigger Greptile

Comment thread flake.nix Outdated
Comment thread package.json Outdated
Comment thread nix/package.nix Outdated
Comment thread nix/README.md Outdated
@m4r1vs m4r1vs force-pushed the main branch 2 times, most recently from 162c5ca to bf6ccca Compare May 7, 2026 19:58
Comment thread nix/devShell.nix
@m4r1vs m4r1vs force-pushed the main branch 3 times, most recently from 3e5012d to cad81ad Compare May 7, 2026 20:28
m4r1vs added 4 commits May 7, 2026 22:45
- Define a development shell in ./nix/devShell.nix
- Use nix-community/bun2nix to package hunk
Having bun install itself was causing weird issues. I've never seen it
installed as a nodjs package anywhere else so I went ahead and removed
it.
Comment thread package.json
@elucid
Copy link
Copy Markdown
Member

elucid commented May 8, 2026

Hey @m4r1vs (cc @emrtnn). Thanks for the PRs and for the discussion. I've looked at both this and #230. #230's approach is simpler but as you've both discovered, the fixed node_modules hash is platform dependent. I think this PR's approach is more nix-native and more maintainable.

Having said that, you'll need to make some fixes before it can be merged:

  1. don't remove bun. hunk uses bun throughout the source/build/dev workflow. a number of things would have to change if it was removed. we can debate about whether the project should use bun, but not within a PR to add nix support
  2. make the nix build use nix's bun e.g. ${bun}/bin/bun build --compile ...
  3. the postinstall hook is too invasive, remove it. we don't need to regenerate nix files on every install. add a "nix:update-lock" script instead.
  4. don't run unnecessary lifecycle hooks (prepare / simple-git-hooks) during nix builds
  5. fix the flake dev shell shape. it should expose devShells.${system}.default not a derivation directly at devShells.${system}
  6. add package metadata similar to what feat(nix): add flake to support nix builds #230 has
  7. the nix package build and basic bun workflows don't need nodejs. but if this PR exposes nix develop as a contributor shell, it should match CONTRIBUTING.md and include nodejs too. otherwise, omit the devShell.

@m4r1vs
Copy link
Copy Markdown
Author

m4r1vs commented May 8, 2026

Thanks for the feedback! Appreciate it!

  1. When bun is installed as a nodejs dependency, bun2nix does not work. I'm happy to open an issue at their repository but I also feel weird about having multiple different bun binaries potentially in conflict (one in node_modules, one from nix and then one often globally installed -- be it through homebrew, mise or bun's install script). Wouldn't it make sense to remove bun as a nodejs dependency and just list it instead of/together with nodejs as a requirement? I mean, we use bun.lock instead of package.lock. If we used the latter, we could make everything much simpler by using fetchNpmDeps directly from nixpkgs.
  2. It should already default to the bun binary in nixpkgs within the nix builder, but I'll check.
  3. Will remove that, 👍 Removing this could also fix 1) because bun2nix was failing as a postinstall script -- not within the build itself.
  4. Yes, thanks
  5. 👍
  6. 👍

@m4r1vs
Copy link
Copy Markdown
Author

m4r1vs commented May 8, 2026

regarding 3) I think it does not run them by default (see here) but I'll make it explicit

@m4r1vs
Copy link
Copy Markdown
Author

m4r1vs commented May 8, 2026

@elucid I think I've been able to address all your points:

  • Added bun to deps, removed bun2nix as dependency and from postinstall. Now it can be run using bun run nix:update-lock. commit
  • Added CI/CD check to make sure nix lock is up to date with helpful error (automation probably overkill?) commit
  • Added nodejs to devShell
  • Fixed devShell shape
  • Made skipping lifecycle hooks explicit
  • Added package metadata (copied from feat(nix): add flake to support nix builds #230 ) commit

Cheers!

I think the removal of postinstall bun2nix should also fix the failed pipeline

@m4r1vs
Copy link
Copy Markdown
Author

m4r1vs commented May 8, 2026

I've noticed that hunk skill path is not working when installed through nix. It walks up from the Hunk binary location so I fixed it by copying SKILLS to $out.

A more idiomatic way would be to copy it to $out/share but that would mean, we'd have to modify src/core/paths.ts to look for the Skills in that directory possible breaking non-nix installs.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Would you be able to add a Nix Flake?

2 participants