Address reviews

mpickering · Jul 14, 2023 · 25e00c3 · 25e00c3
1 parent e9f06f0
commit 25e00c3
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 10 deletions.
diff --git a/Cabal/src/Distribution/Simple/Build.hs b/Cabal/src/Distribution/Simple/Build.hs
@@ -111,7 +111,7 @@ build
   -- ^ preprocessors to run before compiling
   -> IO ()
 build pkg_descr lbi flags suffixes = do
-  checkBuildProblems verbosity (compiler lbi) flags
+  checkSemaphoreSupport verbosity (compiler lbi) flags
   targets <- readTargetInfos verbosity pkg_descr lbi (buildArgs flags)
   let componentsToBuild = neededTargetsInBuildOrder' pkg_descr lbi (map nodeKey targets)
   info verbosity $
@@ -175,10 +175,11 @@ build pkg_descr lbi flags suffixes = do
     distPref = fromFlag (buildDistPref flags)
     verbosity = fromFlag (buildVerbosity flags)
 
-checkBuildProblems
+-- | Check for conditions that would prevent the build from succeeding.
+checkSemaphoreSupport
   :: Verbosity -> Compiler -> BuildFlags -> IO ()
-checkBuildProblems verbosity comp flags = do
-  unless (jsemSupported comp || not (isJust (flagToMaybe (buildUseSemaphore flags)))) $
+checkSemaphoreSupport verbosity comp flags = do
+  unless (jsemSupported comp || (isNothing (flagToMaybe (buildUseSemaphore flags)))) $
     die' verbosity $
       "Your compiler does not support the -jsem flag. "
         ++ "To use this feature you must use GHC 9.8 or later."

diff --git a/Cabal/src/Distribution/Simple/Compiler.hs b/Cabal/src/Distribution/Simple/Compiler.hs
@@ -364,7 +364,7 @@ unitIdSupported = ghcSupported "Uses unit IDs"
 backpackSupported :: Compiler -> Bool
 backpackSupported = ghcSupported "Support Backpack"
 
--- | Does this compiler support the -jsem option
+-- | Does this compiler support the -jsem option?
 jsemSupported :: Compiler -> Bool
 jsemSupported comp = case compilerFlavor comp of
   GHC -> v >= mkVersion [9, 7]

diff --git a/Cabal/src/Distribution/Types/ParStrat.hs b/Cabal/src/Distribution/Types/ParStrat.hs
@@ -10,13 +10,15 @@ data ParStratX sem
     Serial
   deriving (Show)
 
--- Used by Cabal to indicate that we want to use this specific semaphore (created by cabal-install)
+-- | Used by Cabal to indicate that we want to use this specific semaphore (created by cabal-install)
 type ParStrat = ParStratX String
 
--- Used by cabal-install to say we want to create a semaphore with N slots.
+-- | Used by cabal-install to say we want to create a semaphore with N slots.
 type ParStratInstall = ParStratX Int
 
+-- | Determine if the parallelism strategy enables parallel builds.
 isParallelBuild :: ParStratX n -> Bool
 isParallelBuild Serial = False
-isParallelBuild NumJobs{} = True
+isParallelBuild (NumJobs (Just 1)) = False
+isParallelBuild (NumJobs _) = True
 isParallelBuild UseSem{} = True
diff --git a/changelog.d/pr-8557 b/changelog.d/pr-8557
@@ -1,10 +1,10 @@
-synopsis: Add `--enable-semaphore` flag to enable interaction with GHC Job Server protocol
+synopsis: Add `--semaphore` flag to enable interaction with GHC Job Server protocol
 packages: cabal-install
 prs: #8557
 
 description: {
 
-When cabal-install is passed the `--enable-semaphore` flag it will now act as a job server
+When cabal-install is passed the `--semaphore` flag it will now act as a job server
 according to the GHC Jobserver Protocol.
 
 In particular this means that cabal-install will create a semaphore which it then