Skip to content

Comments

lib.fileset: Fix tests on Darwin, more POSIX#291933

Merged
roberth merged 1 commit intoNixOS:masterfrom
tweag:fileset-tests-posix
Feb 28, 2024
Merged

lib.fileset: Fix tests on Darwin, more POSIX#291933
roberth merged 1 commit intoNixOS:masterfrom
tweag:fileset-tests-posix

Conversation

@infinisil
Copy link
Member

@infinisil infinisil commented Feb 27, 2024

Description of changes

This was found when trying to run the fileset tests on Darwin, which mysteriously fail:

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

.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, I can indeed confirm that there's no mention of \s or the like!

However, there is a mention of [[:blank:]], which indeed does work on Darwin too! So we'll use that instead.

Things done

  • Tested that this now works on both Linux and Darwin

This work is sponsored by Antithesis

Add a 👍 reaction to pull requests you find important.

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.
@infinisil infinisil requested a review from roberth February 27, 2024 22:45
@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Feb 27, 2024
@github-actions github-actions bot added the 6.topic: lib The Nixpkgs function library label Feb 27, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 27, 2024
@infinisil infinisil mentioned this pull request Feb 27, 2024
13 tasks
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

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

Alternatively we could use grep which seems to have more consistent behavior.

I mostly just want it to work though.

@roberth roberth merged commit 01973b0 into NixOS:master Feb 28, 2024
@roberth
Copy link
Member

roberth commented Feb 28, 2024

Please also review #291936

@github-actions
Copy link
Contributor

Successfully created backport PR for release-23.11:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: darwin Running or building packages on Darwin 6.topic: lib The Nixpkgs function library 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants