Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ tests = testGroup "Distribution.Utils.Structured"
, testCase "GenericPackageDescription" $
md5Check (Proxy :: Proxy GenericPackageDescription) 0x8d8f340f10a58b8d8a87bf42213dac89
, testCase "LocalBuildInfo" $
md5Check (Proxy :: Proxy LocalBuildInfo) 0xbb22c3258d3092f31e992bc093d09170
md5Check (Proxy :: Proxy LocalBuildInfo) 0x618ab257e99d0b21c617e1f8c39a5a4b
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is LocalBuildInfo changing?

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea

Copy link
Member

@fgaz fgaz Aug 24, 2023

Choose a reason for hiding this comment

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

Try rebasing, it should disappear

Copy link
Collaborator

Choose a reason for hiding this comment

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

🤔

Copy link
Member

Choose a reason for hiding this comment

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

@andreabedini I'm saying that because this appears to be based on an old commit with a different structured hash (0xbb22... doesn't appear anywhere in HEAD)

Copy link
Member Author

Choose a reason for hiding this comment

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

@fgaz rebasing on current 3.10 you mean?

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind, I grepped the wrong branch 🙈

#endif
]

Expand Down
124 changes: 71 additions & 53 deletions Cabal/src/Distribution/Simple/GHC.hs
Original file line number Diff line number Diff line change
Expand Up @@ -705,35 +705,45 @@ buildOrReplLib mReplFlags verbosity numJobs pkg_descr lbi lib clbi = do
| filename <- cxxSources libBi]

