Up to now, existing symlinks were replaced by removing the existing link
and then creating a new one. This left a window where the link did not
exist, which would break cases where the hook was running concurrently
with other things using node_modules.
In my case, I would run multiple webpack builds at once like:
nix develop -c webpack build --mode production
nix develop -c webpack build --mode development
If one of the shells was ready faster than the other, webpack would
start running, then the second would run the linkNodeModules hook, and
the earlier webpack build would run into missing files as the second
linkNodeModules hook removed symlinks to replace them.
This change mitigates this race condition in two ways:
- Not replacing the symlink if it already points to the right target
(this also avoids superfluous filesystem writes)
- Replacing the symlink atomically, using a rename,
otherwise. The symlink was replaced atomically prior to
db7050bb88 as well, but the name of the
temporary symlink was always the same, which led to a similar race
condition. This change reintroduces the atomic replacement, but adds
the PID to the temporary filename in order to avoid conflicting with
other instances of the hook running concurrently.
It's currently possible to run into a race condition when entering the same development environment concurrently:
```
❯ git fetch; jj rebase -b 'all:mutable() & mine()' -d main@origin --skip-emptied
Rebased 4 commits onto destination
Abandoned 1 newly emptied commits
Working copy (@) now at: tnyknvqt 93e36def (empty) (no description set)
Parent commit (@-) : wnqxqyyl e0983a05 main@origin | Increase limit of max number of words
Added 0 files, modified 8 files, removed 0 files
direnv: loading ~/dev/REDACTED/.envrc
direnv: loading https://raw.githubusercontent.com/nix-community/nix-direnv/3.0.4/direnvrc (sha256-DzlYZ33mWF/Gs8DDeyjr8mnVmQGx7ASYqA5WlxwvBG4=)
Executing linkNodeModulesHook
node:internal/fs/promises:782
return await PromisePrototypeThen(
^
Error: ENOENT: no such file or directory, rename 'node_modules/.bin-nix-hook-temp' -> 'node_modules/.bin'
at async Object.rename (node:internal/fs/promises:782:10)
at async /nix/store/ps9ivjjxzi0fks67j6vd4gbw5dcnhp0w-link-node-modules.js:84:7
at async Promise.all (index 0)
at async main (/nix/store/ps9ivjjxzi0fks67j6vd4gbw5dcnhp0w-link-node-modules.js:58:3) {
errno: -2,
code: 'ENOENT',
syscall: 'rename',
path: 'node_modules/.bin-nix-hook-temp',
dest: 'node_modules/.bin'
}
Node.js v20.17.0
Finished executing linkNodeModulesShellHook
```
This change removes the intermediate file creation and simply tries to create a symlink directly.
If the target `node_modules/foo` already exists we unlink it and try to create the symlink again.
`importNpmLock.buildNodeModules` returns a derivation with a pre-built `node_modules` directory, as imported by `importNpmLock`.
This is to be used together with `importNpmLock.hooks.linkNodeModulesHook` to facilitate `nix-shell`/`nix develop` based development workflows:
```nix
pkgs.mkShell {
packages = [
importNpmLock.hooks.linkNodeModulesHook
nodejs
];
npmDeps = importNpmLock.buildNodeModules {
npmRoot = ./.;
inherit nodejs;
};
}
```
will create a development shell where a `node_modules` directory is created & packages symlinked to the Nix store when activated.
This code is adapted from https://github.com/adisbladis/buildNodeModules
