From df46ecdfd03ee94fd1f1ece308c040fdcebebe57 Mon Sep 17 00:00:00 2001 From: Matthew Pickering Date: Fri, 14 Jun 2024 15:56:21 +0100 Subject: [PATCH] external commands: Wait for process to finish before exiting (#10100) Issue #10063 points out that cabal exits before the external command has finished executing. This was a simple oversight to not waitForProcess on the result of calling createProcess. This also points out the flaw that there isn't a way for external commands to signal failure, so we now also propagate the exit code from the external process. Fixes #10063 (cherry picked from commit cb57c37187c62e60fec5d29085ec1520779aa796) --- cabal-install/src/Distribution/Client/Main.hs | 4 +- .../ExternalCommandExitCode/cabal.out | 8 ++++ .../ExternalCommandExitCode/cabal.project | 1 + .../ExternalCommandExitCode/cabal.test.hs | 38 +++++++++++++++++++ .../setup-test/AAAA.hs | 8 ++++ .../setup-test/CHANGELOG.md | 5 +++ .../setup-test/LICENSE | 30 +++++++++++++++ .../setup-test/setup-test.cabal | 25 ++++++++++++ changelog.d/issue-10063 | 14 +++++++ doc/external-commands.rst | 3 +- 10 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.out create mode 100644 cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.project create mode 100644 cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.test.hs create mode 100644 cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/AAAA.hs create mode 100644 cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/CHANGELOG.md create mode 100644 cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/LICENSE create mode 100644 cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/setup-test.cabal create mode 100644 changelog.d/issue-10063 diff --git a/cabal-install/src/Distribution/Client/Main.hs b/cabal-install/src/Distribution/Client/Main.hs index 59597533026..d508ad08576 100644 --- a/cabal-install/src/Distribution/Client/Main.hs +++ b/cabal-install/src/Distribution/Client/Main.hs @@ -270,7 +270,7 @@ import System.IO , stderr , stdout ) -import System.Process (createProcess, env, proc) +import System.Process (createProcess, env, proc, waitForProcess) -- | Entry point -- @@ -378,7 +378,7 @@ mainWorker args = do result <- try $ createProcess ((proc exec (name : cmdArgs)){env = Just new_env}) case result of Left ex -> printErrors ["Error executing external command: " ++ show (ex :: SomeException)] - Right _ -> return () + Right (_, _, _, ph) -> waitForProcess ph >>= exitWith printCommandHelp help = do pname <- getProgName diff --git a/cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.out b/cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.out new file mode 100644 index 00000000000..1c4c24db55c --- /dev/null +++ b/cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.out @@ -0,0 +1,8 @@ +# cabal v2-build +Resolving dependencies... +Build profile: -w ghc- -O1 +In order, the following will be built: + - setup-test-0.1.0.0 (exe:cabal-aaaa) (first run) +Configuring executable 'cabal-aaaa' for setup-test-0.1.0.0... +Preprocessing executable 'cabal-aaaa' for setup-test-0.1.0.0... +Building executable 'cabal-aaaa' for setup-test-0.1.0.0... diff --git a/cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.project b/cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.project new file mode 100644 index 00000000000..1a33bb5a25e --- /dev/null +++ b/cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.project @@ -0,0 +1 @@ +packages: setup-test/ diff --git a/cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.test.hs b/cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.test.hs new file mode 100644 index 00000000000..7fc79d75815 --- /dev/null +++ b/cabal-testsuite/PackageTests/ExternalCommandExitCode/cabal.test.hs @@ -0,0 +1,38 @@ +import Test.Cabal.Prelude +import qualified System.Process as Process +import Control.Concurrent (threadDelay) +import System.Directory (removeFile) +import Control.Exception (catch, throwIO) +import System.IO.Error (isDoesNotExistError) +import qualified Data.Time.Clock as Time +import qualified Data.Time.Format as Time +import Data.Maybe +import System.Environment +import System.FilePath +import System.Exit + +main = do + cabalTest $ do + res <- cabalWithStdin "v2-build" ["all"] "" + exe_path <- withPlan $ planExePath "setup-test" "cabal-aaaa" + addToPath (takeDirectory exe_path) $ do + -- Test that the thing works at all + res <- fails $ cabal_raw_action ["aaaa"] (\h -> () <$ Process.waitForProcess h) + assertOutputContains "aaaa" res + -- Check the exit code is the one returned by subcommand + unless (resultExitCode res == ExitFailure 99) (assertFailure $ "Incorrect exit code: " ++ show (resultExitCode res)) + + +cabal_raw_action :: [String] -> (Process.ProcessHandle -> IO ()) -> TestM Result +cabal_raw_action args action = do + configured_prog <- requireProgramM cabalProgram + env <- getTestEnv + r <- liftIO $ runAction (testVerbosity env) + (Just $ testCurrentDir env) + (testEnvironment env) + (programPath configured_prog) + args + Nothing + action + recordLog r + requireSuccess r diff --git a/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/AAAA.hs b/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/AAAA.hs new file mode 100644 index 00000000000..b9d4e99c553 --- /dev/null +++ b/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/AAAA.hs @@ -0,0 +1,8 @@ +module Main where + +import System.Environment +import System.Exit + +main = do + getArgs >>= print + exitWith (ExitFailure 99) diff --git a/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/CHANGELOG.md b/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/CHANGELOG.md new file mode 100644 index 00000000000..7ae8ff6113d --- /dev/null +++ b/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/CHANGELOG.md @@ -0,0 +1,5 @@ +# Revision history for setup-test + +## 0.1.0.0 -- YYYY-mm-dd + +* First version. Released on an unsuspecting world. diff --git a/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/LICENSE b/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/LICENSE new file mode 100644 index 00000000000..cd8ad2ac8ae --- /dev/null +++ b/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/LICENSE @@ -0,0 +1,30 @@ +Copyright (c) 2023, Matthew Pickering + +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + + * Redistributions in binary form must reproduce the above + copyright notice, this list of conditions and the following + disclaimer in the documentation and/or other materials provided + with the distribution. + + * Neither the name of Matthew Pickering nor the names of other + contributors may be used to endorse or promote products derived + from this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/setup-test.cabal b/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/setup-test.cabal new file mode 100644 index 00000000000..8deb0577a16 --- /dev/null +++ b/cabal-testsuite/PackageTests/ExternalCommandExitCode/setup-test/setup-test.cabal @@ -0,0 +1,25 @@ +cabal-version: 3.0 +name: setup-test +version: 0.1.0.0 +-- synopsis: +-- description: +license: BSD-3-Clause +license-file: LICENSE +author: Matthew Pickering +maintainer: matthewtpickering@gmail.com +-- copyright: +build-type: Simple +extra-doc-files: CHANGELOG.md +-- extra-source-files: + +common warnings + ghc-options: -Wall + +executable cabal-aaaa + import: warnings + main-is: AAAA.hs + -- other-modules: + -- other-extensions: + build-depends: base + hs-source-dirs: . + default-language: Haskell2010 diff --git a/changelog.d/issue-10063 b/changelog.d/issue-10063 new file mode 100644 index 00000000000..568ffe68a9b --- /dev/null +++ b/changelog.d/issue-10063 @@ -0,0 +1,14 @@ +synopsis: External commands now propagate exit code from child process +packages: cabal-install +prs: #10100 +issues: #10063 + +description: { + The exit code from an external command is now propagated as the exit code of + cabal-install when invoked by calling an external command. + + For example, if your external command exits with code 1, then cabal-install will + also exit with code 1. + + This mechanism can be used by an external command to signal failure. +} diff --git a/doc/external-commands.rst b/doc/external-commands.rst index e72495aa160..eca76483cc1 100644 --- a/doc/external-commands.rst +++ b/doc/external-commands.rst @@ -4,7 +4,8 @@ External Commands ``cabal-install`` provides a system for external commands, akin to the ones used by tools like ``git`` or ``cargo``. If you execute ``cabal ``, ``cabal-install`` will search the path for an executable named ``cabal-`` and execute it. The name of the command is passed as the first argument and -the remaining arguments are passed afterwards. An error will be thrown in case the custom command is not found. +the remaining arguments are passed afterwards. An error will be thrown in case the custom command is not found. The exit code of cabal when calling an external command is the same as the exit code +of the command. The ``$CABAL`` environment variable is set to the path of the ``cabal-install`` executable which invoked the subcommand.