Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error message when multiple .cabal files are in the same directory #7396

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cabal-install/src/Distribution/Client/CmdErrorMessages.hs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,12 @@ renderTargetProblem verb _ (TargetProblemNoSuchComponent pkgid cname) =
++ "for the project plan, which would suggest an inconsistency "
++ "between readTargetSelectors and resolveTargets."

renderTargetProblem verb _ (TargetProblemNotSinglePackage []) =
"Internal error when trying to " ++ verb ++ ". Found no package."
renderTargetProblem verb _ (TargetProblemNotSinglePackage pkgids) =
"Internal error when trying to " ++ verb ++ ". Found multiple packages:\n"
++ unlines (map prettyShow pkgids)


------------------------------------------------------------
-- Rendering error messages for TargetProblemNoneEnabled
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/src/Distribution/Client/CmdSdist.hs
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ packageToSdist :: Verbosity -> FilePath -> OutputFormat -> FilePath -> Unresolve
packageToSdist verbosity projectRootDir format outputFile pkg = do
let death = die' verbosity ("The impossible happened: a local package isn't local" <> (show pkg))
dir0 <- case srcpkgSource pkg of
LocalUnpackedPackage path -> pure (Right path)
LocalUnpackedPackage path _ -> pure (Right path)
RemoteSourceRepoPackage _ (Just tgz) -> pure (Left tgz)
RemoteSourceRepoPackage {} -> death
LocalTarballPackage tgz -> pure (Left tgz)
Expand Down
4 changes: 2 additions & 2 deletions cabal-install/src/Distribution/Client/Configure.hs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ configure verbosity packageDBs repoCtxt comp platform progdb
let installPlan = InstallPlan.configureInstallPlan configFlags installPlan0
in case fst (InstallPlan.ready installPlan) of
[pkg@(ReadyPackage
(ConfiguredPackage _ (SourcePackage _ _ (LocalUnpackedPackage _) _)
(ConfiguredPackage _ (SourcePackage _ _ (LocalUnpackedPackage _ _) _)
_ _ _))] -> do
configurePackage verbosity
platform (compilerInfo comp)
Expand Down Expand Up @@ -298,7 +298,7 @@ planLocalPackage verbosity comp platform configFlags configExFlags
localPkg = SourcePackage {
srcpkgPackageId = packageId pkg,
srcpkgDescription = pkg,
srcpkgSource = LocalUnpackedPackage ".",
srcpkgSource = LocalUnpackedPackage "." Nothing,
srcpkgDescrOverride = Nothing
}

Expand Down
2 changes: 1 addition & 1 deletion cabal-install/src/Distribution/Client/Fetch.hs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ checkTarget verbosity target = case target of

fetchPackage :: Verbosity -> RepoContext -> PackageLocation a -> IO ()
fetchPackage verbosity repoCtxt pkgsrc = case pkgsrc of
LocalUnpackedPackage _dir -> return ()
LocalUnpackedPackage _dir _cabalFile -> return ()
LocalTarballPackage _file -> return ()

RemoteTarballPackage _uri _ ->
Expand Down
10 changes: 5 additions & 5 deletions cabal-install/src/Distribution/Client/FetchUtils.hs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ import qualified Hackage.Security.Client as Sec
--
isFetched :: UnresolvedPkgLoc -> IO Bool
isFetched loc = case loc of
LocalUnpackedPackage _dir -> return True
LocalUnpackedPackage _dir _ -> return True
LocalTarballPackage _file -> return True
RemoteTarballPackage _uri local -> return (isJust local)
RepoTarballPackage repo pkgid _ -> doesFileExist (packageFile repo pkgid)
Expand All @@ -91,8 +91,8 @@ isFetched loc = case loc of
checkFetched :: UnresolvedPkgLoc
-> IO (Maybe ResolvedPkgLoc)
checkFetched loc = case loc of
LocalUnpackedPackage dir ->
return (Just $ LocalUnpackedPackage dir)
LocalUnpackedPackage dir cabalFile ->
return (Just $ LocalUnpackedPackage dir cabalFile)
LocalTarballPackage file ->
return (Just $ LocalTarballPackage file)
RemoteTarballPackage uri (Just file) ->
Expand Down Expand Up @@ -126,8 +126,8 @@ fetchPackage :: Verbosity
-> UnresolvedPkgLoc
-> IO ResolvedPkgLoc
fetchPackage verbosity repoCtxt loc = case loc of
LocalUnpackedPackage dir ->
return (LocalUnpackedPackage dir)
LocalUnpackedPackage dir cabalFile ->
return (LocalUnpackedPackage dir cabalFile)
LocalTarballPackage file ->
return (LocalTarballPackage file)
RemoteTarballPackage uri (Just file) ->
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/src/Distribution/Client/Get.hs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ get verbosity repoCtxt _ getFlags userTargets = do
die' verbosity $ "The 'get' command does no yet support targets "
++ "that are remote source repositories."

LocalUnpackedPackage _ ->
LocalUnpackedPackage _ _ ->
error "Distribution.Client.Get.unpack: the impossible happened."
where
usePristine :: Bool
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/src/Distribution/Client/IndexUtils.hs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ readRepoIndex verbosity repoCtxt repo idxState =
, srcpkgDescription = pkgdesc
, srcpkgSource = case pkgEntry of
NormalPackage _ _ _ _ -> RepoTarballPackage repo pkgid Nothing
BuildTreeRef _ _ _ path _ -> LocalUnpackedPackage path
BuildTreeRef _ _ _ path _ -> LocalUnpackedPackage path Nothing
, srcpkgDescrOverride = case pkgEntry of
NormalPackage _ _ pkgtxt _ -> Just pkgtxt
_ -> Nothing
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/src/Distribution/Client/Install.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1229,7 +1229,7 @@ installLocalPackage verbosity pkgid location distPref installPkg =

case location of

LocalUnpackedPackage dir ->
LocalUnpackedPackage dir _cabalFile ->
installPkg (Just dir)

RemoteSourceRepoPackage _repo dir ->
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/src/Distribution/Client/ProjectBuilding.hs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ rebuildTargetsDryRun distDirLayout@DistDirLayout{..} shared =
case mloc of
Nothing -> return BuildStatusDownload

Just (LocalUnpackedPackage srcdir) ->
Just (LocalUnpackedPackage srcdir _) ->
-- For the case of a user-managed local dir, irrespective of the
-- build style, we build from that directory and put build
-- artifacts under the shared dist directory.
Expand Down
17 changes: 15 additions & 2 deletions cabal-install/src/Distribution/Client/ProjectConfig.hs
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,17 @@ reportParseResult verbosity filetype filename (OldParser.ParseFailed err) =
--
data ProjectPackageLocation =
ProjectPackageLocalCabalFile FilePath
| ProjectPackageLocalDirectory FilePath FilePath -- dir and .cabal file
| ProjectPackageLocalDirectory FilePath FilePath
-- ^ In @'ProjectPackageLocalDirectory' directory cabalFile@, @directory@
-- is expected to be a filepath relative to the root of the
-- project. Similarly, @cabalFile@ is also relative to the root of
-- of the project.
--
-- Consequentially, the following should hold:
--
-- > takeDirectory cabalFile == directory
-- > directory </> takeFileName cabalFile == cabalFile
--
| ProjectPackageLocalTarball FilePath
| ProjectPackageRemoteTarball URI
| ProjectPackageRemoteRepo SourceRepoList
Expand Down Expand Up @@ -1041,7 +1051,10 @@ readSourcePackageLocalDirectory
readSourcePackageLocalDirectory verbosity dir cabalFile = do
monitorFiles [monitorFileHashed cabalFile]
root <- askRoot
let location = LocalUnpackedPackage (root </> dir)
-- cabalFile is not required to be just the filename, but might be
-- relative to the project root.
let cabalFileName = takeFileName cabalFile
let location = LocalUnpackedPackage (root </> dir) (Just cabalFileName)
liftIO $ fmap (mkSpecificSourcePackage location)
. readSourcePackageCabalFile verbosity cabalFile
=<< BS.readFile (root </> cabalFile)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -583,12 +583,7 @@ resolveTargets selectPackageTargets selectComponentTarget
= Left (TargetProblemNoSuchPackage pkgid)

checkTarget (TargetPackage _ pkgids _)
= error ("TODO: add support for multiple packages in a directory. Got\n"
++ unlines (map prettyShow pkgids))
-- For the moment this error cannot happen here, because it gets
-- detected when the package config is being constructed. This case
-- will need handling properly when we do add support.
--
= Left (TargetProblemNotSinglePackage pkgids)
-- TODO: how should this use case play together with the
-- '--cabal-file' option of 'configure' which allows using multiple
-- .cabal files for a single package?
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/src/Distribution/Client/ProjectPlanOutput.hs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ encodePlanAsJson distDirLayout elaboratedInstallPlan elaboratedSharedConfig =
packageLocationToJ :: PackageLocation (Maybe FilePath) -> J.Value
packageLocationToJ pkgloc =
case pkgloc of
LocalUnpackedPackage local ->
LocalUnpackedPackage local _cabalFile ->
J.object [ "type" J..= J.String "local"
, "path" J..= J.String local
]
Expand Down
4 changes: 2 additions & 2 deletions cabal-install/src/Distribution/Client/ProjectPlanning.hs
Original file line number Diff line number Diff line change
Expand Up @@ -2096,8 +2096,8 @@ elaborateInstallPlan verbosity platform compiler compilerprogdb pkgConfigDB
shouldBeLocal :: PackageSpecifier (SourcePackage (PackageLocation loc)) -> Maybe PackageId
shouldBeLocal NamedPackage{} = Nothing
shouldBeLocal (SpecificSourcePackage pkg) = case srcpkgSource pkg of
LocalUnpackedPackage _ -> Just (packageId pkg)
_ -> Nothing
LocalUnpackedPackage _ _ -> Just (packageId pkg)
_ -> Nothing

-- | Given a 'ElaboratedPlanPackage', report if it matches a 'ComponentName'.
matchPlanPkg :: (ComponentName -> Bool) -> ElaboratedPlanPackage -> Bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ dataDirEnvVarForPackage distDirLayout pkg =
, Just $ srcPath (elabPkgSourceLocation pkg)
</> dataDir (elabPkgDescription pkg))
where
srcPath (LocalUnpackedPackage path) = path
srcPath (LocalUnpackedPackage path _cabalFile) = path
srcPath (LocalTarballPackage _path) = unpackedPath
srcPath (RemoteTarballPackage _uri _localTar) = unpackedPath
srcPath (RepoTarballPackage _repo _packageId _localTar) = unpackedPath
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/src/Distribution/Client/ScriptUtils.hs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ fakeProjectSourcePackage projectRoot = sourcePackage
sourcePackage = SourcePackage
{ srcpkgPackageId = fakePackageId
, srcpkgDescription = genericPackageDescription
, srcpkgSource = LocalUnpackedPackage projectRoot
, srcpkgSource = LocalUnpackedPackage projectRoot Nothing
, srcpkgDescrOverride = Nothing
}
genericPackageDescription = emptyGenericPackageDescription
Expand Down
1 change: 1 addition & 0 deletions cabal-install/src/Distribution/Client/TargetProblem.hs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ data TargetProblem a
-- directly then of course it can.
| TargetProblemNoSuchPackage PackageId
| TargetProblemNoSuchComponent PackageId ComponentName
| TargetProblemNotSinglePackage [PackageId]

