vim: Fix install phase parallelism and harden against future failures

[samueldr]

Description of changes

• fd1afd3#commitcomment-110127655
• ex dependency missing from Vim package makes foo2zjs installation fail #227660

Our current build of vim has a not-exactly-reproducible issue where some tools may not end-up being installed. Those tools are used as build-time dependencies in other packages.

cd /nix/store/12jh1klrixvqilis5ywqsgqr6nkvrbh1-vim-9.0.1441/bin; ln -s vim ex
cd /nix/store/12jh1klrixvqilis5ywqsgqr6nkvrbh1-vim-9.0.1441/bin; ln -s vim view
cd /nix/store/12jh1klrixvqilis5ywqsgqr6nkvrbh1-vim-9.0.1441/bin; ln -s vim rvim
/nix/store/zlf0f88vj30sc7567b80l52d19pbdmy2-bash-5.2-p15/bin/bash: line 1: cd: /nix/store/12jh1klrixvqilis5ywqsgqr6nkvrbh1-vim-9.0.1441/bin: No such file or directory
/nix/store/zlf0f88vj30sc7567b80l52d19pbdmy2-bash-5.2-p15/bin/bash: line 1: cd: /nix/store/12jh1klrixvqilis5ywqsgqr6nkvrbh1-vim-9.0.1441/bin: No such file or directory
/nix/store/zlf0f88vj30sc7567b80l52d19pbdmy2-bash-5.2-p15/bin/bash: line 1: cd: /nix/store/12jh1klrixvqilis5ywqsgqr6nkvrbh1-vim-9.0.1441/bin: No such file or directory

— https://hydra.nixos.org/build/215173431/nixlog/1

There are two changes in this PR:

• Make fd1afd3 actually take effect (cc @vcunat)
• Prevent future builds from being successful when the build is actually bad.

When attempting to reproduce the issue, I had to use this, or else on my -j16 system issues wouldn't reproduce.

diff --git a/pkgs/applications/editors/vim/common.nix b/pkgs/applications/editors/vim/common.nix
index e946139e6e8..77861ca1f28 100644
--- a/pkgs/applications/editors/vim/common.nix
+++ b/pkgs/applications/editors/vim/common.nix
@@ -1,6 +1,6 @@
 { lib, fetchFromGitHub }:
 rec {
-  version = "9.0.1441";
+  version = "9.0.1441+${toString builtins.currentTime}";
 
   src = fetchFromGitHub {
     owner = "vim";
diff --git a/pkgs/applications/editors/vim/default.nix b/pkgs/applications/editors/vim/default.nix
index 4dacccafe74..bbbc65ccc90 100644
--- a/pkgs/applications/editors/vim/default.nix
+++ b/pkgs/applications/editors/vim/default.nix
@@ -42,6 +42,8 @@ stdenv.mkDerivation {
   # which.sh is used to for vim's own shebang patching, so make it find
   # binaries for the host platform.
   preConfigure = ''
+    export NIX_BUILD_CORES=25
+
     export HOST_PATH
     substituteInPlace src/which.sh --replace '$PATH' '$HOST_PATH'
   '';

Obviously(?), to test the last commit, you should probably revert the first commit of this PR. Note that it may or may not fail on your specific system due to racing and timing.

There is also an upstream fix sent for two observed parallelism bugs, but since it looks like there could be at least another one, let's keep installing without parallelism.

• Fix a pair of bugs for parallel (-jX) make install vim/vim#12288

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.05 Release Notes (or backporting 22.11 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.

[raboof]

Looks good, but the build is failing for me with:

if test -d /nix/store/3aip4s6qj3v0bkvp7p37g84n4iyan4r2-vim-9.0.1441/share/icons/locolor/16x16/apps -a -w /nix/store/3aip4s6qj3v0bkvp7p37g84n4iyan4r2-vim-9.0.1441/share/icons/locolor/16x16/apps \
        -a ! -f /nix/store/3aip4s6qj3v0bkvp7p37g84n4iyan4r2-vim-9.0.1441/share/icons/locolor/16x16/apps/gvim.png; then \
   cp ../runtime/vim16x16.png /nix/store/3aip4s6qj3v0bkvp7p37g84n4iyan4r2-vim-9.0.1441/share/icons/locolor/16x16/apps/gvim.png; \
fi
if test -d /nix/store/3aip4s6qj3v0bkvp7p37g84n4iyan4r2-vim-9.0.1441/share/applications -a -w /nix/store/3aip4s6qj3v0bkvp7p37g84n4iyan4r2-vim-9.0.1441/share/applications; then \
   if test -f po/vim.desktop -a -f po/gvim.desktop; then \
        cp po/vim.desktop po/gvim.desktop \
                /nix/store/3aip4s6qj3v0bkvp7p37g84n4iyan4r2-vim-9.0.1441/share/applications; \
   else \
        cp ../runtime/vim.desktop \
                ../runtime/gvim.desktop \
                /nix/store/3aip4s6qj3v0bkvp7p37g84n4iyan4r2-vim-9.0.1441/share/applications; \
   fi; \
   if test -z "" -a -x ""; then \
       -q /nix/store/3aip4s6qj3v0bkvp7p37g84n4iyan4r2-vim-9.0.1441/share/applications; \
   fi \
fi
make[1]: Leaving directory '/build/source/src'
/nix/store/5s1yg5l36wzgy1dj0vv1ibarc4g7vrdr-stdenv-linux/setup: eval: line 131: syntax error: unexpected end of file

[raboof]

Are those bugs in an upstream issue tracker somewhere? Could we link to that?

[dasJ]

If so I'd also link from common.nix in line 13

[samueldr]

@raboof Here it's hardening from bugs as in future regressions. The two I know about have an upstream fix sent:

• Fix a pair of bugs for parallel (-jX) make install vim/vim#12288

@dasJ what do you mean? It's not intrinsically linked to parallel building. For example, it's unknown what made #227598 fail in a similar way.

[raboof]

Change looks good (and the branch now builds for me).

However, since AFAICS this is not fixing a regression on staging (but a problem on master), it should be targeting staging rather than staging-next, right? (https://nixos.org/manual/nixpkgs/unstable/#submitting-changes-commit-policy)

[samueldr]

I'm not 100% sure. Right now there may be issues with some configurations when cups is used in NixOS, since a dependency can't be built. So it seems to me this is a relatively major breakage that this fixes. I'm unsure where this sits with the workflow, since sending to staging would mean an additional full cycle until the change is done.

I'm leaving the decision of staging vs. staging-next to the peeps actually managing staging/-next.