git-branchless: fix build

[luftmensch-luftmensch]

Description of changes

Closes #338971

Added new tests to skip that fails in an isolated environment

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• 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/)
• 24.11 Release Notes (or backporting 23.11 and 24.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.

---

Add a 👍 reaction to pull requests you find important.

[eclairevoyant]

Please keep any reformatting as its own commit

[luftmensch-luftmensch]

@eclairevoyant Done

changes addressed

[hmenke]

Why are there tests that are not _pty or _interactive in there? These shouldn't fail.

[niklaskorz]

The test output seems to be locale-specific on darwin, thus failing on non-English systems. Is there some environment variable we can pass so the tests are always run in English?

────────────┬───────────────────────────────────────────────────────────────────
    0       │-HEAD detached from 96d1c37
    1       │-Changes to be committed:
    2       │-  (use "git restore --staged <file>..." to unstage)
    3       │-  new file:   test3.txt
          0 │+HEAD losgelöst von 96d1c37
          1 │+Zum Commit vorgemerkte Änderungen:
          2 │+  (benutzen Sie "git restore --staged <Datei>..." zum Entfernen aus der Staging-Area)
          3 │+  neue Datei:     test3.txt
    4     4 │
    5       │-Changes to be committed:
          5 │+Zum Commit vorgemerkte Änderungen:
    6     6 │ diff --git c/test3.txt i/test3.txt
    7     7 │ new file mode 100644
    8     8 │ index 0000000..4706e16
    9     9 │ --- /dev/null
────────────┴───────────────────────────────────────────────────────────────────
To update snapshots run `cargo insta review`
Stopped on the first failure. Run `cargo insta test` to run all snapshots.
The application panicked (crashed).
Message:  snapshot assertion for 'amend_with_children-3' failed in line 45
Location: /private/tmp/nix-build-git-branchless-0.9.0.drv-0/git-branchless-0.9.0-vendor.tar.gz/insta/src/runtime.rs:563

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.


failures:
    test_amend_merge
    test_amend_with_children
    test_amend_with_working_copy

test result: FAILED. 12 passed; 3 failed; 0 ignored; 0 measured; 2 filtered out; finished in 6.94s

error: test failed, to rerun pass `--test test_amend`

Edit: Neither setting env.LANG = "en_US.UTF-8" or adding export LANG="en_US.UTF-8" to preCheck changes this, but I suspect this was broken before. As long as Hydra runs in English and doesn't have this issue...

[luftmensch-luftmensch]

@niklaskorz I don't know if this is a valuable option but we could do something like:

preCheck = lib.optionals stdenv.isDarwin ''
  # Need an UTF-8 locale for test_I test.
  export LANG=en_US.UTF-8
'';

Any thoughts on this approach?

[alixinne]

Why are there tests that are not _pty or _interactive in there? These shouldn't fail.

I did run a git bisect the other day and it started failing with the update of Git in nixos-unstable to 2.46.0. Users seem to be reporting issues with Git 2.46.0 as well, so I'm not sure we should disable those tests indeed:

• Could not parse reference-transaction-line arxanas/git-branchless#1388
• I have this problem when switching branches arxanas/git-branchless#1381

Those issues don't seem to have been traced back to Git 2.46.0 yet, but "updates to Git" are mentioned in arxanas/git-branchless#1321 (comment)

[niklaskorz]

@niklaskorz I don't know if this is a valuable option but we could do something like:

preCheck = lib.optionals stdenv.isDarwin ''
  # Need an UTF-8 locale for test_I test.
  export LANG=en_US.UTF-8
'';

Any thoughts on this approach?

That doesn't seem to change anything, but don't worry about it. If I find out why darwin git would be locale-dependent here and still ignore the LANG env var, I will open another PR.

[samueltardieu]

Result of nixpkgs-review pr 339023 run on x86_64-linux 1

1 package built:

• git-branchless

[hmenke]

@Artturin Why was this merged, even though I (one of the maintainers) requested changes that have not been addressed?

[Artturin]

@Artturin Why was this merged, even though I (one of the maintainers) requested changes that have not been addressed?

Sorry, my bad. I suppose I processed the top level review comment differently from a line review and did not think enough.

Here's a revert of the fix build commit with the formatting commit left alone #340854

Again, sorry for going over your head.

[eclairevoyant]

Reverted - this probably merits discussion under the issue #338971 itself.

[bryango]

I am trying to revive this in #342278, please check out the comments there! Thanks!