-- | A custom target problem
| CustomTargetProblem a
Expand Down
57 changes: 43 additions & 14 deletions cabal-install/src/Distribution/Client/TargetSelector.hs
Original file line number Diff line number Diff line change
Expand Up @@ -471,10 +471,13 @@ resolveTargetSelectors (KnownTargets{knownPackagesAll = []}) [] _ =
resolveTargetSelectors (KnownTargets{knownPackagesPrimary = []}) [] ckf =
([TargetSelectorNoTargetsInCwd (ckf /= Just ExeKind) ], [])

resolveTargetSelectors (KnownTargets{knownPackagesPrimary}) [] _ =
([], [TargetPackage TargetImplicitCwd pkgids Nothing])
resolveTargetSelectors (KnownTargets{knownPackagesPrimary}) [] _
| [(pkgid, _)] <- knownPackages =
([], [TargetPackage TargetImplicitCwd [pkgid] Nothing])
| otherwise =
([TargetSelectorNotSinglePackage knownPackages], [])
where
pkgids = [ pinfoId | KnownPackage{pinfoId} <- knownPackagesPrimary ]
knownPackages = [ (pinfoId, pinfoPackageFile) | KnownPackage{pinfoId, pinfoPackageFile} <- knownPackagesPrimary ]

resolveTargetSelectors knowntargets targetStrs mfilter =
partitionEithers
Expand Down Expand Up @@ -594,6 +597,11 @@ data TargetSelectorProblem
| TargetSelectorUnrecognised String
-- ^ Syntax error when trying to parse a target string.
| TargetSelectorNoCurrentPackage TargetString
| TargetSelectorNotSinglePackage [(PackageId, Maybe (FilePath, FilePath))]
-- ^ Some PackageIds were duplicated.
-- Track them and their provenance if available.
-- The tuple is expected to hold an absolute and a relative filepath to
-- the respective cabal file.
| TargetSelectorNoTargetsInCwd Bool
-- ^ bool that flags when it is acceptable to suggest "all" as a target
| TargetSelectorNoTargetsInProject
Expand Down Expand Up @@ -816,7 +824,29 @@ reportTargetSelectorProblems verbosity problems = do
case [ () | TargetSelectorNoTargetsInProject <- problems ] of
[] -> return ()
_:_ ->
die' verbosity noPackageErrorMessage

