stdenv: fix succeedOnFailure

[SuperSandro2000]

Description of changes

Closes #200663

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/)
• 22.11 Release Notes (or backporting 22.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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[roberth]

I believe this effect is better achieved via #196251
Otherwise lgtm.

[knedlsepp]

Used the following to confirm that it works as intended:

let
  pkgs = import ./. { };
in pkgs.runCommand "foo" {
  succeedOnFailure = true;
  } ''
    mkdir $out
    echo foo > $out/foo
    exit 1
  ''

Thank you! :-)

[roberth]

Apparently ofborg is quick enough to test a stdenv rebuild, so I've added @knedlsepp's test to the tests attribute.

[knedlsepp]

@roberth thanks for adding a test. One note: Due to the nature of succeedOnFailure this test might show up on hydra as failed, which might be confusing. I'm not quite sure how to address that though.

[roberth]

@knedlsepp as far as I know, succeedOnFailure is just a stdenv concept that does not need anything beyond a normal derivation build to work correctly. Nix does not have a special interpretation of it, and I don't expect Hydra to have it either. (If it does, it will just succeed on Hydra, but still be useful on ofborg and all the other tools)

It does not invert the exit status either, like #196251 would.

[knedlsepp]

@roberth

Nix does not have a special interpretation of it, and I don't expect Hydra to have it either.

It's true that nix itself does not have any special interpretation, but hydra does. It renders these outputs with a red cross as I'd they failed, but offers downloads in nix-support/hydra-build-products.
It is used in release-tools in nixpkgs such as coverageReport.

NixOS/hydra@039d1be

Anyway, I couldn't find the test that you added in the most recent hydra evaluation, so it doesn't really matter. 😄

Edit: Just noticed that I didn't look at the staging evaluation, but trunk. 🤔

[roberth]

That's a bad hack for something that should be solved by Nix itself, but alright then. By default the package tests attribute doesn't get built on Hydra, so it's probably fine.

[SuperSandro2000]

Should this be triggered by nix eval? I have encountered this on my system and other peoples and I am unsure if the issue is here or in nix eval.