| 13 Oct 2023 |
 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 |
| infinisil | Well, no there should be some granularity, because otherwise as soon as one package violates the new check, it won't be checked for all other packages either. This would mean having to fix the base branch many more times | 20:00:10 |
| infinisil | This channel is a great rubber duck lol, thanks everybody :P | 20:09:42 |
|  tgamblin changed their profile picture. | 22:25:03 |
| 17 Oct 2023 |
 Artturin | https://github.com/NixOS/nixpkgs/pull/261650 The by-name check thinks this has a merge conflict
no conflict on the merge-base or master
$ gco 9a4743b
HEAD is now at 9a4743b7e6da Merge pull request #261648 from r-ryantm/auto-update/cargo-udeps
$ git ls-remote --exit-code https://github.com/NixOS/nixpkgs.git refs/pull/261650/merge
3b9053f4b3802a4173cd2f6fa5e725999bf5a427 refs/pull/261650/merge
| 18:56:20 |
| Artturin | the message that git ls-remote outputs should be printed | 18:57:07 |
| Artturin | I'll make a pr | 19:03:14 |
| Artturin | https://github.com/NixOS/nixpkgs/pull/261693 | 19:05:59 |
| infinisil | Thanks! | 19:15:20 |
| 18 Oct 2023 |
 @rick:matrix.ciphernetics.nl | I think adding by-name to all-packages is not intended, right? https://github.com/NixOS/nixpkgs/pull/261427 | 13:33:53 |
| infinisil | Mindavi: Nah that's fine, see https://github.com/NixOS/nixpkgs/tree/master/pkgs/by-name#changing-implicit-attribute-defaults :) | 13:34:38 |