case [ knownPkgs | TargetSelectorNotSinglePackage knownPkgs <- problems ] of
[] -> return ()
mknownPkgs -> case concat mknownPkgs of
[] ->
die' verbosity noPackageErrorMessage
pkgids ->
die' verbosity $
"Multiple packages have been found:\n"
++ unlines ( map (uncurry prettyAmbiguousPackageProvenance) pkgids)

case [ t | TargetSelectorNoScript t <- problems ] of
[] -> return ()
target:_ ->
die' verbosity $
"The script '" ++ showTargetString target ++ "' does not exist, "
++ "and only script targets may contain whitespace characters or end "
++ "with ':'"

fail "reportTargetSelectorProblems: internal error"
where
noPackageErrorMessage =
"There is no <pkgname>.cabal package file or cabal.project file. "
++ "To build packages locally you need at minimum a <pkgname>.cabal "
++ "file. You can use 'cabal init' to create one.\n"
Expand All @@ -826,15 +856,12 @@ reportTargetSelectorProblems verbosity problems = do
++ "packages in your project and all other build configuration. "
++ "See the Cabal user guide for full details."

case [ t | TargetSelectorNoScript t <- problems ] of
[] -> return ()
target:_ ->
die' verbosity $
"The script '" ++ showTargetString target ++ "' does not exist, "
++ "and only script targets may contain whitespace characters or end "
++ "with ':'"

