| 11 Oct 2023 |
 @piegames:matrix.org | This is so great | 19:39:01 |
| @piegames:matrix.org | (Actually I unironically don't find it that bad, or at least not significantly worse than having to manually track attributes in all-packages.nix) | 19:39:40 |
 infinisil | I'd rather fix the core issue than swap out hacks for other hacks | 19:44:37 |
 Robert Hensing (roberth) | What's nice about the inherit is that the evaluator doesn't need to open and parse any extra files to figure out the attribute names | 21:11:25 |
| Robert Hensing (roberth) | probably no big deal for one file, but could be bad if it becomes a widespread pattern, particularly because there'd be no incentive to keep the file small | 21:12:15 |
| Robert Hensing (roberth) | * probably no big deal for one file, but could be bad if it becomes a widespread pattern, particularly because there'd be no incentive to keep those files small | 21:12:26 |
| 12 Oct 2023 |
| infinisil | https://github.com/NixOS/nixpkgs/pull/256792 is now ready to review, this is the next step towards migrating all packages | 00:31:45 |
| 13 Oct 2023 |
| infinisil | This is slowly breaking my brain, but I think I know what to do now.. https://github.com/NixOS/nixpkgs/issues/256788#issuecomment-1760610982 | 01:07:17 |
| Robert Hensing (roberth) | is it using pull_request_target? If not, maybe that helps? | 13:48:02 |
| Robert Hensing (roberth) | it runs in the context of the base | 13:48:12 |
| Robert Hensing (roberth) | I would assume they mean the tip of the base branch, but maybe that's exactly what you're running into | 13:48:38 |
| infinisil | Robert Hensing (roberth): It does use pull_request_target, but restarting the job doesn't seem to affect the workflow file used, even if the base branch changed it | 14:51:21 |
| infinisil | Here's my test: https://github.com/tweag/nixpkgs/pull/74
This PR was done against a base branch that had a working check at first, but then I committed a change to break it: https://github.com/tweag/nixpkgs/commit/4276e597262ddfcf5dce1c4050a2ca999e950a45 | 14:52:35 |
| infinisil | But retriggering the check from https://github.com/tweag/nixpkgs/actions/runs/6500830578/job/17657034351?pr=74 doesn't cause it to fail still | 14:53:03 |
| infinisil | But it did use an updated base branch, just due to how it fetches the merge ref, which github always updates automatically:
- Before base update: https://github.com/tweag/nixpkgs/actions/runs/6500830578/attempts/1?pr=74#summary-17657001189
- After base update: https://github.com/tweag/nixpkgs/actions/runs/6500830578?pr=74#summary-17657034351
(note how the stated base branch commit changed)
| 14:55:13 |
| Robert Hensing (roberth) | unfortunate :/ | 15:59:33 |
| infinisil | Hmm but even then it's not perfect, PR's before the introduction of the pkgs/by-name check wouldn't be failing :/ | 16:42:26 |
| infinisil | Alternative is to somehow force all PR's to be rebased | 16:42:57 |
| infinisil | Or even automatically rebase? No that would mess with GPG signatures | 16:43:36 |
| infinisil | Merging master automatically into each PR? No that would be a super messy history | 16:43:50 |
| infinisil | Maybe it's possible to just kind of associate a new failing check run with PR's that should be rebased | 16:44:47 |
| infinisil | Let me try experimenting with that | 16:45:18 |
| infinisil | Aha! This looks pretty good https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28 | 16:49:02 |
| infinisil | This is such a general problem, I'm amazed it isn't solved yet..
Well, there is a solution, it's merge queues | 16:51:22 |
| infinisil | Maybe we should just enable merge queues for Nixpkgs? | 16:51:31 |
| infinisil | Okay maybe it will have to be the idea you suggested Robert Hensing (roberth): To have a CI check that ensures individual packages don't regress | 19:53:10 |
| infinisil | This won't have any of the above mess. The only disadvantage is that "rollout" of stricter checks takes a long time | 19:53:56 |
| infinisil | I guess this is like the original idea suggested in https://github.com/NixOS/nixpkgs/issues/256788#issuecomment-1737965650, but a bit more granular | 19:55:29 |
| infinisil | And at that point actually, might as well implement it without granularity initially | 19:56:28 |
| infinisil | So, I've gone full circle! | 19:56:33 |
