haskell / cabal

Official upstream development repository for Cabal and cabal-install
https://haskell.org/cabal
Other
1.62k stars 697 forks source link

Setup.hs configure with autoconf build type runs configure script in the wrong working directory #7677

Open TerrorJack opened 3 years ago

TerrorJack commented 3 years ago

Describe the bug When building a package with build-type: Configure, the configure script is run with the build directory as the working directory, rather than the source directory. If the configure script attempts to include a header file relative to the source directory, it doesn't work.

To Reproduce Minimal repro: https://github.com/TerrorJack/cabal-configure-pwd-issue-repro. The repro above is minimized from the base package. Run autoreconf -i to generate configure script.

Running cabal act-as-setup --build-type=Configure configure will fail to execute the configure script:

Configuring cabal-configure-pwd-issue-repro-0.1.0.0...
configure: WARNING: unrecognized options: --with-compiler
checking for gcc... /usr/bin/gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether /usr/bin/gcc accepts -g... yes
checking for /usr/bin/gcc option to accept ISO C89... none needed
checking for struct MD5Context... no
configure: error: internal error

While running ./configure manually will succeed.

System information

fgaz commented 3 years ago

Does it work if you list the file in extra-source-files as described here?

jneira commented 2 years ago

@TerrorJack had you got the chance of try the above recommendation?

bgamari commented 2 years ago

I can confirm the behavior seen by @TerrorJack and can also confirm that adding the files needed by the configure script to extra-source-files does not avoid the issue.

gbaz commented 2 years ago

That said -- probably better to copy extra-src-files than to change the cwd of the configure script, no? Since the latter will possibly generate things relative to where it is run, and those should be generated in the build dir?

bgamari commented 2 years ago

Honestly I'm not sure; afterall, it's not clear (to me, at least) how the build is supposed to find things generated by the configure script if they are dumped into the build directory.

bgamari commented 2 years ago

FWIW, I tried implementing @gbaz's suggestion:

diff --git a/Cabal/src/Distribution/Simple.hs b/Cabal/src/Distribution/Simple.hs
index be2d71d56..b603451a7 100644
--- a/Cabal/src/Distribution/Simple.hs
+++ b/Cabal/src/Distribution/Simple.hs
@@ -57,12 +57,14 @@ module Distribution.Simple (
   ) where

 import Control.Exception (try)
+import Control.Monad (forM)

 import Prelude ()
 import Distribution.Compat.Prelude

 -- local
 import Distribution.Simple.Compiler
+import Distribution.Simple.Glob
 import Distribution.Simple.UserHooks
 import Distribution.Package
 import Distribution.PackageDescription
@@ -613,6 +615,14 @@ autoconfUserHooks
               = do let verbosity = fromFlag (configVerbosity flags)
                        baseDir lbi' = fromMaybe ""
                                       (takeDirectory <$> cabalFilePath lbi')
+
+                   -- We guarantee that extra-source-files are available to the
+                   -- configure script. See #7677.
+                   srcFiles <- fmap concat $ forM (extraSrcFiles pkg_descr) $ \pat -> do
+                       matchDirFileGlob verbosity (specVersion pkg_descr) (baseDir lbi) pat
+                   copyFiles verbosity (buildDir lbi)
+                     [(baseDir lbi, fname) | fname <- srcFiles]
+
                    confExists <- doesFileExist $ (baseDir lbi) </> "configure"
                    if confExists
                      then runConfigureScript verbosity

Unfortunately this turns up a different problem: Cabal's "duplicate header" checks end up finding any headers present in extra-source-files, resulting in their deletion. For instance, in GHC's ghc-bignum I see:

Warning: Duplicate header found in
/opt/exp/ghc/ghc-landing/_build/stage1/libraries/ghc-bignum/build/include/WordSize.h
and libraries/ghc-bignum/include/WordSize.h; removing
libraries/ghc-bignum/include/WordSize.h

Any ideas on how to address this would be appreciated.

bgamari commented 2 years ago

To unstick myself I have adopted a variant on the above patch which deletes the copied extra-source-files from the build directory after configure has been run. This is very awkward but nevertheless works.

gbaz commented 2 years ago

the duplicate header check is frightening:

https://github.com/haskell/cabal/blob/ba45b8e6d5a0220fc97bc9779d173b9dc91f731f/Cabal/src/Distribution/Simple/Configure.hs#L1753

Given the comment there and that I can't see changing it, I suppose the proposed patch with deletion would be an acceptable PR.

I suppose we could try running the configure with a different pwd as well, but I sort of fear what regressions that might induce.

bgamari commented 2 years ago

I suppose we could try running the configure with a different pwd as well, but I sort of fear what regressions that might induce.

Yes, I agree that the probability of regressions is substantial. Moreover, I agree that the running configure in the source directory would be not ideal as it may dirty the user's tree. Ultimately I think that copying and deleting extra-source-files is probably the best that we can do. I'll put up my patch tomorrow.