Skip to content

Foreign Library Stanza #518

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Foreign Library Stanza #518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## Unreleased
- Add support for `extra-files` (see #603)
- Add new section: `foreign-libraries`, for Cabal 2.0's `foreign-library`
stanzas (see #518)

## Changes in 0.38.0
- Generate `build-tool-depends` instead of `build-tools` starting with
Expand Down
12 changes: 12 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ library:
| `custom-setup` | · | | See [Custom setup](#custom-setup) | | |
| `flags` | `flag <name>` | | Map from flag name to flag (see [Flags](#flags)) | | |
| `library` | · | | See [Library fields](#library-fields) | | |
| `foreign-libraries` | `foreign-library <name>` | | Map from foreign library name to a dict of [Foreign library fields](#foreign-library-fields) and global top-level fields. | | UNRELEASED |
Copy link
Owner

@sol sol Mar 22, 2025

Choose a reason for hiding this comment

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

Documentation for foreign-library is missing here.

| `internal-libraries` | `library <name>` | | Map from internal library name to a dict of [library fields](#library-fields) and global top-level fields. | | `0.21.0` |
| `executables` | `executable <name>` | | Map from executable name to executable (see [Executable fields](#executable-fields)) | | |
| `executable` | `executable <package-name>` | | Shortcut for `executables: { package-name: ... }` | | `0.18.0` |
Expand Down Expand Up @@ -328,6 +329,17 @@ This is done to allow compatibility with a wider range of `Cabal` versions.
| `reexported-modules` | · | | |
| `signatures` | · | | |

#### Foreign library fields

| Hpack | Cabal | Default | Notes |
| --- | --- | --- | --- |
| `type` | . | | |
Copy link
Owner

@sol sol Mar 22, 2025

Choose a reason for hiding this comment

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

type can only be native-shared, so maybe we just remove the field?

| `lib-version-info` | . | | |
| `options` | . | | |
Copy link
Owner

@sol sol Mar 22, 2025

Choose a reason for hiding this comment

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

The Cabal documentation says "Currently we only support standalone here." and "Currently, standalone must be used on Windows and must not be used on any other platform.

So again here, maybe we just remove that field and produce .cabal files that meet those requirements?

| `mod-def-file` | . | | |
Copy link
Owner

@sol sol Mar 22, 2025

Choose a reason for hiding this comment

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

Even though Cabal and GHC support this, I don't really see a use case for it. Why the hell would you add a foreign export declaration for something, only to later hide it with a mod-def-file?

My verdict is, unless we can come up with a sensible use case for this, remove it.

| `other-modules` | · | All modules in `source-dirs` less `main` less any modules mentioned in `when` | |
Copy link
Owner

@sol sol Mar 22, 2025

Choose a reason for hiding this comment

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

Is the documentation actually correct?

| `generated-other-modules` | | | Added to `other-modules` and `autogen-modules`. Since `0.23.0`.
Copy link
Owner

@sol sol Mar 22, 2025

Choose a reason for hiding this comment

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

I think this was just copied from executables! At least the Since annotation is not needed anymore.


#### Executable fields

| Hpack | Cabal | Default | Notes |
Expand Down
108 changes: 105 additions & 3 deletions src/Hpack/Config.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ module Hpack.Config (
, Section(..)
, Library(..)
, Executable(..)
, ForeignLibrary(..)
, Conditional(..)
, Cond(..)
, Flag(..)
Expand Down Expand Up @@ -167,6 +168,7 @@ package name version = Package {
, packageCustomSetup = Nothing
, packageLibrary = Nothing
, packageInternalLibraries = mempty
, packageForeignLibraries = mempty
, packageExecutables = mempty
, packageTests = mempty
, packageBenchmarks = mempty
Expand All @@ -176,6 +178,7 @@ package name version = Package {
renamePackage :: String -> Package -> Package
renamePackage name p@Package{..} = p {
packageName = name
, packageForeignLibraries = fmap (renameDependencies packageName name) packageForeignLibraries
Copy link
Owner

@sol sol Mar 22, 2025

Choose a reason for hiding this comment

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

Same here, internal libraries are not handled here. If internal libraries cannot depend on the main library, then we are actually good. I'm not sure what's the situation here.

I think currently nobody is using this code, so maybe not a big issue. We could potentially deprecate it.

, packageExecutables = fmap (renameDependencies packageName name) packageExecutables
, packageTests = fmap (renameDependencies packageName name) packageTests
, packageBenchmarks = fmap (renameDependencies packageName name) packageBenchmarks
Expand All @@ -194,6 +197,7 @@ renameDependencies old new sect@Section{..} = sect {sectionDependencies = (Depen
packageDependencies :: Package -> [(String, DependencyInfo)]
packageDependencies Package{..} = nub . sortBy (comparing (lexicographically . fst)) $
(concatMap deps packageExecutables)
++ (concatMap deps packageForeignLibraries)
Copy link
Owner

@sol sol Mar 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Not related to this pull request, but I think this code does not handle internal libraries correctly. I think we need a separate issue or PR for that.

++ (concatMap deps packageTests)
++ (concatMap deps packageBenchmarks)
++ maybe [] deps packageLibrary
Expand Down Expand Up @@ -237,6 +241,29 @@ instance Semigroup LibrarySection where
, librarySectionSignatures = librarySectionSignatures a <> librarySectionSignatures b
}

data ForeignLibrarySection = ForeignLibrarySection {
foreignLibrarySectionType :: Last String
, foreignLibrarySectionLibVersionInfo :: Last String
, foreignLibrarySectionOptions :: Maybe (List String)
, foreignLibrarySectionModDefFile :: Last String
, foreignLibrarySectionOtherModules :: Maybe (List Module)
, foreignLibrarySectionGeneratedOtherModules :: Maybe (List Module)
} deriving (Eq, Show, Generic, FromValue)

instance Monoid ForeignLibrarySection where
mempty = ForeignLibrarySection mempty mempty Nothing mempty Nothing Nothing
mappend = (<>)

instance Semigroup ForeignLibrarySection where
a <> b = ForeignLibrarySection {
foreignLibrarySectionType = foreignLibrarySectionType a <> foreignLibrarySectionType b
, foreignLibrarySectionLibVersionInfo = foreignLibrarySectionLibVersionInfo a <> foreignLibrarySectionLibVersionInfo b
, foreignLibrarySectionOptions = foreignLibrarySectionOptions a <> foreignLibrarySectionOptions b
, foreignLibrarySectionModDefFile = foreignLibrarySectionModDefFile a <> foreignLibrarySectionModDefFile b
, foreignLibrarySectionOtherModules = foreignLibrarySectionOtherModules a <> foreignLibrarySectionOtherModules b
, foreignLibrarySectionGeneratedOtherModules = foreignLibrarySectionGeneratedOtherModules a <> foreignLibrarySectionGeneratedOtherModules b
}

data ExecutableSection = ExecutableSection {
executableSectionMain :: Alias 'True "main-is" (Last FilePath)
, executableSectionOtherModules :: Maybe (List Module)
Expand Down Expand Up @@ -570,10 +597,12 @@ type SectionConfigWithDefaults asmSources cSources cxxSources jsSources a = Prod

type PackageConfigWithDefaults asmSources cSources cxxSources jsSources = PackageConfig_
(SectionConfigWithDefaults asmSources cSources cxxSources jsSources LibrarySection)
(SectionConfigWithDefaults asmSources cSources cxxSources jsSources ForeignLibrarySection)
(SectionConfigWithDefaults asmSources cSources cxxSources jsSources ExecutableSection)

type PackageConfig asmSources cSources cxxSources jsSources = PackageConfig_
(WithCommonOptions asmSources cSources cxxSources jsSources LibrarySection)
(WithCommonOptions asmSources cSources cxxSources jsSources ForeignLibrarySection)
(WithCommonOptions asmSources cSources cxxSources jsSources ExecutableSection)

data PackageVersion = PackageVersion {unPackageVersion :: String}
Expand All @@ -584,7 +613,7 @@ instance FromValue PackageVersion where
String s -> return (T.unpack s)
_ -> typeMismatch "Number or String" v

data PackageConfig_ library executable = PackageConfig {
data PackageConfig_ library foreignLib executable = PackageConfig {
packageConfigName :: Maybe String
, packageConfigVersion :: Maybe PackageVersion
, packageConfigSynopsis :: Maybe String
Expand All @@ -611,6 +640,8 @@ data PackageConfig_ library executable = PackageConfig {
, packageConfigCustomSetup :: Maybe CustomSetupSection
, packageConfigLibrary :: Maybe library
, packageConfigInternalLibraries :: Maybe (Map String library)
, packageConfigForeignLibraries :: Maybe (Map String foreignLib)
, packageConfigForeignLibrary :: Maybe foreignLib
Copy link
Owner

@sol sol Mar 22, 2025

Choose a reason for hiding this comment

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

So the idea here is that in addition to foreign-libraries, we also accept foreign-library where the name defaults to the package name.

I'm still undecided whether I think this is a good idea or not, but if you're gonna do that, then we need a test for it.

Copy link
Owner

@sol sol Mar 26, 2025

Choose a reason for hiding this comment

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

I guess it is actually a good idea, so let's add the test.

, packageConfigExecutable :: Maybe executable
, packageConfigExecutables :: Maybe (Map String executable)
, packageConfigTests :: Maybe (Map String executable)
Expand Down Expand Up @@ -639,13 +670,17 @@ traversePackageConfig :: Traversal PackageConfig
traversePackageConfig t p@PackageConfig{..} = do
library <- traverse (traverseWithCommonOptions t) packageConfigLibrary
internalLibraries <- traverseNamedConfigs t packageConfigInternalLibraries
foreignLibrary <- traverse (traverseWithCommonOptions t) packageConfigForeignLibrary
foreignLibraries <- traverseNamedConfigs t packageConfigForeignLibraries
executable <- traverse (traverseWithCommonOptions t) packageConfigExecutable
executables <- traverseNamedConfigs t packageConfigExecutables
tests <- traverseNamedConfigs t packageConfigTests
benchmarks <- traverseNamedConfigs t packageConfigBenchmarks
return p {
packageConfigLibrary = library
, packageConfigInternalLibraries = internalLibraries
, packageConfigForeignLibrary = foreignLibrary
, packageConfigForeignLibraries = foreignLibraries
, packageConfigExecutable = executable
, packageConfigExecutables = executables
, packageConfigTests = tests
Expand Down Expand Up @@ -739,6 +774,7 @@ addPathsModuleToGeneratedModules pkg
| otherwise = pkg {
packageLibrary = fmap mapLibrary <$> packageLibrary pkg
, packageInternalLibraries = fmap mapLibrary <$> packageInternalLibraries pkg
, packageForeignLibraries = fmap mapForeignLibrary <$> packageForeignLibraries pkg
, packageExecutables = fmap mapExecutable <$> packageExecutables pkg
, packageTests = fmap mapExecutable <$> packageTests pkg
, packageBenchmarks = fmap mapExecutable <$> packageBenchmarks pkg
Expand All @@ -755,6 +791,15 @@ addPathsModuleToGeneratedModules pkg
where
generatedModules = libraryGeneratedModules lib

mapForeignLibrary :: ForeignLibrary -> ForeignLibrary
mapForeignLibrary foreignLibrary
| pathsModule `elem` foreignLibraryOtherModules foreignLibrary = foreignLibrary {
foreignLibraryGeneratedModules = if pathsModule `elem` generatedModules then generatedModules else pathsModule : generatedModules
}
| otherwise = foreignLibrary
where
generatedModules = foreignLibraryGeneratedModules foreignLibrary

mapExecutable :: Executable -> Executable
mapExecutable executable
| pathsModule `elem` executableOtherModules executable = executable {
Expand Down Expand Up @@ -836,6 +881,7 @@ ensureRequiredCabalVersion inferredLicense pkg@Package{..} = pkg {
, makeVersion [3,14] <$ guard (not (null packageExtraFiles))
, packageLibrary >>= libraryCabalVersion
, internalLibsCabalVersion packageInternalLibraries
, foreignLibsCabalVersion packageForeignLibraries
, executablesCabalVersion packageExecutables
, executablesCabalVersion packageTests
, executablesCabalVersion packageBenchmarks
Expand All @@ -859,6 +905,15 @@ ensureRequiredCabalVersion inferredLicense pkg@Package{..} = pkg {
where
versions = libraryCabalVersion <$> Map.elems internalLibraries

foreignLibsCabalVersion :: Map String (Section ForeignLibrary) -> Maybe CabalVersion
foreignLibsCabalVersion = foldr max Nothing . map foreignLibCabalVersion . Map.elems

foreignLibCabalVersion :: Section ForeignLibrary -> Maybe CabalVersion
foreignLibCabalVersion sect = maximum [
makeVersion [2,0] <$ guard (foreignLibraryHasGeneratedModules sect)
, sectionCabalVersion (concatMap getForeignLibraryModules) sect
]

executablesCabalVersion :: Map String (Section Executable) -> Maybe CabalVersion
executablesCabalVersion = foldr max Nothing . map executableCabalVersion . Map.elems

Expand All @@ -868,6 +923,9 @@ ensureRequiredCabalVersion inferredLicense pkg@Package{..} = pkg {
, sectionCabalVersion (concatMap getExecutableModules) sect
]

foreignLibraryHasGeneratedModules :: Section ForeignLibrary -> Bool
foreignLibraryHasGeneratedModules = any (not . null . foreignLibraryGeneratedModules)

executableHasGeneratedModules :: Section Executable -> Bool
executableHasGeneratedModules = any (not . null . executableGeneratedModules)

Expand Down Expand Up @@ -1034,6 +1092,7 @@ data Package = Package {
, packageCustomSetup :: Maybe CustomSetup
, packageLibrary :: Maybe (Section Library)
, packageInternalLibraries :: Map String (Section Library)
, packageForeignLibraries :: Map String (Section ForeignLibrary)
, packageExecutables :: Map String (Section Executable)
, packageTests :: Map String (Section Executable)
, packageBenchmarks :: Map String (Section Executable)
Expand All @@ -1054,6 +1113,15 @@ data Library = Library {
, librarySignatures :: [String]
} deriving (Eq, Show)

data ForeignLibrary = ForeignLibrary {
foreignLibraryType :: Maybe String
, foreignLibraryLibVersionInfo :: Maybe String
, foreignLibraryOptions :: Maybe [String]
, foreignLibraryModDefFile :: Maybe String
, foreignLibraryOtherModules :: [Module]
, foreignLibraryGeneratedModules :: [Module]
} deriving (Eq, Show)

data Executable = Executable {
executableMain :: Maybe FilePath
, executableOtherModules :: [Module]
Expand Down Expand Up @@ -1177,13 +1245,17 @@ expandSectionDefaults
expandSectionDefaults formatYamlParseError userDataDir dir p@PackageConfig{..} = do
library <- traverse (expandDefaults formatYamlParseError userDataDir dir) packageConfigLibrary
internalLibraries <- traverse (traverse (expandDefaults formatYamlParseError userDataDir dir)) packageConfigInternalLibraries
foreignLibrary <- traverse (expandDefaults formatYamlParseError userDataDir dir) packageConfigForeignLibrary
foreignLibraries <- traverse (traverse (expandDefaults formatYamlParseError userDataDir dir)) packageConfigForeignLibraries
executable <- traverse (expandDefaults formatYamlParseError userDataDir dir) packageConfigExecutable
executables <- traverse (traverse (expandDefaults formatYamlParseError userDataDir dir)) packageConfigExecutables
tests <- traverse (traverse (expandDefaults formatYamlParseError userDataDir dir)) packageConfigTests
benchmarks <- traverse (traverse (expandDefaults formatYamlParseError userDataDir dir)) packageConfigBenchmarks
return p{
packageConfigLibrary = library
, packageConfigInternalLibraries = internalLibraries
, packageConfigForeignLibrary = foreignLibrary
, packageConfigForeignLibraries = foreignLibraries
, packageConfigExecutable = executable
, packageConfigExecutables = executables
, packageConfigTests = tests
Expand Down Expand Up @@ -1241,25 +1313,28 @@ type GlobalOptions = CommonOptions AsmSources CSources CxxSources JsSources Empt

toPackage_ :: (MonadIO m, Warnings m, State m) => FilePath -> Product GlobalOptions (PackageConfig AsmSources CSources CxxSources JsSources) -> m Package
toPackage_ dir (Product g PackageConfig{..}) = do
foreignLibraryMap <- toExecutableMap packageName packageConfigForeignLibraries packageConfigForeignLibrary
executableMap <- toExecutableMap packageName packageConfigExecutables packageConfigExecutable
let
globalVerbatim = commonOptionsVerbatim g
globalOptions = g {commonOptionsVerbatim = Nothing}

executableNames = maybe [] Map.keys executableMap
componentNames = maybe [] Map.keys executableMap ++ maybe [] Map.keys foreignLibraryMap
Copy link
Owner

@sol sol Mar 22, 2025

Choose a reason for hiding this comment

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

I don't think this is correct.

Copy link
Owner

@sol sol Mar 26, 2025

Choose a reason for hiding this comment

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

Specifically I think the old is just fine, we don't need to update this.


toSect :: (Warnings m, Monoid a) => WithCommonOptions AsmSources CSources CxxSources JsSources a -> m (Section a)
toSect = toSection packageName executableNames . first ((mempty <$ globalOptions) <>)
toSect = toSection packageName componentNames . first ((mempty <$ globalOptions) <>)

toSections :: (Warnings m, Monoid a) => Maybe (Map String (WithCommonOptions AsmSources CSources CxxSources JsSources a)) -> m (Map String (Section a))
toSections = maybe (return mempty) (traverse toSect)

toLib = toLibrary dir packageName
toForeignLibraries = toSections >=> traverse (toForeignLibrary dir packageName)
toExecutables = toSections >=> traverse (toExecutable dir packageName)

mLibrary <- traverse (toSect >=> toLib) packageConfigLibrary
internalLibraries <- toSections packageConfigInternalLibraries >>= traverse toLib

foreignLibraries <- toForeignLibraries foreignLibraryMap
executables <- toExecutables executableMap
tests <- toExecutables packageConfigTests
benchmarks <- toExecutables packageConfigBenchmarks
Expand All @@ -1269,6 +1344,7 @@ toPackage_ dir (Product g PackageConfig{..}) = do
missingSourceDirs <- liftIO $ nub . sort <$> filterM (fmap not <$> doesDirectoryExist . (dir </>)) (
maybe [] sectionSourceDirs mLibrary
++ concatMap sectionSourceDirs internalLibraries
++ concatMap sectionSourceDirs foreignLibraries
++ concatMap sectionSourceDirs executables
++ concatMap sectionSourceDirs tests
++ concatMap sectionSourceDirs benchmarks
Expand Down Expand Up @@ -1328,6 +1404,7 @@ toPackage_ dir (Product g PackageConfig{..}) = do
, packageCustomSetup = mCustomSetup
, packageLibrary = mLibrary
, packageInternalLibraries = internalLibraries
, packageForeignLibraries = foreignLibraries
, packageExecutables = executables
, packageTests = tests
, packageBenchmarks = benchmarks
Expand Down Expand Up @@ -1437,6 +1514,9 @@ getMentionedLibraryModules (LibrarySection _ _ exposedModules generatedExposedMo
getLibraryModules :: Library -> [Module]
getLibraryModules Library{..} = libraryExposedModules ++ libraryOtherModules

getForeignLibraryModules :: ForeignLibrary -> [Module]
getForeignLibraryModules ForeignLibrary{..} = foreignLibraryOtherModules

getExecutableModules :: Executable -> [Module]
getExecutableModules Executable{..} = executableOtherModules

Expand Down Expand Up @@ -1522,6 +1602,28 @@ fromLibrarySectionPlain LibrarySection{..} = Library {
, librarySignatures = fromMaybeList librarySectionSignatures
}

getMentionedForeignLibraryModules :: ForeignLibrarySection -> [Module]
getMentionedForeignLibraryModules (ForeignLibrarySection _ _ _ _ otherModules generatedModules)=
fromMaybeList (otherModules <> generatedModules)

toForeignLibrary :: (MonadIO m, State m) => FilePath -> String -> Section ForeignLibrarySection -> m (Section ForeignLibrary)
toForeignLibrary dir packageName_ =
inferModules dir packageName_ getMentionedForeignLibraryModules getForeignLibraryModules fromForeignLibrarySection (fromForeignLibrarySection [])
where
fromForeignLibrarySection :: [Module] -> [Module] -> ForeignLibrarySection -> ForeignLibrary
fromForeignLibrarySection pathsModule inferableModules ForeignLibrarySection{..} =
(ForeignLibrary
(getLast foreignLibrarySectionType)
(getLast foreignLibrarySectionLibVersionInfo)
(fromList <$> foreignLibrarySectionOptions)
(getLast foreignLibrarySectionModDefFile)
(otherModules ++ generatedModules)
generatedModules
)
where
otherModules = maybe (inferableModules ++ pathsModule) fromList foreignLibrarySectionOtherModules
generatedModules = maybe [] fromList foreignLibrarySectionGeneratedOtherModules

getMentionedExecutableModules :: ExecutableSection -> [Module]
getMentionedExecutableModules (ExecutableSection (Alias (Last main)) otherModules generatedModules)=
maybe id (:) (toModule . Path.fromFilePath <$> main) $ fromMaybeList (otherModules <> generatedModules)
Expand Down
Loading