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

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

Merged
merged 1 commit into from
Feb 28, 2024

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
@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
30 checks passed
@roberth
Copy link
Member

roberth commented Feb 28, 2024

Please also review #291936

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants