stdenv: pURL implementation - fix checkMeta#453291
stdenv: pURL implementation - fix checkMeta#453291h0nIg wants to merge 2 commits intoNixOS:masterfrom
Conversation
7cf7193 to
65be3b7
Compare
|
This does not fix all cases that broke due to the change, it still breaks on |
fixed. even though it was advised in matrix that tryEval might not catch everything, it matches this exact case, so we should add it |
|
Tested again and can confirm that this now works with everything that is currently listed on |
|
Unfortunately, I am lacking ideas how to fix this properly on the spot, thus I would like to proceed with the revert in #453322. |
@wolfgangwalther a second approach was highlighted by @dramforever in matrix, this looked odd in the beginning, but maybe this is a better approach. would you feel more comfortable with this? |
This would still not allow me to convert all of |
with the current approach in this PR you can json-ify
no, you are right. but lets summarize to get an overview: there are 2 cases we hit:
there are 6 options:
my favorite is 2 + 6. "throws" is commonly used on platforms where there is no support, e.g. similar to #420442 - tryEval catches this exact scenario With this in place, we protect against further problems, still fulfill the request to have a general support |
@jopejoe1 can you elaborate how you tested this? this would create more confidence |
It's not because regressions always require immediate revert, but because
|
I build the release tarball with #451424 so that it checks all meta attrs of all packages included in the packages.json file and not silently ignores errors making it possible to check those. |
Very much agree here. |
|
Closing because #453322 was merged. |
#421125 introduced a problem with checkMeta=true for nixpkgs-review. checkMeta defaults to false in general.
The scenario happened for thunderbird-bin in combination with an unsupported platform.
This PR prevents any evaluation which might be problematic
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.