'cabal build' implies 'cabal configure'; 'cabal test' and 'cabal bench' imply 'cabal build' #997

ttuegel · 2012-08-10T22:58:45Z

These commits improve the user interface of the cabal-install tool by automatically running 'configure' before 'build' as necessary and by running build before test and bench. The end result is that cabal unpack foo; cd foo; cabal test very probably does what the user expects.

The commits for running build before test and bench are largely plumbing. The first commit, which runs configure before build, has a bit of convoluted logic to decide what ConfigFlags to use when reconfiguring.

tibbe · 2012-08-10T23:57:01Z

I'm really excited about this change. Could you please rebase the edits you make so you still end up with three commits? Thanks!

ttuegel · 2012-08-11T19:21:45Z

@tibbe I made the changes you requested, so that calling 'cabal build' alone does not enable tests or benchmarks. I didn't realize it would delete your comments when I pushed the rebased edits (sorry!). I think I addressed all of the issues, though.

tibbe · 2012-08-11T20:27:15Z

Something is not right. This works:

$ cd unordered-containers
$ cabal configure --enable-tests && cabal build
...

but this doesn't

$ ~/src/cabal/cabal-install/dist/build/cabal/cabal test
Configuring with default flags. If this fails, please run configure manually.
Resolving dependencies...
Configuring unordered-containers-0.2.1.0...
cabal: At least the following dependencies are missing:
hashable >=1.0.1.1 && <1.2,
test-framework >=0.3.3 && <0.7,
test-framework-hunit -any,
test-framework-quickcheck2 >=0.2.9 && <0.3

~/src/cabal/cabal-install/dist/build/cabal/cabal is the cabal-install binary that includes your patches. Try getting unordered-containers from https://github.com/tibbe/unordered-containers/ and try it for yourself.

ttuegel · 2012-08-11T21:25:18Z

I can't seem to reproduce this problem; in both cases, unordered-containers builds correctly for me! Is it possible that you have some changes from another branch mixed in? I've also tried pulling in the upstream changes since I originally wrote the patches, but it still works fine.

tibbe · 2012-08-12T04:54:50Z

I just repulled your branch to make sure and the problem remains. This his how I built the cabal-install binary:

cd Cabal
ghc -O Setup.hs
./Setup configure --user
./Setup build
cd ../cabal-install
ghc -O Setup.hs
./Setup configure --user
./Setup build

Here's the output of cabal test using this binary:


$ ~/src/cabal/cabal-install/dist/build/cabal/cabal test -v3
Configuring with default flags. If this fails, please run configure manually.
searching for ghc in path.
found ghc at /usr/bin/ghc
("/usr/bin/ghc",["--numeric-version"])
/usr/bin/ghc is version 7.4.1
looking for tool "ghc-pkg" near compiler in /usr/bin
found ghc-pkg in /usr/bin/ghc-pkg
("/usr/bin/ghc-pkg",["--version"])
/usr/bin/ghc-pkg is version 7.4.1
("/usr/bin/ghc",["--supported-languages"])
("/usr/bin/ghc",["--info"])
Reading installed packages...
("/usr/bin/ghc-pkg",["dump","--global","-v0"])
("/usr/bin/ghc",["--print-libdir"])
Reading available packages...
Choosing modular solver.
Resolving dependencies...
[__0] trying: unordered-containers-0.2.1.0
[__1] trying: base-4.5.0.0/installed-a73...
[__2] trying: rts-1.0/installedbuil...
[__3] trying: integer-gmp-0.4.0.0/installed-ec8...
[__4] trying: ghc-prim-0.2.0.0/installed-bd2...
[__5] trying: unordered-containers-0.2.1.0:-debug
[__6] rejecting: unordered-containers-0.2.1.0:!test (global constraint
requires opposite flag selection)
[__6] rejecting: unordered-containers-0.2.1.0:*test (unknown package:
test-framework-hunit)
[__0] fail (backjumping, conflict set: test-framework-hunit,
unordered-containers, unordered-containers-0.2.1.0:test)
Could not resolve dependencies:
trying: unordered-containers-0.2.1.0
rejecting: unordered-containers-0.2.1.0:!test (global constraint requires
opposite flag selection)
rejecting: unordered-containers-0.2.1.0:*test (unknown package:
test-framework-hunit)
Using internal setup method with build-type Simple and args:
["configure","--verbose=3","--builddir=dist","--ghc","--program-prefix=","--program-suffix=","--enable-library-vanilla","--disable-library-profiling","--disable-shared","--disable-executable-dynamic","--disable-executable-profiling","--enable-optimization","--enable-library-for-ghci","--disable-split-objs","--enable-executable-stripping","--global","--enable-tests","--disable-library-coverage","--disable-benchmarks"]
Configuring unordered-containers-0.2.1.0...
creating dist
searching for ghc in path.
found ghc at /usr/bin/ghc
("/usr/bin/ghc",["--numeric-version"])
/usr/bin/ghc is version 7.4.1
looking for tool "ghc-pkg" near compiler in /usr/bin
found ghc-pkg in /usr/bin/ghc-pkg
("/usr/bin/ghc-pkg",["--version"])
/usr/bin/ghc-pkg is version 7.4.1
("/usr/bin/ghc",["--supported-languages"])
("/usr/bin/ghc",["--info"])
Reading installed packages...
("/usr/bin/ghc-pkg",["dump","--global","-v0"])
("/usr/bin/ghc",["--print-libdir"])
cabal: At least the following dependencies are missing:
hashable >=1.0.1.1 && <1.2,
test-framework >=0.3.3 && <0.7,
test-framework-hunit -any,
test-framework-quickcheck2 >=0.2.9 && <0.3

