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

Allow patching dependency sources #151

Merged
merged 5 commits into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions API.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ The `node_modules` function takes an attribute set with the following attributes
- **nodejs** *(default `nixpkgs.nodejs`, which is the Active LTS version)*: Node.js derivation to use
- **preInstallLinks** *(default `{}`)*: Map of symlinks to create inside npm dependencies in the `node_modules` output (See [Concepts](#concepts) for details).
- **githubSourceHashMap** *(default `{}`)*: Dependency hashes for evaluation in restricted mode (See [Concepts](#concepts) for details).
- **sourceOverrides** *(default `{}`)*: Derivation attributes to apply to sources, allowing patching (See the [source derivation overrides](#source-derivation-overrides) concept for details)

#### Notes
- You may provide additional arguments accepted by `mkDerivation` all of which are going to be passed on.
Expand Down Expand Up @@ -114,3 +115,32 @@ npmlock2nix.build {
};
}
```

### Source derivation overrides

`node_modules` takes a `sourceOverrides` argument, which allows you to modify the source derivations of individual npm packages you depend on, mainly useful for adding Nix-specific fixes to packages. This could be used for patching interpreter or paths, or to replace vendored binaries with ones provided by Nix.

The `sourceOverrides` argument expects an attribute set mapping npm package names to a function describing the modifications of that package. Each function receives an attribute set as a first argument, containing either a `version` attribute if the version is known, or a `github = { org, repo, rev, ref }` attribute if the package is fetched from GitHub. These values can be used to have different overrides depending on the version. The function receives another argument which is the derivation of the fetched source, which can be modified using `.overrideAttrs`. The fetched source mainly runs the [patch phase](https://nixos.org/manual/nixpkgs/stable/#ssec-patch-phase), so of particular interest are the `patches` and `postPatch` attributes, in which `patchShebangs` can be called. Note that `patchShebangs` can only patch shebangs to binaries accessible in the derivation, which you can extend with `buildInputs`. For convenience, the correct version of `nodejs` is always included in `buildInputs`.

```nix
npmlock2nix.node_modules {
sourceOverrides = {
# sourceInfo either contains:
# - A version attribute
# - A github = { org, repo, rev, ref } attribute for GitHub sources
package-name = sourceInfo: drv: drv.overrideAttrs (old: {
buildInputs = [ somePackage ];
patches = [ somePatch ];
postPatch = ''
some script
'';
# ...
})

# Example
node-pre-gyp = sourceInfo: drv: drv.overrideAttrs (old: {
postPatch = "patchShebangs bin";
Copy link
Collaborator

Choose a reason for hiding this comment

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

The interface here is a very leaky abstraction. We just plug some additional variables into the runCommand but only some will ever actually have an impact. I'd rather prefer an interface that isn't passing everything through from the user but defines a stricter interface.

The alternative is to provide an overrideAttrs interface here instead.

I prefer the first approach as it comes with a more limited set of features that we have to take care of.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I think we can limit this to only patches and postPatch for now, but it could be extended with support for other attributes in the future (for now other attributes should give an error)

Copy link
Contributor Author

@infinisil infinisil Mar 17, 2022

Choose a reason for hiding this comment

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

After another thought, I think it's probably better to just support all attributes, both to keep it simpler, but also because people might have different needs. Notably there's also prePatch, you can add dependencies in buildInputs as needed by patchShebangs, you may want to set NIX_DEBUG for debugging, you may want to set sourceRoot to only get a subdirectory, you may want to set dontUnpack to preserve the original .tgz and patch that with tar directly, you may want to set environment variables.

I pushed support for this in this PR now, along with documentation for it

});
};
}
```
203 changes: 158 additions & 45 deletions internal.nix

Large diffs are not rendered by default.

13 changes: 13 additions & 0 deletions tests/examples-projects/source-patching/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions tests/examples-projects/source-patching/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "source-patching",
"version": "1.0.0",
"description": "",
"main": "index.js",
"author": "",
"license": "ISC",
"dependencies": {
"custom-hello-world": "^1.0.0"
}
}
25 changes: 25 additions & 0 deletions tests/examples-projects/source-patching/shell.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{ npmlock2nix }:
npmlock2nix.shell {
src = ./.;
node_modules_attrs = {
sourceOverrides = {
custom-hello-world = sourceInfo: drv: drv.overrideAttrs (old: {
patches = builtins.toFile "custom-hello-world.patch" ''
diff --git a/lib/index.js b/lib/index.js
index 1f66513..64391a7 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -21,7 +21,7 @@ function generateHelloWorld({ comma, exclamation, lowercase }) {
if (comma)
helloWorldStr += ',';

- helloWorldStr += ' World';
+ helloWorldStr += ' Nix';

if (exclamation)
helloWorldStr += '!';
'';
});
};
};
}
9 changes: 9 additions & 0 deletions tests/integration-tests/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -284,4 +284,13 @@ testLib.makeIntegrationTests {
'';
};
};

source-patching = {
description = "Source patching works";
shell = callPackage ../examples-projects/source-patching/shell.nix { };
command = ''
node -e 'console.log(require("custom-hello-world")({}));'
'';
expected = "Hello Nix\n";
};
}
5 changes: 4 additions & 1 deletion tests/lib.nix
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
# Reads a given file (either drv, path or string) and returns it's sha256 hash
hashFile = filename: builtins.hashString "sha256" (builtins.readFile filename);

noGithubHashes = (_: null);
noSourceOptions = {
sourceHashFunc = _: null;
nodejs = null;
};

runTests = tests:
let
Expand Down
6 changes: 3 additions & 3 deletions tests/make-github-source.nix
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{ lib, npmlock2nix, testLib }:
let
inherit (testLib) noGithubHashes;
inherit (testLib) noSourceOptions;
i = npmlock2nix.internal;

testDependency = {
Expand All @@ -12,7 +12,7 @@ in
testSimpleCase = {
expr =
let
version = (i.makeGithubSource noGithubHashes "leftpad" testDependency).version;
version = (i.makeGithubSource noSourceOptions "leftpad" testDependency).version;
in
lib.hasPrefix "file:///nix/store" version;
expected = true;
Expand All @@ -21,7 +21,7 @@ in
testDropsFrom = {
expr =
let
dep = i.makeGithubSource noGithubHashes "leftpad" testDependency;
dep = i.makeGithubSource noSourceOptions "leftpad" testDependency;
in
dep ? from;
expected = false;
Expand Down
5 changes: 4 additions & 1 deletion tests/make-source.nix
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
{ testLib, npmlock2nix }:
let
i = npmlock2nix.internal;
f = builtins.throw "Shouldn't be called";
f = {
sourceHashFunc = builtins.throw "Shouldn't be called";
nodejs = null;
};
in
testLib.runTests {
testMakeSourceRegular = {
Expand Down
28 changes: 14 additions & 14 deletions tests/patch-lockfile.nix
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
{ npmlock2nix, testLib, lib }:
let
inherit (testLib) noGithubHashes;
inherit (testLib) noSourceOptions;
in
testLib.runTests {

testPatchDependencyHandlesGitHubRefsInRequires = {
expr =
let
libxmljsUrl = (npmlock2nix.internal.patchDependency noGithubHashes "test" {
libxmljsUrl = (npmlock2nix.internal.patchDependency [ ] noSourceOptions "test" {
version = "github:tmcw/leftpad#db1442a0556c2b133627ffebf455a78a1ced64b9";
from = "github:tmcw/leftpad#db1442a0556c2b133627ffebf455a78a1ced64b9";
integrity = "sha512-8/UvHFG90J4O4QNRzb0jB5Ni1QuvuB7XFTLfDMQnCzAsFemF29VKnNGUESFFcSP/r5WWh/PMe0YRz90+3IqsUA==";
requires = {
libxmljs = "github:znerol/libxmljs#0517e063347ea2532c9fdf38dc47878c628bf0ae";
};
}
).requires.libxmljs;
).result.requires.libxmljs;
in
lib.hasPrefix builtins.storeDir libxmljsUrl;
expected = true;
};

testBundledDependenciesAreRetained = {
expr = npmlock2nix.internal.patchDependency noGithubHashes "test" {
expr = (npmlock2nix.internal.patchDependency [ ] noSourceOptions "test" {
bundled = true;
integrity = "sha1-hrGk3k+s4YCsVFqD8VA1I9j+0RU=";
something = "bar";
dependencies = { };
};
}).result;
expected = {
bundled = true;
integrity = "sha1-hrGk3k+s4YCsVFqD8VA1I9j+0RU=";
Expand All @@ -37,18 +37,18 @@ testLib.runTests {
};

testPatchLockfileWithoutDependencies = {
expr = (npmlock2nix.internal.patchLockfile noGithubHashes ./examples-projects/no-dependencies/package-lock.json).dependencies;
expr = (npmlock2nix.internal.patchLockfile noSourceOptions ./examples-projects/no-dependencies/package-lock.json).result.dependencies;
expected = { };
};

testPatchDependencyDoesntDropAttributes = {
expr = npmlock2nix.internal.patchDependency noGithubHashes "test" {
expr = (npmlock2nix.internal.patchDependency [ ] noSourceOptions "test" {
a = 1;
foo = "something";
resolved = "https://examples.com/something.tgz";
integrity = "sha1-00000000000000000000000+0RU=";
dependencies = { };
};
}).result;
expected = {
a = 1;
foo = "something";
Expand All @@ -59,7 +59,7 @@ testLib.runTests {
};

testPatchDependencyPatchesDependenciesRecursively = {
expr = npmlock2nix.internal.patchDependency noGithubHashes "test" {
expr = (npmlock2nix.internal.patchDependency [ ] noSourceOptions "test" {
a = 1;
foo = "something";
resolved = "https://examples.com/something.tgz";
Expand All @@ -68,7 +68,7 @@ testLib.runTests {
resolved = "https://examples.com/somethingelse.tgz";
integrity = "sha1-00000000000000000000000+00U=";
};
};
}).result;

expected = {
a = 1;
Expand All @@ -85,7 +85,7 @@ testLib.runTests {
testPatchLockfileTurnsUrlsIntoStorePaths = {
expr =
let
deps = (npmlock2nix.internal.patchLockfile noGithubHashes ./examples-projects/single-dependency/package-lock.json).dependencies;
deps = (npmlock2nix.internal.patchLockfile noSourceOptions ./examples-projects/single-dependency/package-lock.json).result.dependencies;
in
lib.count (dep: lib.hasPrefix "file:///nix/store/" dep.resolved) (lib.attrValues deps);
expected = 1;
Expand All @@ -94,19 +94,19 @@ testLib.runTests {
testPatchLockfileTurnsGitHubUrlsIntoStorePaths = {
expr =
let
leftpad = (npmlock2nix.internal.patchLockfile noGithubHashes ./examples-projects/github-dependency/package-lock.json).dependencies.leftpad;
leftpad = (npmlock2nix.internal.patchLockfile noSourceOptions ./examples-projects/github-dependency/package-lock.json).result.dependencies.leftpad;
in
lib.hasPrefix ("file://" + builtins.storeDir) leftpad.version;
expected = true;
};

testConvertPatchedLockfileToJSON = {
expr = builtins.typeOf (builtins.toJSON (npmlock2nix.internal.patchLockfile noGithubHashes ./examples-projects/nested-dependencies/package-lock.json)) == "string";
expr = builtins.typeOf (builtins.toJSON (npmlock2nix.internal.patchLockfile noSourceOptions ./examples-projects/nested-dependencies/package-lock.json).result) == "string";
expected = true;
};

testPatchedLockFile = {
expr = testLib.hashFile (npmlock2nix.internal.patchedLockfile noGithubHashes ./examples-projects/nested-dependencies/package-lock.json);
expr = testLib.hashFile (npmlock2nix.internal.patchedLockfile noSourceOptions ./examples-projects/nested-dependencies/package-lock.json).result;
expected = "980323c3a53d86ab6886f21882936cfe7c06ac633993f16431d79e3185084414";
};

Expand Down
7 changes: 3 additions & 4 deletions tests/patch-packagefile.nix
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
{ lib, npmlock2nix, testLib }:
let
inherit (testLib) noGithubHashes;
i = npmlock2nix.internal;
in
(testLib.runTests {
testTurnsGitHubRefsToWildcards = {
expr = (npmlock2nix.internal.patchPackagefile noGithubHashes ./examples-projects/github-dependency/package.json).dependencies.leftpad;
expr = (npmlock2nix.internal.patchPackagefile ./examples-projects/github-dependency/package.json).dependencies.leftpad;
expected = "*";
};
testHandlesBranches = {
expr = (npmlock2nix.internal.patchPackagefile noGithubHashes ./examples-projects/github-dependency-branch/package.json).dependencies.leftpad;
expr = (npmlock2nix.internal.patchPackagefile ./examples-projects/github-dependency-branch/package.json).dependencies.leftpad;
expected = "*";
};
testHandlesDevDependencies = {
expr = (npmlock2nix.internal.patchPackagefile noGithubHashes ./examples-projects/github-dev-dependency/package.json).devDependencies.leftpad;
expr = (npmlock2nix.internal.patchPackagefile ./examples-projects/github-dev-dependency/package.json).devDependencies.leftpad;
expected = "*";
};
})