stdenv: re-add meta.repository (resurrected) #301947
Conversation
github-actions
bot
added
8.has: documentation
6.topic: stdenv
There was a problem hiding this comment.
As with its predecessor, I think the design of this (not any particular line of code in this PR, but the dependency of meta on src) needs to be discussed more before this PR is merged. I'll withdraw my objection if it is discussed and I'm outvoted or convinced otherwise, but there's no need for haste here. We should take the time to hash it out in #293838.
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/when-should-meta-attributes-be-computed/42833/1 |
pkgs/stdenv/generic/check-meta.nix
Outdated
| @@ -454,6 +464,15 @@ let | |||
| hasOutput = out: builtins.elem out outputs; | |||
| in | |||
| { | |||
| # get the default value for the meta.repository field. | |||
| # the fetchFrom* fetchers set src.meta.homepage | |||
| # NOTE: this will fail if src fails to eval, in that case either set meta.repository manually to prevent this default from being evaluated, or just make sure src doesn't fail to eval. | |||
There was a problem hiding this comment.
Have you tried builtins.tryEval to catch the src evaluation errors and just put nothing in it?
There was a problem hiding this comment.
builtins.tryEval only catches throw errors. this note was orignally made when packages enconering abort-class errors in src was fairly common (at the time this was causing eval to fail), although both of these issues have been fixed since then.
There was a problem hiding this comment.
abort-class errors in evaluations are abnormal. Throwing in derivations is the only thing allowed to make evaluation fail.
Doing src = { "x86_64-linux" = ...; }.${builtins.currentSystem} is a mistake in nixpkgs, it requires at least:
src = { ... }.${builtins.currentSystem} or (throw "Source not available on {...}")
Thus, tryEval is the only thing you really really need in checkMeta for infaillible evaluations, all the other errors are bugs in nixpkgs and must be fixed as such because Hydra won't catch them neither.
pkgs/stdenv/generic/check-meta.nix
Outdated
| findFirst | ||
| filter | ||
| head |
There was a problem hiding this comment.
| findFirst | |
| filter | |
| head | |
| filter | |
| findFirst | |
| head |
wrapping in median of 11 runs (only really matters for cpu time, everything else is stable i think). using change 253c0db and x86_64-linux check meta tests baseline is change 253c0db, prlist11 is with this PR, prlist15 and prlist16 use tryEval
then running with NIX_SHOW_STATS=1 NIX_SHOW_STATS_PATH=results/baseline-stats$i.json nix eval -f - --show-trace <<EOF
with import ./.{}; ((import ./out-paths.nix) pkgs)
EOF
prlist11.diffdiff --git a/pkgs/stdenv/generic/check-meta.nix b/pkgs/stdenv/generic/check-meta.nix
index 1cd1ae6dd72e..f1f486b115a8 100644
--- a/pkgs/stdenv/generic/check-meta.nix
+++ b/pkgs/stdenv/generic/check-meta.nix
@@ -10,7 +10,9 @@ let
concatMapStrings
concatMapStringsSep
concatStrings
+ filter
findFirst
+ head
isDerivation
length
concatMap
@@ -39,6 +41,11 @@ let
toPretty
;
+ unlist = list:
+ if length list == 1
+ then head list
+ else list;
+
# If we're in hydra, we can dispense with the more verbose error
# messages and make problems easier to spot.
inHydra = config.inHydra or false;
@@ -303,6 +310,10 @@ let
(listOf str)
str
];
+ repository = union [
+ (listOf str)
+ str
+ ];
downloadPage = str;
changelog = union [
(listOf str)
@@ -452,8 +463,14 @@ let
let
outputs = attrs.outputs or [ "out" ];
hasOutput = out: builtins.elem out outputs;
- in
- {
+ in {
+ repository =
+ if attrs ? src.meta.homepage
+ then attrs.src.meta.homepage
+ else if attrs ? srcs && isList attrs.srcs
+ then unlist (map (src: src.meta.homepage) (filter (src: src ? meta.homepage) attrs.srcs))
+ else [];
+
# `name` derivation attribute includes cross-compilation cruft,
# is under assert, and is sanitized.
# Let's have a clean always accessible version here.prlist15.diffdiff --git a/pkgs/stdenv/generic/check-meta.nix b/pkgs/stdenv/generic/check-meta.nix
index bcb2ca249ddf..18764e52a319 100644
--- a/pkgs/stdenv/generic/check-meta.nix
+++ b/pkgs/stdenv/generic/check-meta.nix
@@ -10,7 +10,9 @@ let
concatMapStrings
concatMapStringsSep
concatStrings
+ filter
findFirst
+ head
isDerivation
length
concatMap
@@ -39,6 +41,11 @@ let
toPretty
;
+ unlist = list:
+ if length list == 1
+ then head list
+ else list;
+
# If we're in hydra, we can dispense with the more verbose error
# messages and make problems easier to spot.
inHydra = config.inHydra or false;
@@ -303,6 +310,10 @@ let
(listOf str)
str
];
+ repository = union [
+ (listOf str)
+ str
+ ];
downloadPage = str;
changelog = union [
(listOf str)
@@ -454,8 +465,15 @@ let
let
outputs = attrs.outputs or [ "out" ];
hasOutput = out: builtins.elem out outputs;
- in
- {
+ in {
+ repository = let res = builtins.tryEval (
+ if attrs ? src.meta.homepage
+ then attrs.src.meta.homepage
+ else if attrs ? srcs && isList attrs.srcs
+ then unlist (map (src: src.meta.homepage) (filter (src: src ? meta.homepage) attrs.srcs))
+ else []);
+ in if res.success then res.value else [];
+
# `name` derivation attribute includes cross-compilation cruft,
# is under assert, and is sanitized.
# Let's have a clean always accessible version here.prlist16.diffdiff --git a/pkgs/stdenv/generic/check-meta.nix b/pkgs/stdenv/generic/check-meta.nix
index bcb2ca249ddf..a31c02ec225c 100644
--- a/pkgs/stdenv/generic/check-meta.nix
+++ b/pkgs/stdenv/generic/check-meta.nix
@@ -10,7 +10,9 @@ let
concatMapStrings
concatMapStringsSep
concatStrings
+ filter
findFirst
+ head
isDerivation
length
concatMap
@@ -39,6 +41,16 @@ let
toPretty
;
+ unlist = list:
+ if length list == 1
+ then head list
+ else list;
+
+ valOrEmpty = res:
+ if res.success
+ then res.value
+ else [];
+
# If we're in hydra, we can dispense with the more verbose error
# messages and make problems easier to spot.
inHydra = config.inHydra or false;
@@ -303,6 +315,10 @@ let
(listOf str)
str
];
+ repository = union [
+ (listOf str)
+ str
+ ];
downloadPage = str;
changelog = union [
(listOf str)
@@ -454,8 +470,14 @@ let
let
outputs = attrs.outputs or [ "out" ];
hasOutput = out: builtins.elem out outputs;
- in
- {
+ in {
+ repository = valOrEmpty (builtins.tryEval (
+ if attrs ? src.meta.homepage
+ then attrs.src.meta.homepage
+ else if attrs ? srcs && isList attrs.srcs
+ then unlist (map (src: src.meta.homepage) (filter (src: src ? meta.homepage) attrs.srcs))
+ else []));
+
# `name` derivation attribute includes cross-compilation cruft,
# is under assert, and is sanitized.
# Let's have a clean always accessible version here. |
lolbinarycat
force-pushed
the
meta.repository-v2
branch
from
a95b4fb
to
16139a4
Compare
|
added |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
rhendric
dismissed
their stale review
I still don't endorse this design but Raito's approval is sufficient for me to get out of the way.
Description of changes
previous PR (#300313) got messed up due to me trying to do too many fancy git things at once (mainly using shallow pulls and amending commits)
i've updated the documentation to match the new behavior, and have added a different example
@a-n-n-a-l-e-e @AndersonTorres @nyabinary for participating in the previous PR
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 馃憤 reaction to pull requests you find important.