Skip to content

cabal test mishandles autogenerated modules #3529

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

Closed
khumba opened this issue Jul 9, 2016 · 8 comments
Closed

cabal test mishandles autogenerated modules #3529

khumba opened this issue Jul 9, 2016 · 8 comments

Comments

@khumba
Copy link

khumba commented Jul 9, 2016

Cabal (or is this a GHC issue?) rebuilds auto-generated sources when running tests, but doesn't rebuild regular sources. If the regular sources depend on auto-generated sources, then there can be multiple conflicting builds of types, and GHC gets confused.

In this test case, the generated Box module exports newtype Box = Box Int, and the regular Debox provides an unboxing function. The tests for debox fail to compile, since the Box module is recompiled but Debox isn't:

$ cabal test
Package has never been configured. Configuring with default flags. If this
fails, please run configure manually.
Warning: The package list for 'hackage.haskell.org' does not exist. Run 'cabal
update' to download it.
Resolving dependencies...
[1 of 1] Compiling Main             ( dist/setup/setup.hs, dist/setup/Main.o )
Linking ./dist/setup/setup ...
Configuring hs-test-autogen-0.0.1...
Preprocessing library hs-test-autogen-0.0.1...
[1 of 2] Compiling Box              ( dist/build/autogen/Box.hs, dist/build/Box.o )
[2 of 2] Compiling Debox            ( src/Debox.hs, dist/build/Debox.o )
In-place registering hs-test-autogen-0.0.1...
Preprocessing test suite 'hs-test-autogen-test' for hs-test-autogen-0.0.1...
[1 of 2] Compiling Box              ( dist/build/autogen/Box.hs, dist/build/hs-test-autogen-test/hs-test-autogen-test-tmp/Box.o )
[2 of 2] Compiling Main             ( tests/Main.hs, dist/build/hs-test-autogen-test/hs-test-autogen-test-tmp/Main.o )

tests/Main.hs:12:47:
    Couldn't match expected type ‘hs-test-autogen-0.0.1:Box.Box’
                with actual type ‘Box’
    NB: ‘hs-test-autogen-0.0.1:Box.Box’
          is defined in ‘Box’ in package ‘hs-test-autogen-0.0.1’
        ‘Box’ is defined at dist/build/autogen/Box.hs:3:1-41
    In the first argument of ‘debox’, namely ‘(Box 42)’
    In the first argument of ‘(@?=)’, namely ‘debox (Box 42)’
@ezyang
Copy link
Contributor

ezyang commented Jul 9, 2016

Thanks for the report. As a quick question, which version of Cabal are you using?

@khumba
Copy link
Author

khumba commented Jul 9, 2016

Oops, forgot to mention specs :).

Gentoo amd64, GHC 7.10.3, Cabal 1.22.8.0, cabal-install 1.22.9.0.

@ezyang
Copy link
Contributor

ezyang commented Jul 9, 2016

So, I can't say this is entirely Cabal's fault because of the Custom setup script. But there is a bit of nastiness in pre Cabal-1.24 component handling. Is forcing Cabal-1.24 for your custom script possible? If so there might be a way to do this in a nice way.

@khumba
Copy link
Author

khumba commented Jul 9, 2016

I'd rather have a lower Cabal bound; it looks like most distros are still <=1.22 (is cabal install --user Cabal cabal-install something that works?), but I could require 1.24 if necessary (this is for Qtah which isn't even on Hackage yet).

This came up because I just moved my generated sources out of src and into dist/build/autogen (so alternatively I could revert this change). Things work fine when writing into src:

$ cabal test
Package has never been configured. Configuring with default flags. If this
fails, please run configure manually.
Warning: The package list for 'hackage.haskell.org' does not exist. Run 'cabal
update' to download it.
Resolving dependencies...
[1 of 1] Compiling Main             ( dist/setup/setup.hs, dist/setup/Main.o )
Linking ./dist/setup/setup ...
Configuring hs-test-autogen-0.0.1...
Preprocessing library hs-test-autogen-0.0.1...
[1 of 2] Compiling Box              ( src/Box.hs, dist/build/Box.o )
[2 of 2] Compiling Debox            ( src/Debox.hs, dist/build/Debox.o )
In-place registering hs-test-autogen-0.0.1...
Preprocessing test suite 'hs-test-autogen-test' for hs-test-autogen-0.0.1...
[1 of 1] Compiling Main             ( tests/Main.hs, dist/build/hs-test-autogen-test/hs-test-autogen-test-tmp/Main.o )
Linking dist/build/hs-test-autogen-test/hs-test-autogen-test ...
Running 1 test suites...
Test suite hs-test-autogen-test: RUNNING...
Test suite hs-test-autogen-test: PASS
Test suite logged to: dist/test/hs-test-autogen-0.0.1-hs-test-autogen-test.log
1 of 1 test suites (1 of 1 test cases) passed.

dist/build/autogen isn't just another hsSourceDirs? :)

I'll try Cabal 1.24 and see if it helps. Should switching to 1.24 be sufficient, or do I need to make some other changes too?

