Skip to content

Commit 2917d72

Browse files
committed
[v2][bugfix] Relax package dependencies version bounds
Relaxing dependencies version bounds for github refs. If we happen to leave them as is, it'll force NPM to checkout the remote repo to get the actual ref. At this point, we already pinned everything through the "resolved" field, there's no point in keeping the unresolved reference in dependencies. We can relax those to "*".
1 parent 24b9619 commit 2917d72

File tree

3 files changed

+53
-7
lines changed

3 files changed

+53
-7
lines changed

internal-v2.nix

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@ rec {
3939
)
4040
else false;
4141

42+
# Description: Checks if a string looks like a valid github
43+
# reference who do not have a rev. You shouldn't find any of those
44+
# in a "resolved" field. It's however possible to find them in a
45+
# "dependencies" part of a package.
46+
# Type: String -> Boolean
47+
isGitHubRefWithoutRev = str:
48+
let
49+
parts = builtins.split "[:#/@]" str;
50+
partsLen = builtins.length parts;
51+
in
52+
partsLen == 5 && builtins.elemAt parts 0 == "github";
53+
4254
# Description: Takes a string of the format
4355
# "git+ssh://git@github.com/owner/repo.git#revision",
4456
# "git@github.com/owner/repo.git#revision" or
@@ -251,6 +263,12 @@ rec {
251263
let
252264
name = genericPackageName raw_name;
253265
defaultedIntegrity = if spec ? integrity then spec.integrity else null;
266+
# Relaxing dependencies version bounds: it could be a GitHub
267+
# ref, forcing NPM to checkout the remote repo to get the actual
268+
# version.
269+
# We already pinned everything through the "resolved", we can
270+
# relax those.
271+
patchDependencies = deps: lib.mapAttrs (_n: dep: if isGitHubRef dep || isGitHubRefWithoutRev dep then "*" else dep) deps;
254272
patchedResolved =
255273
if (!isGitHubRef spec.resolved)
256274
then makeUrlSource sourceOptions name spec.version spec.resolved defaultedIntegrity
@@ -271,7 +289,7 @@ rec {
271289
in
272290
spec // {
273291
inherit (patchedResolved) resolved integrity;
274-
};
292+
} // lib.optionalAttrs (spec ? dependencies) { dependencies = (patchDependencies spec.dependencies); };
275293

276294
genericPackageName = name:
277295
(lib.last (lib.strings.splitString "node_modules/" name));

tests/tests-v2/parse-github-ref.nix

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,14 @@ in
4848
expr = i.isGitHubRef "git@github.com/znerol/libxmljs.git#0517e063347ea2532c9fdf38dc47878c628bf0ae";
4949
expected = true;
5050
};
51+
52+
testIsGitHubRefWithoutRef = {
53+
expr = i.isGitHubRefWithoutRev "github:frozeman/bignumber.js-nolookahead";
54+
expected = true;
55+
};
56+
57+
testIsNotGitHubRefWithoutRef = {
58+
expr = i.isGitHubRefWithoutRev "github:znerol/libxmljs#0517e063347ea2532c9fdf38dc47878c628bf0ae";
59+
expected = false;
60+
};
5161
})

tests/tests-v2/patch-package.nix

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ let
1515
resolved = "git+ssh://git@github.com/tmcw/leftpad.git#db1442a0556c2b133627ffebf455a78a1ced64b9";
1616
version = "0.0";
1717
};
18+
specWithGhDependencies = {
19+
resolved = "git+ssh://git@github.com/tmcw/leftpad.git#db1442a0556c2b133627ffebf455a78a1ced64b9";
20+
version = "0.0";
21+
dependencies = {
22+
utf8 = "^2.1.1";
23+
"bignumber.js" = "github:frozeman/bignumber.js-nolookahead";
24+
};
25+
};
1826
in
1927
(testLib.runTests {
2028
testGhSourceRef = {
@@ -50,8 +58,8 @@ in
5058
integrity = "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==";
5159
});
5260
in
53-
res.version == "4.0.0" && lib.hasPrefix "file:///nix/store" res.resolved && res.integrity == "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==";
54-
expected = true;
61+
[ (res.version == "4.0.0") (lib.hasPrefix "file:///nix/store" res.resolved) (res.integrity == "sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==") ];
62+
expected = [ true true true ];
5563
};
5664
testPatchDepGithub = {
5765
expr =
@@ -63,9 +71,19 @@ in
6371
license = "BSD-3-Clause";
6472
});
6573
in
66-
lib.hasPrefix "file:///nix/store" res.resolved && res.integrity == null;
67-
expected = true;
74+
[ (lib.hasPrefix "file:///nix/store" res.resolved) (res.integrity == null) ];
75+
expected = [ true true ];
76+
};
77+
testSpecWithGhDependencies = {
78+
expr =
79+
let
80+
result = (i.patchPackage noSourceOptions "leftpad" specWithGhDependencies);
81+
in
82+
[
83+
(lib.hasPrefix "file:///nix/store" result.resolved)
84+
(result.dependencies.utf8 == "^2.1.1")
85+
(result.dependencies."bignumber.js" == "*")
86+
];
87+
expected = [ true true true ];
6888
};
69-
70-
7189
})

0 commit comments

Comments
 (0)