Skip to content

Commit

Permalink
Add check for recursive glob in root directory (#8441)
Browse files Browse the repository at this point in the history
* Add check for recursive glob in root directory

Such globs might be expensive to include, as they might pull unnecessary
 directories just like `.git` or `dist-newstyle`.

* Move expensive glob warning to its own constructor

* Add changelog entry

* Add test for recursive glob warning

* Fix formatting
  • Loading branch information
dyniec authored Jan 18, 2023
1 parent 96dea0f commit fff7a98
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 6 deletions.
38 changes: 32 additions & 6 deletions Cabal/src/Distribution/PackageDescription/Check.hs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ data CheckExplanation =
| BadRelativePAth String FilePath String
| DistPoint (Maybe String) FilePath
| GlobSyntaxError String String
| RecursiveGlobInRoot String FilePath
| InvalidOnWin [FilePath]
| FilePathTooLong FilePath
| FilePathNameTooLong FilePath
Expand Down Expand Up @@ -532,6 +533,10 @@ ppExplanation (DistPoint mfield path) =
mfield
ppExplanation (GlobSyntaxError field expl) =
"In the '" ++ field ++ "' field: " ++ expl
ppExplanation (RecursiveGlobInRoot field glob) =
"In the '" ++ field ++ "': glob '" ++ glob
++ "' starts at project root directory, this might "
++ "include `.git/`, ``dist-newstyle/``, or other large directories!"
ppExplanation (InvalidOnWin paths) =
"The " ++ quotes paths ++ " invalid on Windows, which "
++ "would cause portability problems for this package. Windows file "
Expand Down Expand Up @@ -1601,20 +1606,35 @@ checkPaths pkg =
++
[ PackageDistInexcusable $
GlobSyntaxError "data-files" (explainGlobSyntaxError pat err)
| pat <- dataFiles pkg
, Left err <- [parseFileGlob (specVersion pkg) pat]
| (Left err, pat) <- zip globsDataFiles $ dataFiles pkg
]
++
[ PackageDistInexcusable
(GlobSyntaxError "extra-source-files" (explainGlobSyntaxError pat err))
| pat <- extraSrcFiles pkg
, Left err <- [parseFileGlob (specVersion pkg) pat]
| (Left err, pat) <- zip globsExtraSrcFiles $ extraSrcFiles pkg
]
++
[ PackageDistInexcusable $
GlobSyntaxError "extra-doc-files" (explainGlobSyntaxError pat err)
| pat <- extraDocFiles pkg
, Left err <- [parseFileGlob (specVersion pkg) pat]
| (Left err, pat) <- zip globsExtraDocFiles $ extraDocFiles pkg
]
++
[ PackageDistSuspiciousWarn $
RecursiveGlobInRoot "data-files" pat
| (Right glob, pat) <- zip globsDataFiles $ dataFiles pkg
, isRecursiveInRoot glob
]
++
[ PackageDistSuspiciousWarn $
RecursiveGlobInRoot "extra-source-files" pat
| (Right glob, pat) <- zip globsExtraSrcFiles $ extraSrcFiles pkg
, isRecursiveInRoot glob
]
++
[ PackageDistSuspiciousWarn $
RecursiveGlobInRoot "extra-doc-files" pat
| (Right glob, pat) <- zip globsExtraDocFiles $ extraDocFiles pkg
, isRecursiveInRoot glob
]
where
isOutsideTree path = case splitDirectories path of
Expand Down Expand Up @@ -1655,6 +1675,12 @@ checkPaths pkg =
[ (path, "extra-lib-dirs-static", PathKindDirectory) | path <- extraLibDirsStatic bi ]
| bi <- allBuildInfo pkg
]
globsDataFiles :: [Either GlobSyntaxError Glob]
globsDataFiles = parseFileGlob (specVersion pkg) <$> dataFiles pkg
globsExtraSrcFiles :: [Either GlobSyntaxError Glob]
globsExtraSrcFiles = parseFileGlob (specVersion pkg) <$> extraSrcFiles pkg
globsExtraDocFiles :: [Either GlobSyntaxError Glob]
globsExtraDocFiles = parseFileGlob (specVersion pkg) <$> extraDocFiles pkg

--TODO: check sets of paths that would be interpreted differently between Unix
-- and windows, ie case-sensitive or insensitive. Things that might clash, or
Expand Down
6 changes: 6 additions & 0 deletions Cabal/src/Distribution/Simple/Glob.hs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ module Distribution.Simple.Glob (
fileGlobMatches,
parseFileGlob,
explainGlobSyntaxError,
isRecursiveInRoot,
Glob,
) where

Expand Down Expand Up @@ -336,3 +337,8 @@ splitConstantPrefix = unfoldr' step
where
step (GlobStem seg pat) = Right (seg, pat)
step (GlobFinal pat) = Left pat


isRecursiveInRoot :: Glob -> Bool
isRecursiveInRoot (GlobFinal (FinalMatch Recursive _ _)) = True
isRecursiveInRoot _ = False
Empty file.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# cabal check
Warning: These warnings may cause trouble when distributing the package:
Warning: In the 'data-files': glob '**/*.dat' starts at project root
directory, this might include `.git/`, ``dist-newstyle/``, or other large
directories!
Warning: In the 'extra-source-files': glob '**/*.hs' starts at project root
directory, this might include `.git/`, ``dist-newstyle/``, or other large
directories!
Warning: In the 'extra-doc-files': glob '**/*.md' starts at project root
directory, this might include `.git/`, ``dist-newstyle/``, or other large
directories!
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import Test.Cabal.Prelude

main = cabalTest $
cabal "check" []
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
cabal-version: 3.8
name: pkg
version: 0
extra-source-files:
**/*.hs
data-files:
**/*.dat
extra-doc-files:
**/*.md
license: BSD-3-Clause
synopsis: no
description: none
category: Test
maintainer: none

library
default-language: Haskell2010
exposed-modules:
Foo
10 changes: 10 additions & 0 deletions changelog.d/pr-8441
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
synopsis: Add warning about expensive globs
packages: Cabal
prs: #8441
issues: #5311
description: {

- Now cabal check will emit a warning when package uses
recursive globs starting at root of the project

}

0 comments on commit fff7a98

Please sign in to comment.