fail "reportTargetSelectorProblems: internal error"
prettyAmbiguousPackageProvenance :: PackageId -> Maybe (FilePath, FilePath) -> String
prettyAmbiguousPackageProvenance pkgid provenance =
prettyShow pkgid ++ (case provenance of
Nothing -> mempty
Just (_, relativeCabalFile) -> " defined in: " ++ relativeCabalFile)


----------------------------------
Expand Down Expand Up @@ -1738,7 +1765,9 @@ data KnownPackage =
KnownPackage {
pinfoId :: PackageId,
pinfoDirectory :: Maybe (FilePath, FilePath),
-- ^ Absolute path and relative path of the root directory of this package.
pinfoPackageFile :: Maybe (FilePath, FilePath),
-- ^ Absolute path and relative path to the .cabal file of this package.
pinfoComponents :: [KnownComponent]
}
| KnownPackageName {
Expand Down Expand Up @@ -1814,12 +1843,12 @@ collectKnownPackageInfo dirActions@DirActions{..}
(pkgdir, pkgfile) <-
case loc of
--TODO: local tarballs, remote tarballs etc
LocalUnpackedPackage dir -> do
LocalUnpackedPackage dir cabalFile -> do
dirabs <- canonicalizePath dir
dirrel <- makeRelativeToCwd dirActions dirabs
--TODO: ought to get this earlier in project reading
let fileabs = dirabs </> prettyShow (packageName pkg) <.> "cabal"
filerel = dirrel </> prettyShow (packageName pkg) <.> "cabal"
let fileabs = dirabs </> fromMaybe (prettyShow (packageName pkg) <.> "cabal") cabalFile
filerel = dirrel </> fromMaybe (prettyShow (packageName pkg) <.> "cabal") cabalFile
exists <- doesFileExist fileabs
return ( Just (dirabs, dirrel)
, if exists then Just (fileabs, filerel) else Nothing
Expand Down
8 changes: 4 additions & 4 deletions cabal-install/src/Distribution/Client/Targets.hs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ import qualified Data.ByteString.Lazy as BS
import qualified Distribution.Client.GZipUtils as GZipUtils
import qualified Distribution.Compat.CharParsing as P
import System.FilePath
( takeExtension, dropExtension, takeDirectory, splitPath )
( takeExtension, dropExtension, takeDirectory, splitPath, takeFileName )
import System.Directory
( doesFileExist, doesDirectoryExist )
import Network.URI
Expand Down Expand Up @@ -348,12 +348,12 @@ expandUserTarget verbosity userTarget = case userTarget of
in return [PackageTargetNamedFuzzy name props userTarget]

UserTargetLocalDir dir ->
return [ PackageTargetLocation (LocalUnpackedPackage dir) ]
return [ PackageTargetLocation (LocalUnpackedPackage dir Nothing) ]

UserTargetLocalCabalFile file -> do
let dir = takeDirectory file
_ <- tryFindPackageDesc verbosity dir (localPackageError dir) -- just as a check
return [ PackageTargetLocation (LocalUnpackedPackage dir) ]
return [ PackageTargetLocation (LocalUnpackedPackage dir (Just $ takeFileName file)) ]

UserTargetLocalTarball tarballFile ->
return [ PackageTargetLocation (LocalTarballPackage tarballFile) ]
Expand Down Expand Up @@ -392,7 +392,7 @@ readPackageTarget verbosity = traverse modifyLocation
modifyLocation :: ResolvedPkgLoc -> IO UnresolvedSourcePackage
modifyLocation location = case location of

LocalUnpackedPackage dir -> do
LocalUnpackedPackage dir _cabalFile -> do
pkg <- tryFindPackageDesc verbosity dir (localPackageError dir) >>=
readGenericPackageDescription verbosity
return SourcePackage
Expand Down
11 changes: 9 additions & 2 deletions cabal-install/src/Distribution/Client/Types/PackageLocation.hs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,15 @@ type ResolvedPkgLoc = PackageLocation FilePath

data PackageLocation local =

-- | An unpacked package in the given dir, or current dir
LocalUnpackedPackage FilePath
-- | An unpacked package in the given directory.
-- @'LocalUnpackedPackage' directory mCabalFile@, where @directory@ is an
-- absolute filepath which points to the root of the *package*.
-- I.e., the filepath points to the directory where the '.cabal' file of
-- this package can be found.
-- If @mCabalFile@ is 'Nothing', then the '.cabal' file can be assumed to
-- be '<package-name>.cabal'.
-- Otherwise, @mCabalFile@ points to the '.cabal' file within @directory@.
LocalUnpackedPackage FilePath (Maybe FilePath)
Copy link
Member

@jneira jneira Feb 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this a record would suppose lot of changes? field names would help a lot in understand the two params


-- | A package as a tarball that's available as a local tarball
| LocalTarballPackage FilePath
Expand Down
2 changes: 1 addition & 1 deletion cabal-install/tests/IntegrationTests2.hs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ testTargetSelectorAmbiguous reportSubCase = do
mkpkgAt pkgidstr exes loc =
SourcePackage {
srcpkgPackageId = pkgid,
srcpkgSource = LocalUnpackedPackage loc,
srcpkgSource = LocalUnpackedPackage loc Nothing,
srcpkgDescrOverride = Nothing,
srcpkgDescription = GenericPackageDescription {
packageDescription = emptyPackageDescription { package = pkgid },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ testEmpty = do
testPassLocalPackage :: Assertion
testPassLocalPackage = do
let repoCtxt = error "repoCtxt undefined"
loc = LocalUnpackedPackage "a"
loc = LocalUnpackedPackage "a" Nothing
res <- asyncFetchPackages verbosity repoCtxt [loc] $ \downloadMap ->
waitAsyncFetchPackage verbosity downloadMap loc
res @?= LocalUnpackedPackage "a"
res @?= LocalUnpackedPackage "a" Nothing

testHttp :: Assertion
testHttp = withFakeRepoCtxt get200 $ \repoCtxt repo -> do
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module Main where

main :: IO ()
main = putStrLn "Hello, Haskell!"
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
packages: multiple-cabal-files.cabal multiple-cabal-files2.cabal
Loading