Merge branch 'master' into yt/fix-paths-windows

Author: yutotakano

.github/workflows/validate.yml

@@ -336,7 +336,11 @@ jobs:
           apk add bash curl sudo jq pkgconfig \
           zlib-dev zlib-static binutils-gold curl \
           gcc g++ gmp-dev libc-dev libffi-dev make \
-          musl-dev ncurses-dev perl tar xz
+          musl-dev ncurses-dev perl tar xz git
+
+      # Apparently there's some permissions thing inside vs. outside the container
+      # that Alpine's git doesn't like. Hack around it.
+      - run: git config --global --add safe.directory $(pwd)
 
       - uses: actions/checkout@v4
 

CONTRIBUTING.md

@@ -360,6 +360,23 @@ If your PR is trivial you can omit this process (but explain in the PR why you
 think it does not warrant an issue). Feel free to open a new issue (or new
 issues) when appropriate.
 
+### Pull request size
+
+Keep your pull requests small, write one pull request per feature, let
+the content of the pull request match the title of the pull request.
+
+To get merged, your pull request needs to be reviewed by two other
+contributors. Large pull requests are daunting to inspect, and the
+back-and-forth between the author and reviewer can get frustrating and
+difficult to follow.
+
+Split your pull requests in multiple ones if possible (e.g. a refactor
+and a feature implementation should go in two different pull requests).
+This is *especially* important when we decide to backport a pull request
+(be it fix or a feature).
+
+Thorough reviews mean less regressions, keeping your pull requests small
+will improve Cabal codebase quality.
 
 ## Changelog
 

Cabal-hooks/Cabal-hooks.cabal

@@ -30,7 +30,7 @@ library
     , Cabal-syntax    >= 3.15      && < 3.17
     , Cabal           >= 3.15      && < 3.17
     , base            >= 4.13      && < 5
-    , containers      >= 0.5.0.0   && < 0.8
+    , containers      >= 0.5.0.0   && < 0.9
     , transformers    >= 0.5.6.0   && < 0.7
 
   ghc-options: -Wall -fno-ignore-asserts -Wtabs -Wincomplete-uni-patterns -Wincomplete-record-updates

Cabal-syntax/Cabal-syntax.cabal

@@ -32,7 +32,7 @@ library
     , base       >= 4.13     && < 5
     , binary     >= 0.7      && < 0.9
     , bytestring >= 0.10.0.0 && < 0.13
-    , containers >= 0.5.0.0  && < 0.8
+    , containers >= 0.5.0.0  && < 0.9
     , deepseq    >= 1.3.0.1  && < 1.7
     , directory  >= 1.2      && < 1.4
     , filepath   >= 1.3.0.1  && < 1.6

Cabal/Cabal.cabal

@@ -43,7 +43,7 @@ library
     , array      >= 0.4.0.1  && < 0.6
     , base       >= 4.13     && < 5
     , bytestring >= 0.10.0.0 && < 0.13
-    , containers >= 0.5.0.0  && < 0.8
+    , containers >= 0.5.0.0  && < 0.9
     , deepseq    >= 1.3.0.1  && < 1.7
     , directory  >= 1.2      && < 1.4
     , filepath   >= 1.3.0.1  && < 1.6

Cabal/src/Distribution/PackageDescription/Check.hs

@@ -268,8 +268,6 @@ checkGenericPackageDescription
       checkP
         (not . null $ dups names)
         (PackageBuildImpossible $ DuplicateSections dupes)
-      -- PackageDescription checks.
-      checkPackageDescription packageDescription_
       -- Flag names.
       mapM_ checkFlagName genPackageFlags_
 

Cabal/src/Distribution/Simple/Bench.hs

@@ -22,7 +22,6 @@ module Distribution.Simple.Bench
 import Distribution.Compat.Prelude
 import Prelude ()
 
-import Distribution.Compat.Environment
 import qualified Distribution.PackageDescription as PD
 import Distribution.Pretty
 import Distribution.Simple.Build (addInternalBuildTools)
@@ -89,15 +88,12 @@ bench args pkg_descr lbi flags = do
               dieWithException verbosity $
                 NoBenchMarkProgram cmd
 
-            existingEnv <- getEnvironment
-
             -- Compute the appropriate environment for running the benchmark
             let progDb = LBI.withPrograms lbiForBench
                 pathVar = progSearchPath progDb
                 envOverrides = progOverrideEnv progDb
             newPath <- programSearchPathAsPATHVar pathVar
-            overrideEnv <- fromMaybe [] <$> getEffectiveEnvironment ([("PATH", Just newPath)] ++ envOverrides)
-            let shellEnv = overrideEnv ++ existingEnv
+            shellEnv <- getFullEnvironment ([("PATH", Just newPath)] ++ envOverrides)
 
             -- Add (DY)LD_LIBRARY_PATH if needed
             shellEnv' <-

Cabal/src/Distribution/Simple/Program/Internal.hs

@@ -17,21 +17,28 @@ import Prelude ()
 
 -- | Extract the version number from the output of 'strip --version'.
 --
