| 20 Dec 2023 |
 hexa | you can check the dashboard in the room topic | 19:49:27 |
| 21 Dec 2023 |
|  symphorien joined the room. | 20:25:06 |
| 23 Dec 2023 |
|  raitobezarius changed their display name from raitobezarius to raitobezarius (DECT 2128). | 22:22:43 |
| 25 Dec 2023 |
|  trofi joined the room. | 14:43:18 |
| trofi | I would expect ofborg to add 2 listed maintainers to review nix_2_3 in https://github.com/NixOS/nixpkgs/pull/276696 . Do you know by chance why it did not add any reviewers? | 14:44:13 |
 raphi | i think because nix_2_3 is in aliases.nix so it doesn't get picked up in CI
nixVersions.nix_2_3 should work | 14:56:25 |
| trofi | Oh, that's tricky! Thank you! | 16:24:12 |
 Lily Foster | In reply to @raphi:tapesoftware.net
i think because nix_2_3 is in aliases.nix so it doesn't get picked up in CI
nixVersions.nix_2_3 should work yeah but commit message should just be for builds iirc. it uses changed paths/attrs from the outpath calculation iirc (though maybe it had other problems with determining the changed path there idk or maybe it is something alias heckery) | 16:59:51 |
| Lily Foster | if i remember, i'll double check the code later | 17:00:15 |
| Lily Foster | * yeah but commit message should just be for builds iirc. it uses changed paths/attrs from the outpath calculation iirc for maintainer pings (though maybe it had other problems with determining the changed path there idk or maybe it is something alias heckery) | 17:00:31 |
 Artturin | In reply to @lily:lily.flowers
