From 562527499ff68d08c732bdfbadbbfbb13b9483cf Mon Sep 17 00:00:00 2001 From: Artem Pelenitsyn Date: Sat, 2 Nov 2024 19:41:31 -0400 Subject: [PATCH] fix `cabal install --program-suffix/prefix` (fix #10290 and #10476) (#10483) * fix cabal install --program-suffix/prefix (fix #10290 and #10476) When checking for existing installations, cabal would not account for an affix (suffix or prefix). So, if you had a `hello` binary installed, installing a second one with a non-empty affix (a perfectly legal operation) would fail. The reason seemed to be a typo in 09c04e9aca5de2ca391eb859a5b295fdd617f5c6, which passed the arguments to the Symlink structure in a wrong order. When failing to install a binary because of an existing one, cabal would report suffix-less existing target even if a suffix was set. * Add regression tests for overwrite policies and porgram-affixes Add regression tests for the `program-prefix` and `program-suffix` flags combined with the overwrite-policy. In short, the overwrite-policy needs to take potential program affixes into account when deciding whether it will need to overwrite a program path during installation. --------- Co-authored-by: Fendor (cherry picked from commit ee3c31398c776ce210fd6bbb5000a7532ea43c31) --- .../src/Distribution/Client/CmdInstall.hs | 4 +- .../ProgramAffixes/overwrite-policy.out | 160 ++++++++++++++++++ .../ProgramAffixes/overwrite-policy.test.hs | 69 ++++++++ 3 files changed, 231 insertions(+), 2 deletions(-) create mode 100644 cabal-testsuite/PackageTests/Install/ProgramAffixes/overwrite-policy.out create mode 100644 cabal-testsuite/PackageTests/Install/ProgramAffixes/overwrite-policy.test.hs diff --git a/cabal-install/src/Distribution/Client/CmdInstall.hs b/cabal-install/src/Distribution/Client/CmdInstall.hs index b39b97278ae..8df481f589a 100644 --- a/cabal-install/src/Distribution/Client/CmdInstall.hs +++ b/cabal-install/src/Distribution/Client/CmdInstall.hs @@ -1123,8 +1123,8 @@ symlink overwritePolicy installDir (mkSourceBinDir unit) - (mkExeName exe) (mkFinalExeName exe) + (mkExeName exe) -- | -- -- * When 'InstallCheckOnly', warn if install would fail overwrite policy @@ -1169,7 +1169,7 @@ installCheckUnitExes errorMessage installdir exe = case overwritePolicy of NeverOverwrite -> "Path '" - <> (installdir prettyShow exe) + <> (installdir mkFinalExeName exe) <> "' already exists. " <> "Use --overwrite-policy=always to overwrite." -- This shouldn't even be possible, but we keep it in case symlinking or diff --git a/cabal-testsuite/PackageTests/Install/ProgramAffixes/overwrite-policy.out b/cabal-testsuite/PackageTests/Install/ProgramAffixes/overwrite-policy.out new file mode 100644 index 00000000000..cf1c952c9d3 --- /dev/null +++ b/cabal-testsuite/PackageTests/Install/ProgramAffixes/overwrite-policy.out @@ -0,0 +1,160 @@ +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Build profile: -w ghc- -O1 +In order, the following will be built: + - p-1.0 (exe:p) (requires build) +Configuring p-1.0... +Preprocessing executable 'p' for p-1.0... +Building executable 'p' for p-1.0... +Installing executable p in +Warning: The directory /overwrite-policy.dist/home/.cabal/store/ghc-/incoming/new-/overwrite-policy.dist/home/.cabal/store/ghc-/-/bin is not in the system search path. +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Error: [Cabal-7149] +Path '/overwrite-policy.dist/usr/bin/p' already exists. Use --overwrite-policy=always to overwrite. +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/p-my-suffix' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Error: [Cabal-7149] +Path '/overwrite-policy.dist/usr/bin/p-my-suffix' already exists. Use --overwrite-policy=always to overwrite. +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/p-my-suffix' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/my-prefix-p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Error: [Cabal-7149] +Path '/overwrite-policy.dist/usr/bin/my-prefix-p' already exists. Use --overwrite-policy=always to overwrite. +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/my-prefix-p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/my-prefix-p-my-suffix' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Error: [Cabal-7149] +Path '/overwrite-policy.dist/usr/bin/my-prefix-p-my-suffix' already exists. Use --overwrite-policy=always to overwrite. +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/my-prefix-p-my-suffix' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Symlinking 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Error: [Cabal-7149] +Path '/overwrite-policy.dist/usr/bin/p' already exists. Use --overwrite-policy=always to overwrite. +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/p-my-suffix' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Error: [Cabal-7149] +Path '/overwrite-policy.dist/usr/bin/p-my-suffix' already exists. Use --overwrite-policy=always to overwrite. +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/p-my-suffix' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/my-prefix-p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Error: [Cabal-7149] +Path '/overwrite-policy.dist/usr/bin/my-prefix-p' already exists. Use --overwrite-policy=always to overwrite. +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/my-prefix-p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/p' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/my-prefix-p-my-suffix' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Error: [Cabal-7149] +Path '/overwrite-policy.dist/usr/bin/my-prefix-p-my-suffix' already exists. Use --overwrite-policy=always to overwrite. +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/my-prefix-p-my-suffix' +# cabal install +Wrote tarball sdist to /overwrite-policy.dist/work/./dist/sdist/p-1.0.tar.gz +Resolving dependencies... +Copying 'p' to '/overwrite-policy.dist/usr/bin/p' diff --git a/cabal-testsuite/PackageTests/Install/ProgramAffixes/overwrite-policy.test.hs b/cabal-testsuite/PackageTests/Install/ProgramAffixes/overwrite-policy.test.hs new file mode 100644 index 00000000000..5df258f2144 --- /dev/null +++ b/cabal-testsuite/PackageTests/Install/ProgramAffixes/overwrite-policy.test.hs @@ -0,0 +1,69 @@ +import Test.Cabal.Prelude +import Data.Bool (bool) +import System.FilePath (()) +import System.Directory (removeFile) + +main = cabalTest $ do + runTestForInstallMethod "symlink" + runTestForInstallMethod "copy" + +runTestForInstallMethod :: String -> TestM () +runTestForInstallMethod method = do + env <- getTestEnv + let installdir = testPrefixDir env "bin" + + -- install the binary, don't overwrite anything + cabal "install" + ["p", "--installdir", installdir, "--install-method", method, "--overwrite-policy", "never"] + -- install the binary, don't overwrite anything + fails $ cabal "install" + ["p", "--installdir", installdir, "--install-method", method, "--overwrite-policy", "never"] + -- install the binary again, forcing an overwrite, should succeed. + cabal "install" + ["p", "--installdir", installdir, "--install-method", method, "--overwrite-policy", "always"] + -- remove the installed binary. + ext <- exeExt + liftIO $ removeFile (installdir "p" <.> ext) + + testPolicyForAffix installdir method ["--program-suffix", "-my-suffix"] + testPolicyForAffix installdir method ["--program-prefix", "my-prefix-"] + testPolicyForAffix installdir method ["--program-prefix", "my-prefix-", "--program-suffix", "-my-suffix"] + -- remove the installed binaries. + liftIO $ removeFile (installdir "p" <.> ext) + liftIO $ removeFile (installdir "p-my-suffix" <.> ext) + liftIO $ removeFile (installdir "my-prefix-p" <.> ext) + liftIO $ removeFile (installdir "my-prefix-p-my-suffix" <.> ext) + +-- | Run a policy test for a given 'install-method' and program-affix +-- (i.e., '--program-suffix' or '--program-prefix'). +-- +-- When a program affix is given, the installation should not be affected +-- by installing the binary with no affix and vice-versa. +-- So, installing the program without any affix is not affected by installations with +-- some program affix. +testPolicyForAffix :: FilePath -> String -> [String] -> TestM () +testPolicyForAffix installdir method affixArgs = do + -- install the binary again, forcing an overwrite, must succeed. + -- The rest of this test assumes the binary has been installed before. + cabal "install" + ["p", "--installdir", installdir, "--install-method", method, "--overwrite-policy", "always"] + + -- Install the binary with some program affix, don't need overwrite anything + cabal "install" + (["p", "--installdir", installdir, "--install-method", method, "--overwrite-policy", "never"] ++ affixArgs) + -- Once the binary is installed, we can't overwrite it unless we are told so. + fails $ cabal "install" + (["p", "--installdir", installdir, "--install-method", method, "--overwrite-policy", "never"] ++ affixArgs) + -- Successfully overwrite the binary if told so. + cabal "install" + (["p", "--installdir", installdir, "--install-method", method, "--overwrite-policy", "always"] ++ affixArgs) + + -- remove the installed binary. + ext <- exeExt + liftIO $ removeFile (installdir "p" <.> ext) + -- Make sure we can still install the original program with no program affix without overwriting, + -- even though, the program is already installed with some affix. + cabal "install" + ["p", "--installdir", installdir, "--install-method", method, "--overwrite-policy", "never"] + +exeExt = isWindows >>= pure . bool "" "exe"