--- Invoking "strip --version" gives very inconsistent results. We ignore
--- everything in parentheses (see #2497), look for the first word that starts
--- with a number, and try parsing out the first two components of it. Non-GNU
--- 'strip' doesn't appear to have a version flag.
+-- Invoking "strip --version" gives very inconsistent results. We
+-- ignore everything in parentheses (see #2497), look for the first
+-- word that starts with a number, and try parsing out the first two
+-- components of it. Non-GNU, non-LLVM 'strip' doesn't appear to have
+-- a version flag.
 stripExtractVersion :: String -> String
 stripExtractVersion str =
   let numeric "" = False
       numeric (x : _) = isDigit x
 
+      closingParentheses =
+        [ ")"
+        , -- LLVM strip outputs "llvm-strip, compatible with GNU strip\nLLVM (http://llvm.org/):\n..."
+          "):"
+        ]
+
       -- Filter out everything in parentheses.
       filterPar' :: Int -> [String] -> [String]
       filterPar' _ [] = []
       filterPar' n (x : xs)
         | n >= 0 && "(" `isPrefixOf` x = filterPar' (n + 1) ((safeTail x) : xs)
-        | n > 0 && ")" `isSuffixOf` x = filterPar' (n - 1) xs
+        | n > 0 && any (`isSuffixOf` x) closingParentheses = filterPar' (n - 1) xs
         | n > 0 = filterPar' n xs
         | otherwise = x : filterPar' n xs
 

Cabal/src/Distribution/Simple/Program/Run.hs

@@ -29,6 +29,7 @@ module Distribution.Simple.Program.Run
   , getProgramInvocationOutputAndErrors
   , getProgramInvocationLBSAndErrors
   , getEffectiveEnvironment
+  , getFullEnvironment
   ) where
 
 import Distribution.Compat.Prelude
@@ -237,6 +238,12 @@ getProgramInvocationIODataAndErrors
 -- | Return the current environment extended with the given overrides.
 -- If an entry is specified twice in @overrides@, the second entry takes
 -- precedence.
+--
+-- getEffectiveEnvironment returns 'Nothing' when there are no overrides.
+-- It returns an argument that is suitable to pass directly to 'CreateProcess' to
+-- override the environment.
+-- If you need the full environment to manipulate further, even when there are no overrides,
+-- then call 'getFullEnvironment'.
 getEffectiveEnvironment
   :: [(String, Maybe String)]
   -> IO (Maybe [(String, String)])
@@ -248,6 +255,17 @@ getEffectiveEnvironment overrides =
     update (var, Nothing) = Map.delete var
     update (var, Just val) = Map.insert var val
 
+-- | Like 'getEffectiveEnvironment', but when no overrides are specified,
+-- returns the full environment instead of 'Nothing'.
+getFullEnvironment
+  :: [(String, Maybe String)]
+  -> IO [(String, String)]
+getFullEnvironment overrides = do
+  menv <- getEffectiveEnvironment overrides
+  case menv of
+    Just env -> return env
+    Nothing -> getEnvironment
+
 -- | Like the unix xargs program. Useful for when we've got very long command
 -- lines that might overflow an OS limit on command line length and so you
 -- need to invoke a command multiple times to get all the args in.

Cabal/src/Distribution/Simple/Test/ExeV10.hs

@@ -8,7 +8,6 @@ module Distribution.Simple.Test.ExeV10
 import Distribution.Compat.Prelude
 import Prelude ()
 
-import Distribution.Compat.Environment
 import qualified Distribution.PackageDescription as PD
 import Distribution.Simple.BuildPaths
 import Distribution.Simple.Compiler
@@ -64,8 +63,6 @@ runTest pkg_descr lbi clbi hpcMarkupInfo flags suite = do
       way = guessWay lbi
       tixDir_ = i $ tixDir distPref way
 
-  existingEnv <- getEnvironment
-
   let cmd =
         i (LBI.buildDir lbi)
           </> testName'
@@ -92,13 +89,18 @@ runTest pkg_descr lbi clbi hpcMarkupInfo flags suite = do
       pathVar = progSearchPath progDb
       envOverrides = progOverrideEnv progDb
   newPath <- programSearchPathAsPATHVar pathVar
-  overrideEnv <- fromMaybe [] <$> getEffectiveEnvironment ([("PATH", Just newPath)] ++ envOverrides)
   let opts =
         map
           (testOption pkg_descr lbi suite)
           (testOptions flags)
       tixFile = packageRoot (testCommonFlags flags) </> getSymbolicPath (tixFilePath distPref way (testName'))
-      shellEnv = [("HPCTIXFILE", tixFile) | isCoverageEnabled] ++ overrideEnv ++ existingEnv
+
+  shellEnv <-
+    getFullEnvironment
+      ( [("PATH", Just newPath)]
+          ++ [("HPCTIXFILE", Just tixFile) | isCoverageEnabled]
+          ++ envOverrides
+      )
 
   -- Add (DY)LD_LIBRARY_PATH if needed
   shellEnv' <-