yeah but commit message should just be for builds iirc. it uses changed paths/attrs from the outpath calculation iirc (though maybe it had other problems with determining the changed path there idk or maybe it is something alias heckery) The maintainers for all reverse feps aren't pinged | 17:01:04 |
| Lily Foster | In reply to @artturin:matrix.org
The maintainers for all reverse feps aren't pinged i guess that's true, i'm probably just misremembering then | 17:03:55 |
| Lily Foster | (i really thought it based it on files touched for some reason though rather than commit messages....) | 17:04:24 |
| Lily Foster | * (i really thought it based it on source files touched for some reason though rather than commit messages....) | 17:04:32 |
| Lily Foster | (idk am on mobile and also with family rn so i can't check code) | 17:04:49 |
| Artturin | Ofborg doesn't use the source names for anything IIRC | 17:06:03 |
| trofi | I changed it to nixVersions.nix_2_3 and ofborg still did not add anyone :( | 19:11:02 |
| 26 Dec 2023 |
| Lily Foster | In reply to @artturin:matrix.org
Ofborg doesn't use the source names for anything IIRC i'm looking at the ofborg code and it definitely just does maintainer pings for any changed outpaths that also had a file modified in the PR that corresponds to the derivations' meta.maintainers, src, pname, or version attr positions: https://github.com/NixOS/ofborg/blob/de415d372959b7e6fc6b2f6c95f0c21e5010348d/ofborg/src/maintainers.nix#L56-L64 | 01:31:56 |
| Lily Foster | (i.e. so it requires that 1. the package has an attr, 2. the outpath for that package attr has changed, and 3. the meta.maintainers, src, pname, or version attribute for that package must be defined in one of the files that was changed) | 01:32:59 |
| Lily Foster | In reply to @trofi:matrix.org
I changed it to nixVersions.nix_2_3 and ofborg still did not add anyone :( I'm trying to trace back why that didn't work here, because it definitely should have even with the original commit message | 01:33:41 |
| Lily Foster | * (i.e. so it requires that 1. the package has an attr on any supported arch, 2. the outpath for that package attr/arch has changed, and 3. the meta.maintainers, src, pname, or version attribute for that package must be defined in one of the files that was changed) | 01:34:56 |
| Lily Foster | Ah so the nix common function doesn't directly inherit the meta attrset so it doesn't think the attr pos for any of those 4 is in pkgs/tools/package-management/nix/default.nix (just pkgs/tools/package-management/nix/common.nix), it didn't think there were any relevant pings since none of the changed files "corresponded" to the changed attrs | 01:39:00 |
| Lily Foster | So I'm not sure this is a bug so much as an unfortunate consequence of ofborg relying on invisible magic and that magic being broken by the nix derivation refactor | 01:39:33 |
| Lily Foster | * So I'm not sure this is a bug so much as an unfortunate consequence of ofborg relying on invisible magic and that magic being broken by the nix derivation refactor that made that common function | 01:39:50 |
| Lily Foster | a minor refactor that is something like this would make the magic work again if you really wanted it to trofi:
diff --git a/pkgs/tools/package-management/nix/common.nix b/pkgs/tools/package-management/nix/common.nix
index 0ea47dd7e17c..6d62d8a31bfd 100644
--- a/pkgs/tools/package-management/nix/common.nix
+++ b/pkgs/tools/package-management/nix/common.nix
@@ -5,7 +5,7 @@
, hash ? null
, src ? fetchFromGitHub { owner = "NixOS"; repo = "nix"; rev = version; inherit hash; }
, patches ? [ ]
-, maintainers ? with lib.maintainers; [ eelco lovesegfault artturin ]
+, meta ? {}
}:
assert (hash == null) -> (src != null);
let
@@ -243,10 +243,10 @@ self = stdenv.mkDerivation {
'';
homepage = "https://nixos.org/";
license = licenses.lgpl2Plus;
- inherit maintainers;
+ maintainers = with lib.maintainers; [ eelco lovesegfault artturin ];
platforms = platforms.unix;
outputsToInstall = [ "out" ] ++ optional enableDocumentation "man";
mainProgram = "nix";
- };
+ } // meta;
};
in self
diff --git a/pkgs/tools/package-management/nix/default.nix b/pkgs/tools/package-management/nix/default.nix
index 4652ddb76a5b..e1d566ecabaa 100644
--- a/pkgs/tools/package-management/nix/default.nix
+++ b/pkgs/tools/package-management/nix/default.nix
@@ -157,7 +157,7 @@ in lib.makeExtensible (self: ({
patches = [
patch-monitorfdhup
];
- maintainers = with lib.maintainers; [ flokli raitobezarius ];
+ meta.maintainers = with lib.maintainers; [ flokli raitobezarius ];
}).override { boehmgc = boehmgc-nix_2_3; };
nix_2_10 = common {
| 01:53:44 |
| raitobezarius | In reply to @lily:lily.flowers
a minor refactor that is something like this would make the magic work again if you really wanted it to trofi:
diff --git a/pkgs/tools/package-management/nix/common.nix b/pkgs/tools/package-management/nix/common.nix
index 0ea47dd7e17c..6d62d8a31bfd 100644
--- a/pkgs/tools/package-management/nix/common.nix
+++ b/pkgs/tools/package-management/nix/common.nix
@@ -5,7 +5,7 @@
, hash ? null
, src ? fetchFromGitHub { owner = "NixOS"; repo = "nix"; rev = version; inherit hash; }
, patches ? [ ]
-, maintainers ? with lib.maintainers; [ eelco lovesegfault artturin ]
+, meta ? {}
}:
assert (hash == null) -> (src != null);
let
@@ -243,10 +243,10 @@ self = stdenv.mkDerivation {
'';
homepage = "https://nixos.org/";
license = licenses.lgpl2Plus;
- inherit maintainers;
+ maintainers = with lib.maintainers; [ eelco lovesegfault artturin ];
platforms = platforms.unix;
outputsToInstall = [ "out" ] ++ optional enableDocumentation "man";
mainProgram = "nix";
- };
+ } // meta;
};
in self
diff --git a/pkgs/tools/package-management/nix/default.nix b/pkgs/tools/package-management/nix/default.nix
index 4652ddb76a5b..e1d566ecabaa 100644
--- a/pkgs/tools/package-management/nix/default.nix
+++ b/pkgs/tools/package-management/nix/default.nix
@@ -157,7 +157,7 @@ in lib.makeExtensible (self: ({
patches = [
patch-monitorfdhup
];
- maintainers = with lib.maintainers; [ flokli raitobezarius ];
+ meta.maintainers = with lib.maintainers; [ flokli raitobezarius ];
}).override { boehmgc = boehmgc-nix_2_3; };
nix_2_10 = common {
please send the patch; looks good to me :( | 05:16:21 |
| trofi | I must say I expected $attr.meta.maintainers to Just Work :( | 07:50:13 |
| trofi | Filed https://github.com/NixOS/ofborg/issues/668 if that makes sense. | 10:03:50 |
| trofi | As a workaround WDYT of adding a pos attribute to the lookup list? I could sprinkle a few pos = allocations for nix edit to do the right thing. | 10:36:42 |
| trofi | * As a workaround WDYT of adding a pos attribute to the lookup list? I could sprinkle a few pos = annotations for nix edit to do the right thing. | 10:36:55 |
