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

unnecessary rebuilding when tests share modules declared in other-modules #1446

Closed
coreyoconnor opened this issue Aug 29, 2013 · 13 comments
Closed
Assignees

Comments

@coreyoconnor
Copy link

  1. clone https://github.com/coreyoconnor/vty
  2. cabal configure --enable-tests
  3. cabal test
  4. cabal test

The expectation is that no compilation is performed for the second "cabal test". On my system using Cabal 1.18 and cabal-install 1.18 this is not the case. The modules in common to each test suite listen in other-modules are all recompiled.

These modules are not part of the vty library because they have dependencies specific to testing. Which should not be part of the vty depdnencies.

This may be a configuration issue in the vty.cabal. I just have had no luck figuring out what the issue is :-)

@23Skidoo
Copy link
Member

I'll try to reproduce, but I suspect that this is a side-effect of delegating all dependency management to ghc --make. It is the main cause why do-nothing builds are slow.

@23Skidoo
Copy link
Member

BTW, your package does not build on GHC < 7.6 because it uses the MultiWayIf extension.

@coreyoconnor
Copy link
Author

Hm. Thanks for the info. I think I added that and then didn't even use it! Fixing.

@coreyoconnor
Copy link
Author

I removed the MultiWayIf pragma in jtdaugherty/vty@b3d84a5.

@23Skidoo
Copy link
Member

Yes, there definitely seems to be some unnecessary compilation going on. Can be reproduced by just running cabal build twice. It looks like this happens only with test suites of type detailed-0.9. @ttuegel, can you take a look?

@ttuegel
Copy link
Member

ttuegel commented Aug 30, 2013

I was just looking at this myself. The stub executable for detailed-0.9 test suites is automatically regenerated every time you cabal build or cabal test. GHC sees the updated timestamp on the file and thinks it has changed, and so ghc --make rebuilds all the modules needlessly. We should check for the generated file first and only update it if its contents need to be changed. In fact, all the generated modules should do this, but as far as I can tell they don't (you could probably trigger the same bug by including a Paths_ module, e.g.).

@23Skidoo
Copy link
Member

@ttuegel Yes, but this shouldn't trigger the recompilation of all dependencies, only of the stub module. That's what I'm seeing:

[1 of 7] Compiling Verify           ( test/Verify.hs, dist/build/Verify.o )
[2 of 7] Compiling Verify.Graphics.Vty.DisplayRegion ( test/Verify/Graphics/Vty/DisplayRegion.hs, dist/build/Verify/Graphics/Vty/DisplayRegion.o )
[3 of 7] Compiling Verify.Graphics.Vty.Attributes ( test/Verify/Graphics/Vty/Attributes.hs, dist/build/Verify/Graphics/Vty/Attributes.o )
[4 of 7] Compiling Verify.Graphics.Vty.Image ( test/Verify/Graphics/Vty/Image.hs, dist/build/Verify/Graphics/Vty/Image.o )
[5 of 7] Compiling Verify.Graphics.Vty.Picture ( test/Verify/Graphics/Vty/Picture.hs, dist/build/Verify/Graphics/Vty/Picture.o )
[6 of 7] Compiling Verify.Graphics.Vty.Span ( test/Verify/Graphics/Vty/Span.hs, dist/build/Verify/Graphics/Vty/Span.o )

We should check for the generated file first and only update it if its contents need to be changed.

There's rewriteFile in D.S.Utils for doing this.

@ttuegel
Copy link
Member

ttuegel commented Aug 30, 2013

@23Skidoo I see that, about rewriteFile, and in fact it's only the test stub module that has the problem.

@23Skidoo
Copy link
Member

I think I know what happens here. We put all intermediate files in dist/build, so for example the Verify.{o,hi} files for verify-layers-span-generation end up overwriting the files with the same name for verify-utf8-width. In theory this should be fine, since the source files are the same. But since the library names are different, this changes the interface hash, which triggers recompilation.

Here's what I see when I run ghc --show-iface dist/build/Verify/Graphics/Vty/Image.hi:

interface verify-layers-span-generation-5.0.0:Verify.Graphics.Vty.Image [orphan module] 7061

Proposed solution: put intermediate files of each component in its own subdirectory of dist/build.

@23Skidoo
Copy link
Member

Proposed solution: put intermediate files of each component in its own subdirectory of dist/build.

I think that this change is too dangerous to go in the next release. Also, this behaviour should also happen in 1.16, so this is not a 1.18 regression. So I'm removing the cabal-install-1.18 milestone.

@23Skidoo
Copy link
Member

Smaller test case: https://gist.github.com/23Skidoo/6388894

On each cabal build TriggerUnnecessaryRebuild is recompiled, though nothing has changed:

Building issue1446-0.1.0.0...
Preprocessing executable 'issue1446' for issue1446-0.1.0.0...
Preprocessing test suite 'issue1446test1' for issue1446-0.1.0.0...
[1 of 2] Compiling TriggerUnnecessaryRebuild ( TriggerUnnecessaryRebuild.hs, dist/build/TriggerUnnecessaryRebuild.o )
In-place registering issue1446test1-0.1.0.0...
[1 of 1] Compiling Main             ( dist/build/issue1446test1Stub/issue1446test1Stub-tmp/issue1446test1Stub.hs, dist/build/issue1446test1Stub/issue1446test1Stub-tmp/Main.o )
Linking dist/build/issue1446test1Stub/issue1446test1Stub ...
Preprocessing test suite 'issue1446test2' for issue1446-0.1.0.0...
[1 of 2] Compiling TriggerUnnecessaryRebuild ( TriggerUnnecessaryRebuild.hs, dist/build/TriggerUnnecessaryRebuild.o )
In-place registering issue1446test2-0.1.0.0...
[1 of 1] Compiling Main             ( dist/build/issue1446test2Stub/issue1446test2Stub-tmp/issue1446test2Stub.hs, dist/build/issue1446test2Stub/issue1446test2Stub-tmp/Main.o )
Linking dist/build/issue1446test2Stub/issue1446test2Stub ...

@ghost ghost assigned 23Skidoo Sep 7, 2013
@nh2
Copy link
Member

nh2 commented Sep 11, 2013

Might be related (or have some effect), but do not fix the problem:

@ttuegel ttuegel modified the milestones: cabal-install-1.24, cabal-install-1.22 Apr 23, 2015
@ezyang
Copy link
Contributor

ezyang commented Jan 11, 2016

I'm pretty sure I accidentally fixed this bug in #3015.

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

No branches or pull requests

5 participants