-- build any C sources
unless (not has_code || null (cSources libBi)) $ do
let (cSrcs', others) = partition (\filepath -> ".c"`isSuffixOf` filepath) (cSources libBi)
unless (not has_code || null cSrcs') $ do
info verbosity "Building C Sources..."
sequence_
[ do let baseCcOpts = Internal.componentCcGhcOptions verbosity implInfo
lbi libBi clbi relLibTargetDir filename
vanillaCcOpts = if isGhcDynamic
-- Dynamic GHC requires C sources to be built
-- with -fPIC for REPL to work. See #2207.
then baseCcOpts { ghcOptFPic = toFlag True }
else baseCcOpts
profCcOpts = vanillaCcOpts `mappend` mempty {
ghcOptProfilingMode = toFlag True,
ghcOptObjSuffix = toFlag "p_o"
}
sharedCcOpts = vanillaCcOpts `mappend` mempty {
ghcOptFPic = toFlag True,
ghcOptDynLinkMode = toFlag GhcDynamicOnly,
ghcOptObjSuffix = toFlag "dyn_o"
}
odir = fromFlag (ghcOptObjDir vanillaCcOpts)
createDirectoryIfMissingVerbose verbosity True odir
let runGhcProgIfNeeded ccOpts = do
needsRecomp <- checkNeedsRecompilation filename ccOpts
when needsRecomp $ runGhcProg ccOpts
runGhcProgIfNeeded vanillaCcOpts
unless forRepl $
whenSharedLib forceSharedLib (runGhcProgIfNeeded sharedCcOpts)
unless forRepl $ whenProfLib (runGhcProgIfNeeded profCcOpts)
| filename <- cSources libBi]
unless (null others) $ do
let files = intercalate ", " others
let libraryName = case libName lib of
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have showLibraryName

Copy link
Member Author

Choose a reason for hiding this comment

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

The output is unsatisfying. Especially because it is:

"library '" ++ prettyShow name ++ "'"

So the output would be:

"The following files listed in library 'lib2''s c-sources will not be used:

The double single quotes are not a mistake.

And if it's the main library, it only outputs "library", which breaks the flow of the sentence.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is

libraryNameStanza :: LibraryName -> String
libraryNameStanza LMainLibName       = "library"
libraryNameStanza (LSubLibName name) = "library " ++ prettyShow name

any better? Otherwise, it's ok.

Copy link
Member

@fgaz fgaz Aug 24, 2023

Choose a reason for hiding this comment

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

Or rephrase as "The following files listed in the c-sources of the library 'lib2' will not be used"

Copy link
Member Author

Choose a reason for hiding this comment

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

@fgaz I don't understand what this suggestion is addressing, sorry

Copy link
Member

Choose a reason for hiding this comment

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

If you phrase it like that, you can use showLibraryName

Copy link
Member Author

Choose a reason for hiding this comment

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

But we lose the "main" bit, which is fairly important.

Copy link
Member

Choose a reason for hiding this comment

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

Good point. I wonder if showLibraryName produces confusing messages elsewhere too. Maybe we should add "main" there too...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes indeed, we will have to formalise a bit more the presentation layer for cabal-install.

LMainLibName -> "the main library"
LSubLibName name -> "library " <> prettyShow name
warn verbosity $ unlines
[ "The following files listed in " <> libraryName <> "'s c-sources will not be used: " <> files <> "."
, "Header files should be in the 'include' or 'install-include' stanza."
, "See https://cabal.readthedocs.io/en/3.10/cabal-package.html#pkg-field-includes"
]
forM_ cSrcs' $ \filename -> do
let baseCcOpts = Internal.componentCcGhcOptions verbosity implInfo
lbi libBi clbi relLibTargetDir filename
vanillaCcOpts = if isGhcDynamic
-- Dynamic GHC requires C sources to be built
-- with -fPIC for REPL to work. See #2207.
then baseCcOpts { ghcOptFPic = toFlag True }
else baseCcOpts
profCcOpts = vanillaCcOpts `mappend` mempty {
ghcOptProfilingMode = toFlag True,
ghcOptObjSuffix = toFlag "p_o"
}
sharedCcOpts = vanillaCcOpts `mappend` mempty {
ghcOptFPic = toFlag True,
ghcOptDynLinkMode = toFlag GhcDynamicOnly,
ghcOptObjSuffix = toFlag "dyn_o"
}
odir = fromFlag (ghcOptObjDir vanillaCcOpts)
createDirectoryIfMissingVerbose verbosity True odir
let runGhcProgIfNeeded ccOpts = do
needsRecomp <- checkNeedsRecompilation filename ccOpts
when needsRecomp $ runGhcProg ccOpts
runGhcProgIfNeeded vanillaCcOpts
unless forRepl $
whenSharedLib forceSharedLib (runGhcProgIfNeeded sharedCcOpts)
unless forRepl $ whenProfLib (runGhcProgIfNeeded profCcOpts)

-- build any JS sources
unless (not has_code || not hasJsSupport || null (jsSources libBi)) $ do
Expand Down Expand Up @@ -1527,32 +1537,40 @@ gbuild verbosity numJobs pkg_descr lbi bm clbi = do
| filename <- cxxSrcs ]

-- build any C sources
unless (null cSrcs) $ do
info verbosity "Building C Sources..."
sequence_
[ do let baseCcOpts = Internal.componentCcGhcOptions verbosity implInfo
let (cSrcs', others) = partition (\filepath -> ".c"`isSuffixOf` filepath) cSrcs
unless (null cSrcs') $ do
info verbosity "Building C Sources..."
unless (null others) $ do
let files = intercalate ", " others
let currentComponentName = gbuildName bm
warn verbosity $ unlines
[ "The following files listed in " <> currentComponentName <> "'s c-sources will not be used: " <> files <> "."
, "Header files should be in the 'include' or 'install-include' stanza."
, "See https://cabal.readthedocs.io/en/3.10/cabal-package.html#pkg-field-includes"
]
forM_ cSrcs' $ \filename -> do
let baseCcOpts = Internal.componentCcGhcOptions verbosity implInfo
lbi bnfo clbi tmpDir filename
vanillaCcOpts = if isGhcDynamic
-- Dynamic GHC requires C sources to be built
-- with -fPIC for REPL to work. See #2207.
then baseCcOpts { ghcOptFPic = toFlag True }
else baseCcOpts
profCcOpts = vanillaCcOpts `mappend` mempty {
ghcOptProfilingMode = toFlag True
}
sharedCcOpts = vanillaCcOpts `mappend` mempty {
ghcOptFPic = toFlag True,
ghcOptDynLinkMode = toFlag GhcDynamicOnly
}
opts | needProfiling = profCcOpts
| needDynamic = sharedCcOpts
| otherwise = vanillaCcOpts
odir = fromFlag (ghcOptObjDir opts)
createDirectoryIfMissingVerbose verbosity True odir
needsRecomp <- checkNeedsRecompilation filename opts
when needsRecomp $
runGhcProg opts
| filename <- cSrcs ]
let vanillaCcOpts = if isGhcDynamic
-- Dynamic GHC requires C sources to be built
-- with -fPIC for REPL to work. See #2207.
then baseCcOpts { ghcOptFPic = toFlag True }
else baseCcOpts
let profCcOpts = vanillaCcOpts `mappend` mempty {
ghcOptProfilingMode = toFlag True
}
let sharedCcOpts = vanillaCcOpts `mappend` mempty {
ghcOptFPic = toFlag True,
ghcOptDynLinkMode = toFlag GhcDynamicOnly
}
let opts | needProfiling = profCcOpts
| needDynamic = sharedCcOpts
| otherwise = vanillaCcOpts
let odir = fromFlag (ghcOptObjDir opts)
createDirectoryIfMissingVerbose verbosity True odir
needsRecomp <- checkNeedsRecompilation filename opts
when needsRecomp $
runGhcProg opts

-- TODO: problem here is we need the .c files built first, so we can load them
-- with ghci, but .c files can depend on .h files generated by ghc by ffi
Expand Down
4 changes: 4 additions & 0 deletions cabal-testsuite/PackageTests/CSourcesSanitisation/Main.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
module Main where

main :: IO ()
main = putStrLn "main"
25 changes: 25 additions & 0 deletions cabal-testsuite/PackageTests/CSourcesSanitisation/build.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# cabal build
Resolving dependencies...
Build profile: -w ghc-<GHCVER> -O1
In order, the following will be built:
- repro-0.1.0.0 (lib) (first run)
- repro-0.1.0.0 (exe:exec1) (first run)
- repro-0.1.0.0 (lib:lib2) (first run)
Configuring library for repro-0.1.0.0..
Preprocessing library for repro-0.1.0.0..
Building library for repro-0.1.0.0..
Warning: The following files listed in the main library's c-sources will not be used: cbits/gwinsz.h.
Header files should be in the 'include' or 'install-include' stanza.
See https://cabal.readthedocs.io/en/3.10/cabal-package.html#pkg-field-includes
Configuring executable 'exec1' for repro-0.1.0.0..
Preprocessing executable 'exec1' for repro-0.1.0.0..
Building executable 'exec1' for repro-0.1.0.0..
Warning: The following files listed in exec1's c-sources will not be used: cbits/gwinsz.h.
Header files should be in the 'include' or 'install-include' stanza.
See https://cabal.readthedocs.io/en/3.10/cabal-package.html#pkg-field-includes
Configuring library 'lib2' for repro-0.1.0.0..
Preprocessing library 'lib2' for repro-0.1.0.0..
Building library 'lib2' for repro-0.1.0.0..
Warning: The following files listed in library lib2's c-sources will not be used: cbits/gwinsz.h.
Header files should be in the 'include' or 'install-include' stanza.
See https://cabal.readthedocs.io/en/3.10/cabal-package.html#pkg-field-includes
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import Test.Cabal.Prelude

main = cabalTest $ do
cabal "build" []
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
packages: .
Empty file.
Empty file.
23 changes: 23 additions & 0 deletions cabal-testsuite/PackageTests/CSourcesSanitisation/repro.cabal
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
cabal-version: 3.0
name: repro
version: 0.1.0.0
build-type: Simple

library
default-language: Haskell2010
c-sources: cbits/gwinsz.h
cbits/gwinsz.c
build-depends: base

library lib2
default-language: Haskell2010
c-sources: cbits/gwinsz.h
cbits/gwinsz.c
build-depends: base

executable exec1
main-is: Main.hs
default-language: Haskell2010
c-sources: cbits/gwinsz.h
cbits/gwinsz.c
build-depends: base