ttuegel · 2012-08-12T13:36:25Z

I started from scratch by deleting my ~/.ghc and cloning a new copy of my repo, but I still cannot reproduce this problem (i.e., cabal test in a clean unordered-containers check-out builds and runs successfully). There is only one thing I can think of: If I follow your commands exactly, cabal-install fails to build because Cabal-1.17 is not installed (you didn't do ./Setup install for Cabal). So, you are linking cabal-install against whatever version of the Cabal library you have installed. I have no idea why that would matter, since my patches only modify cabal-install, but I'm at a loss otherwise.

tibbe · 2012-08-12T15:06:02Z

There is only one thing I can think of: If I follow your commands exactly, cabal-install fails to build because Cabal-1.17 is not installed (you didn't do ./Setup install for Cabal). So, you are linking cabal-install against whatever version of the Cabal library you have installed.

I don't think so. The --package-db flag makes my cabal-install binary be linked against the version of the Cabal library in the same repo. Just in case I tried to ./Setup install the Cabal library and it made no difference.

I think I've identified the issue: cabal test seems to imply ./Setup configure --global, not ./Setup configure --user which is what cabal configure normally implies. If I install the dependencies that cabal complains about (e.g. hashable) globally things work. It seems like you're not (re-)configuring in the same way as the cabal configure command used to.

kosmikus · 2012-08-12T17:43:50Z

Can you explain what the difference between this pull request and request #995 is?

tibbe · 2012-08-12T17:48:10Z

@kosmikus This pull request is a subset of #995. #995 would have e.g. cabal test take all the options take by cabal configure. After a discussion with @dcoutts we decided to not do that for now (we'll discuss that issue at ICFP) and only do the basic thing of having e.g. cabal test imply build and configure, but requiring that the user use configure explicitly if she wants to set some options.

I will close #995 for now, until we agree on the design.

ttuegel · 2012-08-12T18:50:32Z

Yes, that's it! Thank you. The problem is that I'm configuring with defaultConfigFlags which turn configUserInstall off by default. I'm having trouble right now figuring out where the actual defaults for ConfigFlags are set, but as soon as I do, I'll upload a fix.

(I feel silly that I didn't notice this, because I've run into this problem before; the actual default flags for commands are only incidentally related to their default flags in Distribution.Simple.Setup. The line I referred to above even has a TODO that says configUserInstall should be True by default.)

ttuegel · 2012-08-12T19:22:03Z

OK, I fixed the commits. Apparently, the correct way to get the default ConfigFlags is to use mempty, rather than defaultConfigFlags. I don't want to run into this problem again, so I'm also using mempty instead of defaultBuildFlags.

ttuegel · 2012-08-12T21:16:57Z

cabal-install/Main.hs

+        verbosity = fromFlagOrDefault normal (buildVerbosity buildFlags)
+
+    reconfigure verbosity distPref mempty [] globalFlags (const Nothing)
+    build verbosity distPref buildFlags extraArgs


This is a place where Cabal and cabal-install are very inconsistent. If you run ./Setup build, the default BuildFlags are defaultBuildFlags. Prior to my patches, cabal build used mempty for the default BuildFlags because buildCommand was invoked using wrapperAction, which overrides each command's default flags, using mempty instead.

Here's the rub: on L143, I use buildCommand unmodified, which means that with my patches in their current state, cabal build uses defaultBuildFlags. However, cabal test and cabal bench use mempty. In summary,

Before:

./setup build uses defaultBuildFlags

cabal build uses mempty

After:

./Setup build uses defaultBuildFlags

cabal build uses defaultBuildFlags

cabal test and cabal bench use mempty

I assume I should change the behaviour of my patch to respect the prior behaviour of cabal build and ignore the behaviour of ./Setup build entirely?

I assume I should change the behaviour of my patch to respect the prior behaviour of cabal build and ignore the behaviour of ./Setup build entirely?

Yes please.

ttuegel · 2012-08-12T21:41:54Z

You're right, the commit message does need to be changed. I'll do that as soon as I get feedback on the defaultBuildFlags vs mempty issue so that my line comment doesn't get erased.

The problem with cabal build vs cabal configure before was not using the correct default flags, which I have now correctly set to mempty for the default ConfigFlags. There are two remaining issues:

(Edit: Actually, this isn't relevant; see below.) I need to load saved ConfigFlags, ConfigExFlags and GlobalFlags as in configureAction. This can all be done in reconfigure because none of these flag settings are used in cabal build, cabal test, or cabal bench outside the configuration process.
(Which just occurred to me...) Because ConfigExFlags are not saved in the LocalBuildInfo as ConfigFlags are, every reconfiguration uses defaultConfigExFlags. This influences, among other things, preferences and the dependency solver used. Silently reconfiguring with different ConfigExFlags than those originally specified to cabal configure could lead to build failures or, even worse, subtle runtime errors cause by building against an unexpected version of some dependency (for example).

I have to admit that when the second issue occurred to me, I didn't initially think it was a big deal because I assumed that any errors would cause configuration to fail, prompting the user to reconfigure manually and avoiding later disaster. That is obviously not the case. Now I'm thinking that this whole thing is for nought without a way to save ConfigExFlags inside or alongside the LocalBuildInfo... I will reflect on this.

Edit: On second thought, I don't actually need to do anything about the first problem. Those flags only affect configureAction, which already loads them, and my patches only reconfigure through configureAction, so everything is fine in that respect.

Change the behavior of 'cabal build' to automatically run 'cabal configure' if the package in the current directory has never been configured or if the configuration is outdated. In the former case, use the default options or in the latter case, use the most recently used options.

Package will be reconfigured with test suites enabled, if necessary.

tibbe · 2012-08-13T05:15:53Z

Regarding (2) above, is the behavior you've implemented any worse than what cabal build (which you are no longer touching any worse than before? In other words, since cabal build already had a reconfigure behavior that led to ConfigExFlags being dropped, will cabal test and cabal bench just behave as cabal build, or are there other complications?

I think we should definitely start a discussion with Duncan about (2) e.g. on the [email protected] mailing list, but as long the new behavior of cabal test/bench matches the current behavior of cabal build, when it comes to configuring, then I think we're OK to merge this.

Package will be reconfigured with benchmarks enabled, if necessary.

ttuegel · 2012-08-13T16:46:04Z

In other words, since cabal build already had a reconfigure behavior that led to ConfigExFlags being dropped, will cabal test and cabal bench just behave as cabal build, or are there other complications?

No, cabal test and cabal bench will behave the same way as cabal build always has.

I just pushed the updated patches. While I was patching, I noticed that there was a corner case where the user was not advised to reconfigure manually if automatic reconfiguration failed, so I took care of that, too.

I'm writing up that message to the mailing list now, so that will go out later today.

tibbe · 2012-08-13T16:51:47Z

No, cabal test and cabal bench will behave the same way as cabal build always has.

In that case I think you can go ahead and push your patches to the master branch (which will close this pull request). The new behavior is an improvement to the past behavior and we can always improve it further in the future, when we resolved the ConfigExFlags issue.

'cabal build' reconfigures if necessary or configures with default options if the package has never been configured. 'cabal test' and 'cabal bench' configure or reconfigure and rebuild the package as necessary.

tibbe mentioned this pull request Aug 12, 2012

Automatically configure and build before 'cabal test' and 'cabal bench' #995

Closed

ttuegel reviewed Aug 12, 2012
View reviewed changes

ttuegel added 2 commits August 12, 2012 16:50

'cabal test' implies 'cabal build'.

ce5d926

Package will be reconfigured with test suites enabled, if necessary.

'cabal bench' implies 'cabal build'.

d438ceb

Package will be reconfigured with benchmarks enabled, if necessary.

ttuegel merged commit 36d61d6 into haskell:master Aug 13, 2012

'cabal build' implies 'cabal configure'; 'cabal test' and 'cabal bench' imply 'cabal build' #997

'cabal build' implies 'cabal configure'; 'cabal test' and 'cabal bench' imply 'cabal build' #997

Uh oh!

Conversation

ttuegel commented Aug 10, 2012

Uh oh!

tibbe commented Aug 10, 2012

Uh oh!

ttuegel commented Aug 11, 2012

Uh oh!

tibbe commented Aug 11, 2012

Uh oh!

ttuegel commented Aug 11, 2012

Uh oh!

tibbe commented Aug 12, 2012

Uh oh!

ttuegel commented Aug 12, 2012

Uh oh!

tibbe commented Aug 12, 2012

Uh oh!

kosmikus commented Aug 12, 2012

Uh oh!

tibbe commented Aug 12, 2012

Uh oh!

ttuegel commented Aug 12, 2012

Uh oh!

ttuegel commented Aug 12, 2012

Uh oh!

ttuegel Aug 12, 2012

Choose a reason for hiding this comment

Uh oh!

tibbe Aug 13, 2012

Choose a reason for hiding this comment

Uh oh!

ttuegel commented Aug 12, 2012

Uh oh!

tibbe commented Aug 13, 2012

Uh oh!

ttuegel commented Aug 13, 2012

Uh oh!

tibbe commented Aug 13, 2012

Uh oh!

Uh oh!