Unverified Commit 7e533bab authored by Silvan Mosberger's avatar Silvan Mosberger Committed by GitHub
Browse files

Merge pull request #267381 from tweag/fileset.fileFilter-path 

`fileset.fileFilter`: Don't run predicate unnecessarily
parents 5268a6e2 2035f8a3

lib/fileset/default.nix

+2 −2
Original line number Diff line number Diff line
@@ -380,7 +380,7 @@ in { 
      fileFilter (file: hasPrefix "." file.name) ./.



      # Include all regular files (not symlinks or others) in the current directory

      fileFilter (file: file.type == "regular")

      fileFilter (file: file.type == "regular") ./.

  */

  fileFilter =

    /*
@@ -401,7 +401,7 @@ in { 
    fileset:

    if ! isFunction predicate then

      throw ''

        lib.fileset.fileFilter: First argument is of type ${typeOf predicate}, but it should be a function.''

        lib.fileset.fileFilter: First argument is of type ${typeOf predicate}, but it should be a function instead.''

    else

      _fileFilter predicate

        (_coerce "lib.fileset.fileFilter: Second argument" fileset);

lib/fileset/internal.nix

+25 −14
Original line number Diff line number Diff line
@@ -786,29 +786,40 @@ rec { 
        _differenceTree (path + "/${name}") lhsValue (rhs.${name} or null)

      ) (_directoryEntries path lhs);



  # Filters all files in a file set based on a predicate

  # Type: ({ name, type, ... } -> Bool) -> FileSet -> FileSet

  _fileFilter = predicate: fileset:

    let

      recurse = path: tree:

        mapAttrs (name: subtree:

          if isAttrs subtree || subtree == "directory" then

            recurse (path + "/${name}") subtree

          else if

      # Check the predicate for a single file

      # Type: String -> String -> filesetTree

      fromFile = name: type:

        if

          predicate {

              inherit name;

              type = subtree;

            inherit name type;

            # To ensure forwards compatibility with more arguments being added in the future,

            # adding an attribute which can't be deconstructed :)

            "lib.fileset.fileFilter: The predicate function passed as the first argument must be able to handle extra attributes for future compatibility. If you're using `{ name, file }:`, use `{ name, file, ... }:` instead." = null;

          }

        then

            subtree

          type

        else

          null;



      # Check the predicate for all files in a directory

      # Type: Path -> filesetTree

      fromDir = path: tree:

        mapAttrs (name: subtree:

          if isAttrs subtree || subtree == "directory" then

            fromDir (path + "/${name}") subtree

          else if subtree == null then

            null

          else

            fromFile name subtree

        ) (_directoryEntries path tree);

    in

    if fileset._internalIsEmptyWithoutBase then

      _emptyWithoutBase

    else

      _create fileset._internalBase

        (recurse fileset._internalBase fileset._internalTree);

        (fromDir fileset._internalBase fileset._internalTree);

}

lib/fileset/tests.sh

+27 −0
Original line number Diff line number Diff line
@@ -810,6 +810,13 @@ checkFileset 'difference ./. ./b' 


## File filter



# The first argument needs to be a function

expectFailure 'fileFilter null (abort "this is not needed")' 'lib.fileset.fileFilter: First argument is of type null, but it should be a function instead.'



# The second argument can be a file set or an existing path

expectFailure 'fileFilter (file: abort "this is not needed") null' 'lib.fileset.fileFilter: Second argument is of type null, but it should be a file set or a path instead.'

expectFailure 'fileFilter (file: abort "this is not needed") ./a' 'lib.fileset.fileFilter: Second argument \('"$work"'/a\) is a path that does not exist.'



# The predicate is not called when there's no files

tree=()

checkFileset 'fileFilter (file: abort "this is not needed") ./.'
@@ -875,6 +882,26 @@ checkFileset 'union ./c/a (fileFilter (file: assert file.name != "a"; true) ./.) 
# but here we need to use ./c

checkFileset 'union (fileFilter (file: assert file.name != "a"; true) ./.) ./c'



# Also lazy, the filter isn't called on a filtered out path

tree=(

    [a]=1

    [b]=0

    [c]=0

)

checkFileset 'fileFilter (file: assert file.name != "c"; file.name == "a") (difference ./. ./c)'



# Make sure single files are filtered correctly

tree=(

    [a]=1

    [b]=0

)

checkFileset 'fileFilter (file: assert file.name == "a"; true) ./a'

tree=(

    [a]=0

    [b]=0

)

checkFileset 'fileFilter (file: assert file.name == "a"; false) ./a'



## Tracing



# The second trace argument is returned