Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to nixos-23.11 #9546

Merged
merged 9 commits into from
Feb 28, 2024
Merged

Update to nixos-23.11 #9546

merged 9 commits into from
Feb 28, 2024

Conversation

thufschmitt
Copy link
Member

Motivation

Update to the latest stable NixOS release.

I didn't try to deeply check anything, just updated the flake input (and removed a redundant fetch) and saw that the build and tests didn't break.

Priorities

Add 👍 to pull requests you find important.

@thufschmitt thufschmitt requested a review from edolstra as a code owner December 6, 2023 13:14
@Ericson2314
Copy link
Member

I think I would like to do #9535 first. Some of the more exotic things have regressions in 23.11 I need to fix also.

@edolstra
Copy link
Member

This is failing because the bear package is marked as broken. (Don't know what that is, but we're using it in the clang devshell.)

@cole-h
Copy link
Member

cole-h commented Jan 12, 2024

It's used for LSP (if you run your make commands with bear, it'll generate compile_commands.json that LSP servers use to find e.g. headers and whatnot).

It was marked broken in NixOS/nixpkgs#250435 (comment) because it fails to build on x86_64-darwin (it built fine on aarch64-darwin for me).

@Ericson2314
Copy link
Member

I guess let's adjust that meta.broken to be more fine-grained? And then the dep here can be corresponding adjusted.

@SuperSandro2000
Copy link
Member

That was already started in NixOS/nixpkgs#279035

@lf-
Copy link
Member

lf- commented Feb 27, 2024

@thufschmitt can you have another go at this? bear is fixed now, and I now have a PR blocked on the C++ compiler being antique.

@thufschmitt
Copy link
Member Author

@thufschmitt can you have another go at this? bear is fixed now, and I now have a PR blocked on the C++ compiler being antique.

The fix hasn't been backported to 23.11 (I've just opened NixOS/nixpkgs#291814 for that).
But that shouldn't block us, we can just disable bear on Darwin. Trying to build the thing right now

@thufschmitt
Copy link
Member Author

@Ericson2314 the evaluation failures on Darwin (https://github.com/NixOS/nix/actions/runs/8064971474/job/22029891317?pr=9546) are above my pay grade, do you think you can have a look?

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Feb 27, 2024
@thufschmitt
Copy link
Member Author

OK, so I managed to fix the evaluation errors on Darwin (by just disabling everything that failed).

I'm quite bewildered by the evaluation performance between 23.05 and 23.11 though: evaluating the flake on 23.11 takes nearly 3x longer than on 23.05 (despite having removed some outputs)

$ hyperfine --warmup 1 \
    "nix flake show nix/nixos-23.11 --option eval-cache false --all-systems" \                                                            
    "nix flake show nix --all-systems --option eval-cache false"
Benchmark 1: nix flake show nix/nixos-23.11 --option eval-cache false --all-systems
  Time (mean ± σ):     27.201 s ±  0.065 s    [User: 19.490 s, System: 2.875 s]
  Range (min … max):   27.100 s … 27.310 s    10 runs
 
Benchmark 2: nix flake show nix --all-systems --option eval-cache false
  Time (mean ± σ):      9.398 s ±  0.216 s    [User: 7.261 s, System: 1.046 s]
  Range (min … max):    9.054 s …  9.843 s    10 runs
 
Summary
  nix flake show nix --all-systems --option eval-cache false ran
    2.89 ± 0.07 times faster than nix flake show nix/nixos-23.11 --option eval-cache false --all-systems

any one knows what that could be due to?

@thufschmitt
Copy link
Member Author

@infinisil there's a failure in the fileset tests on darwin: https://github.com/NixOS/nix/actions/runs/8069576505/job/22044909419?pr=9546 does that match anything already known?

@infinisil
Copy link
Member

infinisil commented Feb 27, 2024

Oof this is weird, it looks like a bash regex matching bug. The behavior of matching \s (any space character) using =~ differs between Linux and Darwin:

# default.nix
{ system ? builtins.currentSystem }:
let
  pkgs = import (fetchTarball "https://github.com/NixOS/nixpkgs/tarball/931ee755b560f905775a11b3a719a303c8f992e1") {
    inherit system;
    config = {};
    overlays = [];
  };
in
pkgs.runCommandNoCC "bash-regex-${system}" {} ''
  r="^\s$"
  if [[ " " =~ $r ]]; then
    echo "\s working"
  else
    echo "\s NOT working"
  fi > $out
''
❯ nix-build --argstr system x86_64-linux && cat result
/nix/store/w0dlf0zzg4n8s8ga9axicbw2axdpinqv-bash-regex-x86_64-linux
\s working
❯ nix-build --argstr system x86_64-darwin && cat result
/nix/store/ikwd9iqq44pbilrbca7lk6zhh6q4ykry-bash-regex-x86_64-darwin
\s NOT working
❯ nix-build --argstr system aarch64-darwin && cat result
/nix/store/mwpgr44xk5bvsr71mjs8cfcqm6x4krdn-bash-regex-aarch64-darwin
\s NOT working

(can't test on aarch64-linux)

@infinisil
Copy link
Member

Turns out \s is not POSIX.. See NixOS/nixpkgs#291933 for a fix

roberth pushed a commit to NixOS/nixpkgs that referenced this pull request Feb 28, 2024
This was found when trying to run the fileset tests on Darwin
(NixOS/nix#9546 (comment)), which mysteriously fail on Darwin:

  test case at lib/fileset/tests.sh:342 failed: toSource { root = "/nix/store/foobar"; fileset = ./.; } should have errored with this regex pattern:

  lib.fileset.toSource: `root` \(/nix/store/foobar\) is a string-like value, but it should be a path instead.
  \s*Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.

  but this was the actual error:

  error: lib.fileset.toSource: `root` (/nix/store/foobar) is a string-like value, but it should be a path instead.
      Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.

After dissecting this, I find out that apparently \s works on Linux, but not on Darwin for some reason!

From the bash source code, it looks like <regex.h> with `REG_EXTENDED` is used for all platforms the same,
so there's nothing odd there.

It's almost impossible to know where <regex.h> comes from,
but it looks to be a POSIX thing.

So after digging through the almost impossible to find POSIX specifications
(https://pubs.opengroup.org/onlinepubs/007908799/xbd/re.html#tag_007_003_005),
I can indeed confirm that there's no mention of \s or the like!

_However_, there is a mention of `[[:blank:]]`, so we'll use that instead.
github-actions bot pushed a commit to NixOS/nixpkgs that referenced this pull request Feb 28, 2024
This was found when trying to run the fileset tests on Darwin
(NixOS/nix#9546 (comment)), which mysteriously fail on Darwin:

  test case at lib/fileset/tests.sh:342 failed: toSource { root = "/nix/store/foobar"; fileset = ./.; } should have errored with this regex pattern:

  lib.fileset.toSource: `root` \(/nix/store/foobar\) is a string-like value, but it should be a path instead.
  \s*Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.

  but this was the actual error:

  error: lib.fileset.toSource: `root` (/nix/store/foobar) is a string-like value, but it should be a path instead.
      Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.

After dissecting this, I find out that apparently \s works on Linux, but not on Darwin for some reason!

From the bash source code, it looks like <regex.h> with `REG_EXTENDED` is used for all platforms the same,
so there's nothing odd there.

It's almost impossible to know where <regex.h> comes from,
but it looks to be a POSIX thing.

So after digging through the almost impossible to find POSIX specifications
(https://pubs.opengroup.org/onlinepubs/007908799/xbd/re.html#tag_007_003_005),
I can indeed confirm that there's no mention of \s or the like!

_However_, there is a mention of `[[:blank:]]`, so we'll use that instead.

(cherry picked from commit 3429594)
Théophane Hufschmitt and others added 6 commits February 28, 2024 07:08
About time :)

This required disabling `bear` on darwin as it's currently broken (fixed
on master, but not yet on 23.11).
The required version check was a bit too lenient, and
`nixpkgs#nixUnstable` was considered valid while it didn't have the fix.
Just `stdenv.isDarwin` isn't enough because it doesn't apply to the
build platform, which mean that cross packages building from darwin to
another platform will have `isDarwin` set to false.
Replace it by `stdenv.buildPlatform.isDarwin`.
Don't evaluate, and probably not really useful (if at all)
Flake lock file updates:

• Updated input 'nixpkgs':
    'github:NixOS/nixpkgs/4dd376f7943c64b522224a548d9cab5627b4d9d6' (2024-02-26)
  → 'github:NixOS/nixpkgs/b550fe4b4776908ac2a861124307045f8e717c8e' (2024-02-28)
I hate this.
We should have it, but for now we can't.
Théophane Hufschmitt added 2 commits February 28, 2024 07:11
The required version check was a bit too lenient, and
`nixpkgs#nixUnstable` was considered valid while it didn't have the fix.
Apparently gcc is able to implicitly cast from `FileIngestionMethod` to
`ContentAddressMethod`, but clang isn't. So explicit the cast
@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Feb 28, 2024
@thufschmitt thufschmitt removed the store Issues and pull requests concerning the Nix store label Feb 28, 2024
`NIX_HARDENING_ENABLE` causes `_FORTIFY_SOURCE` to be defined.
This isn't compatible with `-O0`, and the compiler will happily remind
us about it at every call, spamming the terminal with warnings and stack
traces.

We don't really care hardening in that case, so just disable it if we
pass `OPTIMIZE=0`.
@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Feb 28, 2024
++ lib.optional stdenv.cc.isClang pkgs.buildPackages.bear
# TODO: Remove the darwin check once
# https://github.com/NixOS/nixpkgs/pull/291814 is available
++ lib.optional (stdenv.cc.isClang && !stdenv.buildPlatform.isDarwin) pkgs.buildPackages.bear
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed #9900 did not need it. 👍

@roberth roberth merged commit 587c7dc into master Feb 28, 2024
15 checks passed
@roberth roberth deleted the nixos-23.11 branch February 28, 2024 16:51
@infinisil
Copy link
Member

Docker push is failing: https://github.com/NixOS/nix/actions/runs/8084099897/job/22090423845

This is caused by using Nix 2.13.3:

install_url: https://releases.nixos.org/nix/nix-2.13.3/install

Which doesn't have builtins.readFileType, causing lib.fileset to not work correctly in pure evaluation mode, see also NixOS/nixpkgs#266356 (comment)

@Ericson2314
Copy link
Member

Clang shell is now broken because a change in how fileset works

error: access to absolute path '/nix/store' is forbidden in pure eval mode (use '--impure' to override)

@infinisil
Copy link
Member

@Ericson2314 As above, that only happens with Nix versions <2.13.5, though no idea why such an old version is used there

Ericson2314 added a commit to NixLayeredStore/nix that referenced this pull request Feb 29, 2024
This reverts commit 587c7dc, reversing
changes made to 864fc85.
Ericson2314 added a commit to NixLayeredStore/nix that referenced this pull request Feb 29, 2024
github-actions bot pushed a commit to nix-community/nixpkgs.lib that referenced this pull request Mar 3, 2024
This was found when trying to run the fileset tests on Darwin
(NixOS/nix#9546 (comment)), which mysteriously fail on Darwin:

  test case at lib/fileset/tests.sh:342 failed: toSource { root = "/nix/store/foobar"; fileset = ./.; } should have errored with this regex pattern:

  lib.fileset.toSource: `root` \(/nix/store/foobar\) is a string-like value, but it should be a path instead.
  \s*Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.

  but this was the actual error:

  error: lib.fileset.toSource: `root` (/nix/store/foobar) is a string-like value, but it should be a path instead.
      Paths in strings are not supported by `lib.fileset`, use `lib.sources` or derivations instead.

After dissecting this, I find out that apparently \s works on Linux, but not on Darwin for some reason!

From the bash source code, it looks like <regex.h> with `REG_EXTENDED` is used for all platforms the same,
so there's nothing odd there.

It's almost impossible to know where <regex.h> comes from,
but it looks to be a POSIX thing.

So after digging through the almost impossible to find POSIX specifications
(https://pubs.opengroup.org/onlinepubs/007908799/xbd/re.html#tag_007_003_005),
I can indeed confirm that there's no mention of \s or the like!

_However_, there is a mention of `[[:blank:]]`, so we'll use that instead.
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-55/40996/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
store Issues and pull requests concerning the Nix store with-tests Issues related to testing. PRs with tests have some priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants