fetchPnpmDeps: don't fail on empty lockfile#513736
Conversation
|
Also, I should note that this does not bite people with fetcherVersion = 1, only 2 or 3, hence it has an unfortunate interaction with #513215: without fixing this, people will be being nagged about a deprecation, but may not have a simple way to bump the fetcher version and squelch it. |
|
I was working on adding tests in this PR: #505103 |
Scrumplex
left a comment
There was a problem hiding this comment.
Changes LGTM. Due to this fixing a build failure, I think we can safely assume that this won't break any existing hashes.
I'd say this is good to merge even without a test at the moment, but if you want to add one based on my previous comment, I am happy to re-review then :D
|
Ta - that was enough to pattern off, and now there is a test: validated that it fails with the previous code, and that it's fixed by the PR. |
741240b to
9c9958c
Compare
If the lockfile is empty, no files will be put in the output directory. In this case, the find invocations produce an empty list of results. xargs' default behaviour is to always invoke its command at least once; if its input is empty, then its command will not be passed any filenames. This produces an invocation like `chmod 555`. chmod, however, fails in this case, expecting an operand. So we need to tell xargs not to invoke its command at all in the case of empty input.
9c9958c to
3795543
Compare
|
Thank you! |
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release-25.11
git worktree add -d .worktree/backport-513736-to-release-25.11 origin/release-25.11
cd .worktree/backport-513736-to-release-25.11
git switch --create backport-513736-to-release-25.11
git cherry-pick -x 37955437a78e6effe93022d70a131ff88e92a08e |
If the lockfile is empty, no files will be put in the output directory. In this case, the find invocations produce an empty list of results. xargs' default behaviour is to always invoke its command at least once; if its input is empty, then its command will not be passed any filenames. This produces an invocation like
chmod 555, with no further operands. chmod, however, fails in this case, expecting an operand. So we need to tell xargs not to invoke its command at all in the case of empty input.I went looking for some examples of fetchPnpmDeps tests to pattern off, since this is an almost ideal bug to have a regression test for (literally no network access needed!), but I couldn't find any; figuring out a good way to set up tests for it is more effort than I want to spend on this, so no test is being added. If there are some and I just missed them, I'm happy to amend the PR and add to their number. In their absence, I offer an assurance that I have manually tested it and found the patch fixed the failure I saw (albeit rebased on a commit from a few days ago instead of the present master).
Because this is only fixing a case that was failing builds before, this shouldn't change any previously-working output and hence isn't a fetcher version change. (I think, anyway!)
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.