workflows/check-by-name: print failed command output

[Artturin]

Description of changes

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[infinisil]

Oh I was surprised to not get requested as a reviewer automatically, but I figured it out 😅 #261698

[infinisil]

Looking good. Can't test this easily so let's just merge and watch it for a minute, should be fine.

[infinisil]

Although, I think if it printed anything (even on stderr) we should've seen it already..

[infinisil]

This should be fully fixed with #261741

[infinisil]

Btw here's a workflow run where we can see the effect of this PR, but as I suspected no extra information is printed by the git ls-remote: https://github.com/NixOS/nixpkgs/actions/runs/6558134544/job/17811091261#step:2:17

Anyways, I merged #261741, so this should be fixed now.

[ghost]

@Artturin where is the "escape hatch" from these tests, like you asked for in #263082 (comment) (and I added in bde78f5)

I'm very disturbed by the fact that, contrary to discussion, these checks have been designed so that if there is a bug in the tests you can't fix the test within the same PR that it's obstructing. You have to file a separate PR to fix the test and then wait for Hydra to advance the channel before you can resume work on the obstructed PR. The by-name checks are even foolishly using include_str!("eval.nix") to further ossify the whole mess.

This is absurd.

This is precisely the problem with ofborg being out-of-tree, and we're making it worse.

This is not what was agreed to.

Right now the only way to fix broken tests is to simply disable the github action. As it stands that is precisely what I will do if I have a PR that gets blocked by malfunctioning tests.

[Artturin]

@Artturin where is the "escape hatch" from these tests, like you asked for in #263082 (comment) (and I added in bde78f5)

The currently unfixable (or very difficult to fix) splicing issues are not the same as a fixable test

I'm very disturbed by the fact that, contrary to discussion, these checks have been designed so that if there is a bug in the tests you can't fix the test within the same PR that it's obstructing. You have to file a separate PR to fix the test and then wait for Hydra to advance the channel before you can resume work on the obstructed PR. The by-name checks are even foolishly using include_str!("eval.nix") to further ossify the whole mess.

Relevant issue #256788

Architecture team is available at the architecture team room

[infinisil]

@amjoseph-nixpkgs I have no idea how the splicing relates to this PR here, they seem totally unrelated.

And the supposed issue with the by-name check I'm already discussing with you in #252057 (which would probably better in a separate issue), it's totally unrelated to this PR here too. And the include_str! problem is also unrelated, and it's already tracked in #257748.

And if you think the above issues aren't tracked appropriately, please open a new one and ping me. I won't discuss this here.

[ghost]

I have no idea how the splicing relates to this PR here

Nor do I. My comment was about the need for an escape hatch from malfunctioning checks.

[Artturin]

I have no idea how the splicing relates to this PR here

Nor do I. My comment was about the need for an escape hatch from malfunctioning checks.

The splicing check test isn't malfunctioning.

[ghost]

I have no idea how the splicing relates to this PR here

Nor do I. My comment was about the need for an escape hatch from malfunctioning checks.

The splicing check test isn't malfunctioning.

Alright, malfunctioning or counterproductive checks then.