Does Cabal expose the dist/build/autogen path somehow, or do I have to hard code that, by the way?

@ezyang
Copy link
Contributor

ezyang commented Jul 9, 2016

No you will need to make other changes.

So let's talk about how this works in Cabal 1.24, because that is the version of Cabal when this all started making sense.

How do I get the directory where autogenerated modules go? Use Distribution.Simple.BuildPaths.autogenModulesDir.

Wait, this takes a ComponentLocalBuildInfo, why do I need this, and where does that come from? OK, get ready to run for the hills.

First, why do we need this? The answer is that in Cabal 1.24, we now maintain a separate autogenerated module directory for EACH component (e.g., test suite). So, to get the directory, we need to know what component we are building, which is identified by ComponentLocalBuildInfo.

OK, so how do we get this? If you have a LocalBuildInfo, you can get a ComponentLocalBuildInfo if you know the name of the component you want to generate files for. But what should you generate files for? This is an interesting question. Currently, you are generating the files after configuration. But Cabal itself writes autogenerated files (writeAutogenFiles) immediately prior to building. Cabal is probably wrong and should generate the files after configuration.

So, you should probably loop through every component in your post-configure hook (I guess allComponentsInBuildOrder is an OK function in this case) and generate your files for each of them. See also #2910.

dist/build/autogen isn't just another hsSourceDirs? Well, the autogenerated module path is hard-coded into the GHC command line we invoke. Honestly it's an implementation detail of Cabal and you're not supposed to rely on it. The contract the API gives you is that GHC will see autogenModulesDir.

Will this fix your problem? Yes probably, because in Cabal 1.24 we no longer put the build files for each component in the same place (which caused your proximal problem.)

@khumba
Copy link
Author

khumba commented Jul 10, 2016

Thanks for the detailed explanation! I'm glad to see things are improving (and getting more documentation) in Cabal land. (Had to upgrade to git master to check this out, looks like it didn't quite make it into 1.24; git merge-base Cabal-v1.24.0.0 90e908b8.)

I gave using autogenModulesDir a shot as you suggested (pushed to the test repo), but unfortunately it still didn't build. Building the generated code once per component in this case actually guarantees the problem, because src/Debox.hs only ever gets built once, so there are conflicting versions of Box.hs around. I either need to build Box.hs once (e.g. put it in src/), or build Debox.hs twice.

Although, since the autogenModulesDir for the library component is now src/ with the latest Cabal, it works to only generate there, although cleaning doesn't clean those files up automatically like with dist/build/autogen.

<snip>
Configuring hs-test-autogen-0.0.1...
*** Creating dist/build/autogen/Box.hs.
*** Creating dist/build/hs-test-autogen-test/autogen/Box.hs.
Preprocessing library hs-test-autogen-0.0.1...
[1 of 2] Compiling Box              ( src/Box.hs, dist/build/Box.o )
[2 of 2] Compiling Debox            ( src/Debox.hs, dist/build/Debox.o )
Preprocessing test suite 'hs-test-autogen-test' for hs-test-autogen-0.0.1...
[1 of 2] Compiling Box              ( dist/build/hs-test-autogen-test/autogen/Box.hs, dist/build/hs-test-autogen-test/hs-test-autogen-test-tmp/Box.o )
[2 of 2] Compiling Main             ( tests/Main.hs, dist/build/hs-test-autogen-test/hs-test-autogen-test-tmp/Main.o )

tests/Main.hs:12:47:
    Couldn't match expected type ‘hs-test-autogen-0.0.1:Box.Box’
                with actual type ‘Box’
    NB: ‘hs-test-autogen-0.0.1:Box.Box’
          is defined in ‘Box’ in package ‘hs-test-autogen-0.0.1’
        ‘Box’ is defined at
          dist/build/hs-test-autogen-test/autogen/Box.hs:3:1-41
    In the first argument of ‘debox’, namely ‘(Box 42)’
    In the first argument of ‘(@?=)’, namely ‘debox (Box 42)’

@ezyang
Copy link
Contributor

ezyang commented Jul 10, 2016

Oh! I blithely assumed you wanted a different Box for each component but reading your ticket more carefully that's what you DON'T want. So then the fix is very simple: ONLY generate Box for the library. It's as simple as using withLibLBI instead of withComponentsInBuildOrder (I guess since I suggested allComponentsInBuildOrder originally you'll have to rejigger your code slightly. Only very slightly.

@khumba
Copy link
Author

khumba commented Jul 10, 2016

Ah yes, great! Then this is the best kind of bug, EALREADY, and the fix for me is actually just git reset --hard HEAD^; I think I will keep things in src for compatibility with older Cabal for now, since that works, and doing the right thing with autogen is easy enough when it comes time to switch. Thanks a lot for your help @ezyang.

(I don't know where I got

Although, since the autogenModulesDir for the library component is now src/ with the latest Cabal ...

, must have had a stale file lying around.)

@khumba khumba closed this as completed Jul 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants