cherry-picks check is generally incorrect

[eclairevoyant]

Describe the bug

Any backport PR with merge conflicts will fail CI.

Steps To Reproduce

Backport a PR that can't be automatically backported (with the release-2x.xx label).

Expected behavior

No CI error.

Screenshots

n/a

Additional context

If I was backporting a PR manually, that means there were merge conflicts during automatic backport that had to be resolved manually. In other words, every manually backported PR would end up getting CI error spam.

See #324885, #324886 for examples.

This check is fundamentally broken due to the use of git; some semantic diffing tool should be considered instead.

Notify maintainers

@risicle

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
output here

---

Add a :+1: reaction to issues you find important.

[risicle]

See the message in the failure:

Note this should not necessarily be treated as a hard fail, but a reviewer's attention should be drawn to it and github actions have no way of doing that but to raise a 'failure'

The whole point of this is to cause a reviewer to take a look at it.

In the vast vast majority of cases, even most "manual" cherry-picks, this doesn't cause a problem. I think you're seeing this problem because your commits are large and wide-ranging and by their very nature have to diverge quite a bit between the two branches. Luckily you can ignore the "failure".

There exists a PR #305628 that uses a different github API to post the check results, marking situations like this as "inconclusive". However there are several other problems with that approach, and it's also questionable whether that sort of failure is noticeable enough to make a reviewer go and look at it.

[eclairevoyant]

See the message in the failure:

I did, and it's just spam if every manual backport is going to fail. And then it becomes a question of security if everything is raising the alarm, then people will even start ignoring CI errors.

We should not be introducing errors that people have to ignore.

Like I said above, the check is fundamentally broken, and should not use git.

CC @NixOS/security

[risicle]

every manual backport is going to fail

Every manual backport is not going to fail, and if you took the trouble to look at all the its runs you will see many cases of manual backports passing https://github.com/NixOS/nixpkgs/actions/workflows/check-cherry-picks.yml. You will also see plenty of cases where it fails the first time, nudging the author to fix up their references causing it to pass the second or third time.

Like I said above, the check is fundamentally broken, and should not use git.

Any "semantic diff" tool would have to understand not only nix, but also json, yaml, toml, markdown, xml, bash, and untold other formats we have in our source tree. And remember we're not talking about diffing e.g. json here, we're talking about diffing diffs of json. Semantically. And it would still create false positives because backports frequently fundamentally have to have differences between them.

You also seem to have perfected the technique of reporting issues in antagonistic ways, so I don't think I really have anything further to add to this conversation other than: patches welcome.

[eclairevoyant]

Every manual backport is not going to fail

Fine, I should have more carefully repeated "every manual backport that requires manual resolution of merge conflicts is going to fail".

Look, I'm not here to argue, I barely even create issues in this repo, compared to how many other people's issues I respond to and send PRs for. The only reason I created this issue was because I believe such an issue warrants getting feedback, and I tagged you in case you are already working on some similar topic.

If you want to respond to an issue report with did you read the message and then project an accusation of antagonism, and you're not interested in actually resolving the issue, feel free to keep that to reddit.

Any "semantic diff" tool would have to understand not only nix, but also json, yaml, toml, markdown, xml, bash, and untold other formats we have in our source tree.

Yes, tree-sitter exists, and for diff tools specifically I know of difftastic. I've no idea if it's scriptable but surely that's a much better false-positive rate than what we have now.