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

Cabal-1.22.6.0 incompatible with binary-0.8 #3003

Closed
hvr opened this issue Dec 28, 2015 · 57 comments
Closed

Cabal-1.22.6.0 incompatible with binary-0.8 #3003

hvr opened this issue Dec 28, 2015 · 57 comments

Comments

@hvr
Copy link
Member

hvr commented Dec 28, 2015

Here's what happens if you have binary-0.8 installed (this was with GHC 7.10.3):

$ cabal install Cabal
Resolving dependencies...
[ 1 of 79] Compiling Distribution.GetOpt ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/GetOpt.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/GetOpt.o )
[ 2 of 79] Compiling Distribution.Compat.Binary ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/Compat/Binary.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/Compat/Binary.o )
[ 3 of 79] Compiling Distribution.TestSuite ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/TestSuite.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/TestSuite.o )
[ 4 of 79] Compiling Distribution.Simple.PreProcess.Unlit ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/Simple/PreProcess/Unlit.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/Simple/PreProcess/Unlit.o )
[ 5 of 79] Compiling Distribution.Simple.CCompiler ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/Simple/CCompiler.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/Simple/CCompiler.o )
[ 6 of 79] Compiling Distribution.PackageDescription.Utils ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/PackageDescription/Utils.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/PackageDescription/Utils.o )
[ 7 of 79] Compiling Distribution.Compat.ReadP ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/Compat/ReadP.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/Compat/ReadP.o )
[ 8 of 79] Compiling Distribution.ReadE ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/ReadE.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/ReadE.o )
[ 9 of 79] Compiling Distribution.Verbosity ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/Verbosity.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/Verbosity.o )
[10 of 79] Compiling Distribution.Text ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/Text.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/Text.o )
[11 of 79] Compiling Distribution.ModuleName ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/ModuleName.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/ModuleName.o )
[12 of 79] Compiling Distribution.System ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/System.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/System.o )
[13 of 79] Compiling Distribution.Version ( /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/Version.hs, /tmp/cabal-tmp-7012/Cabal-1.22.6.0/dist/setup/Distribution/Version.o )

/tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/Version.hs:135:10:
    Duplicate instance declarations:
      instance Binary Version
        -- Defined at /tmp/cabal-tmp-7012/Cabal-1.22.6.0/Distribution/Version.hs:135:10
      instance [safe] Binary Version
        -- Defined in ‘binary-0.8.0.0:Data.Binary.Class’
Failed to install Cabal-1.22.6.0
cabal: Error: some packages failed to install:
Cabal-1.22.6.0 failed during the configure step. The exception was:
ExitFailure 1
@hvr
Copy link
Member Author

hvr commented Dec 28, 2015

Sadly, I don't know if we can fix the situation for pre-setup-depends cabals at all... :-/

/cc @edsko @dcoutts

@phadej
Copy link
Collaborator

phadej commented Dec 28, 2015

you can install Cabal if you do cabal configure --ghc-options=-DMIN_VERSION_binary_0_8_0=1;
unfortunately it's something you just have to know :(

@hvr
Copy link
Member Author

hvr commented Dec 28, 2015

@phadej it's a problem though as it breaks as soon as any install-plan automatically results in an (re)installation of Cabal... we can't expect cabal users to start manually setting CPP flags :-(

@hvr
Copy link
Member Author

hvr commented Dec 28, 2015

Btw, adding custom-setup section with setup-depends mirroring library's build-depends doesn't work either for Cabal... so it seems we can't even fix this easily for setup-depends aware Cabals :-(

Resolving dependencies...

Distribution/Simple/Utils.hs:202:18:
    Could not find module ‘Paths_Cabal’
    it is a hidden module in the package ‘Cabal-1.22.5.0@Cabal_3ux67khMI118y6VpwrFnXN’
    Use -v to see a list of the files searched for.

and adding Cabal as a dependency is a bad idea, as it sends the solver into an endless loop...

@phadej
Copy link
Collaborator

phadej commented Dec 28, 2015

@hvr, I'm not sure if reinstallation of Cabal is a good idea, i.e. if there is a real problem. I try to downplay this, as I just don't know if this can be solved, except by setting binary <0.8 on ghc <8.0.

Which OTOH might be good enough?

@hvr
Copy link
Member Author

hvr commented Dec 28, 2015

With the new custom-setup feature, reinstallation of Cabal will happen, as this feature was added to allow us to specify which Cabal package version range a Setup.hs file was written against (or is known to be compatible with).

I hope you're right and this problem won't occur so much for users, but if it does, users will just have yet another complaint about cabal, and no amount of downplaying is gonna resolve that... :-(

At the very least, we need to find a way to make this work properly with cabal 1.24+ if we can't fix this for pre-1.24 cabal

@phadej
Copy link
Collaborator

phadej commented Dec 28, 2015

I'm ok, with

  • changing binary <0.8 for just release Cabal
  • making
if impl (ghc >= 8.0)
  binary <0.9
else
  binary <0.8

for yet another release, if we really need binary-0.8 support

@hvr
Copy link
Member Author

hvr commented Dec 28, 2015

@phadej are you sure that works?

I.e. if I do something like

cabal-1.22 install Cabal --constraint 'binary < 0.8'

I run into the very same build-failure, because version constraints don't apply to Setup.hs whatsoever.

@phadej
Copy link
Collaborator

phadej commented Dec 28, 2015

@hvr, what if the if Impl ghc is added to custom-setup ? So it could work with custom-setup aware cabal and recent enough ghc? Or is there still so corner cases?

@hvr
Copy link
Member Author

hvr commented Dec 28, 2015

Once you can use custom-setup (which requires Cabal>=1.24, but this is broken for self-bootstrapping Cabal anyway, currently - but I have an idea for that one...), you also get properly working MIN_VERSION_... macros anyway. So there's no need to reject binary-0.8 in that case :-/

We would only need to reject binary-0.8 for the pre-1.24 situation where have no way yet to do so :-/

@phadej
Copy link
Collaborator

phadej commented Dec 28, 2015

Extreme solution, would to have Cabal's master(and 1.24) be binary >= 0.8; and 1.22 series with <0.8?

@phadej
Copy link
Collaborator

phadej commented Dec 28, 2015

Yet, the problem will probably still exist if user has both binary-0.7 and binary-0.8 in package database, GHC will probably pick 0.8 for Setup compilation?

@ezyang
Copy link
Contributor

ezyang commented Dec 28, 2015

The analysis in this bug is incomplete.

The problem is that 1.22 and prior release of Cabal had "build-type: Custom", which causes GHC to first try to build the Setup executable with "ghc --make" (using the default database) before actually building it proper. Prior to setup dependencies, no dependencies were specified for this call, which is why binary-0.8 is automatically being picked when this occurs.

First, this should NOT be a problem with Cabal HEAD, because we have removed the self-bootstrapping behavior (build-type: Simple), so we should never attempt to build Cabal with the default package database. The normal build of Cabal will properly have version macros specified, so no problems there.

Second, hvr's setup-depends solution WILL work (if we want bootstrapping), we just need to fix the Distribution.Simple.Utils to test for the availability of Paths_Cabal differently. Right now, it assumes that the presence of a version macro means that Paths_Cabal is available; in fact, with GHC 8.0, this will no longer be the case. This should be fixed.

Third, constraints will do no good without setup-depends, so @phadej's proposed solution will not work.

If our problem is that the cabal-install dependency solver will accidentally try to build old versions of Cabal when the new DB doesn't work, then there is a simple solution: we should make emergency new releases of the old release series of Cabal with "build-type: Custom" turned off, as far as possible. Assuming cabal-install prefers the latest version and no one has set patch level constraints, it will pick a version that will compile.

One possible problem is that the self-bootstrapping behavior was necessary to build Cabal. In that case, if your cabal-install is too old and you try to install a new Cabal, it won't work. But we know this is NOT the case for the versions of GHC we support (back to 7.4), because our Travis instance is successfully building Cabal using the default shipped Cabal (I think). Travis builds a bootstrap setup first, so this is not true.

@23Skidoo 23Skidoo added this to the Cabal-1.24 milestone Dec 28, 2015
@23Skidoo
Copy link
Member

To summarise, to solve this we can either:

  • Add a setup-depends section to Cabal.cabal (which will require us to bump cabal-version to >= 1.23, which will break the upgrade path for users of old cabal-install versions).
  • Or disable bootstrapping by changing the build-type in Cabal.cabal to Simple (already done in master).

@ezyang says the following about the latter solution:

One possible problem is that the self-bootstrapping behavior was necessary to build Cabal.

I think that this can only be the case when cabal-version in Cabal.cabal is higher than the installed Cabal version. Currently it's >= 1.10, which was the version released with GHC 7.0. GHC 7.4 used Cabal 1.14. As can be seen here, master builds fine with bootstrapping disabled on all supported GHC versions (though the test suite doesn't work).

So unless someone objects, I think I'll just disable bootstrapping in the 1.22 branch (and maybe also 1.20 and 1.18).

/cc @dcoutts

@23Skidoo 23Skidoo assigned 23Skidoo and unassigned rthomas Dec 30, 2015
@hvr
Copy link
Member Author

hvr commented Jan 4, 2016

Add a setup-depends section to Cabal.cabal (which will require us to bump cabal-version to >= 1.23, which will break the upgrade path for users of old cabal-install versions).

Fwiw, we don't need to bump cabal-version for that. Older cabal version will just nag about the unrecognised stanza :-)

However, setting build-type: Simple sounds like the better alternative to me. We should rather optimise for Hackage than for bootstrapping IMO. And It's not like bootstrapping becomes impossible by this...

@phadej
Copy link
Collaborator

phadej commented Jan 4, 2016

@hvr, your message seems to be clipped

@hvr
Copy link
Member Author

hvr commented Jan 4, 2016

fixed =)

@23Skidoo
Copy link
Member

23Skidoo commented Jan 4, 2016

Older cabal version will just nag about the unrecognised stanza :-)

I think Hackage itself does a correctness check on upload, though that can probably also be circumvented.

23Skidoo added a commit that referenced this issue Jan 15, 2016
Fixes #3003.

(cherry picked from commit bd52675)
23Skidoo added a commit that referenced this issue Jan 15, 2016
23Skidoo added a commit that referenced this issue Jan 15, 2016
Fixes #3003.

(cherry picked from commit bd52675)
@23Skidoo
Copy link
Member

Switched the 1.22, 1.20 and 1.18 branches to build-type: Simple.

@23Skidoo 23Skidoo assigned rthomas and unassigned 23Skidoo Jan 15, 2016
@23Skidoo
Copy link
Member

The problem is special casing in cabal-install: when compiling the setup script for the Cabal library, it assumes that the setup script will be bootstrapped and sets cabalLibVersionToUse to the package version of the Cabal library being installed.

@hvr
Copy link
Member Author

hvr commented Jan 26, 2016

ok, but this won't be an issue for future cabal-install versions (if I understood your earlier comments right this time...)?

@23Skidoo
Copy link
Member

Yes, I removed that special casing in master and in 1.18, 1.20 and 1.22.

@ezyang
Copy link
Contributor

ezyang commented Jan 28, 2016

NB: the move of the source directories breaks GHC's build system. I think you need this patch:

diff --git a/utils/ghc-cabal/ghc.mk b/utils/ghc-cabal/ghc.mk
index 49a2ba3..93cdb77 100644
--- a/utils/ghc-cabal/ghc.mk
+++ b/utils/ghc-cabal/ghc.mk
@@ -43,7 +43,7 @@ $(ghc-cabal_DIST_BINARY): utils/ghc-cabal/Main.hs $(TOUCH_DEP) | $$(dir $$@)/. b
               -optP-include -optPutils/ghc-cabal/cabal_macros_boot.h \
               -odir  bootstrapping \
               -hidir bootstrapping \
-              -ilibraries/Cabal/Cabal \
+              -ilibraries/Cabal/Cabal/src \
               -ilibraries/binary/src -DGENERICS \
               -ilibraries/filepath \
               -ilibraries/hpc \

There is still something deeply dodgy going on here but I'm not sure what it is.

@ezyang
Copy link
Contributor

ezyang commented Jan 28, 2016

@23Skidoo Re #3003 (comment), let me clarify this some more, because the rabbit hole goes deeper.

Prior to 03b02fb, externalSetupMethod was used when forceExternalSetupMethod was True, which occurred with -j was set. But if we look a bit further in the logic for externalSetupMethod, we notice that there is a special case for useCachedSetupExecutable (triggered when build-type is Simple) to attempt to use a "cached setup executable". So, in fact, you would THINK that this logic would simply involve building some pre-canned Setup.hs against a Cabal from the package database. Which is indeed, what ALMOST happens.

The PROBLEM is that every version of Cabal (including HEAD, but we'll get to that in a moment) has a bug, which is that it does NOT correctly set the -i for the Setup.hs build. Here's the invocation that I get from cabal-install-1.16.0.2 /usr/bin/ghc --make ./dist/setup/setup.hs -o ./dist/setup/setup -odir ./dist/setup -hidir ./dist/setup -i -i. -package Cabal-1.23.2.0. If you look closely, ./dist/setup/setup.hs is correct (Cabal did NOT blindly copy the Setup.hs from the directory), but -i. is set, meaning that if you have modules in src which shadowed Cabal (which was the case for the Cabal library prior to moving the source files to src) bad things happen.

To add INSULT to injury, the resulting Setup script is cached, which means that (1) if there was a GOOD cached setup script, then you would not see this behavior, and (2) if you poison your cache with a bad setup script, then you will perpetually see bad behavior until you blow away your setup cache (in practice, your ~/.cabal directory).

OK, so what's going on with HEAD? Johann ended the nonsense in the case of -j by adding a way for cabal-install to invoke itself, which means that the by-in-large biggest way that external setup with build-type: Simple got trigger is eliminated. However, this codepath STILL persists in the case that a cabal-install with a too old version of Cabal wants to build something that requires something newer, and it is STILL WRONG. But it is hard to tickle, because HEAD cabal-install is generally built against the latest Cabal, and so no one in their right mind would ask for a newer Cabal, that's unsatisfiable!

What's the moral of the story. It seems to me that making it easy for users to bootstrap to a new version of Cabal/cabal-install that is not buggy is more important than ensuring old versions of Cabal build in the presence of a exposed binary-0.8. Un-bootstrapping Cabal makes it easier to build when binary-0.8 is installed but harder to build in some configurations of the pre-existing cabal-install. So I think we have two options:

  1. Undo all of these fixes, blacklist the releases we just made, and just tell people that they should upgrade to the latest Cabal/cabal-install. If they try to bootstrap and run into a binary-0.8 error, they can work around it by saying ghc-pkg hide binary-0.8 and trying again. If they blow away their .cabal and .ghc directories, and start by IMMEDIATELY upgrading Cabal/cabal-install, that will also fix the problem.
  2. Continue with these fixes, unbootstrapping Cabal and moving the source files away so they are not picked up when building setups. If they try to bootstrap and run into an unrecognized options error, they work around it by disabling parallel build (UN-INTUITIVE!) Also, now we will never be able to bootstrap Cabal because of custom-setup should respect hs-source-dirs #3085 which means that we will have to make our test suites stop using the LBI trick to figure out where the inplace copy of Cabal is installed.

Honestly, I kind of like option (1) now.

@ezyang
Copy link
Contributor

ezyang commented Jan 28, 2016

Continuation based on discussion with @hvr on IRC: proposal (1) is incomplete, since we still need cabal-install-1.24 to be able to build old versions of the Cabal library. However, there is a fix: we simply need to specify a custom-setup section with an appropriate build-depends.

What should this build-depends be? This will need a little experimenting: it should roughly reflect the build-depends for the Cabal library, but some of the dependencies are controlled by flags which are not supported in custom-setup. Fortunately, a bootstrap Setup.hs doesn't use the entirety of Cabal, so you may indeed be able to get away with a more minimal setup-depends set. If we can't find a set that works cross-platform (e.g. unix really is needed by Setup.hs), however, this plan is in trouble.

@23Skidoo
Copy link
Member

@ezyang

The PROBLEM is that every version of Cabal (including HEAD, but we'll get to that in a moment) has a bug, which is that it does NOT correctly set the -i for the Setup.hs build.

But this only matters in the case of build-type: Custom, correct? All released versions of Cabal with build-type: Custom don't have hs-source-files: src, and custom setup exes are not cached. And for build-type: Simple we don't want bootstrapping anyway (that's the actual reason we moved sources to src - to prevent it happening by accident).

if you poison your cache with a bad setup script, then you will perpetually see bad behavior until you blow away your setup cache (in practice, your ~/.cabal directory)

Correct me if I'm wrong, but with this change the only way cache can be poisoned is when compiling a version of Cabal that has build-type: Simple and hs-source-dirs not set to src. So if we decide to undo the hs-source-dirs change (your option (1)), we should still do something about bootstrapping happening by accident.

@23Skidoo
Copy link
Member

To summarise the current situation. Let's say that the user has Cabal-1.18 installed and runs cabal install Cabal-1.22.$MINORVER. The following will happen depending on the cabal-install/Cabal-1.22.$MINORVER combination:

build-type: Custom build-type: Simple, hs-source-dirs: . build-type: Simple, hs-source-dirs: src
cabal-install without e2bf243 works as expected, incompatible with binary-0.8 accidental bootstrapping, benign cache poisoning (cached setup-1.22 is setup-1.22), incompatible with binary-0.8 without -j1 Unless -j1 is given: cache poisoned (cached setup-1.22 is actually setup-1.18), "unrecognized options" error, incompatible with binary-0.8
cabal-install with e2bf243 works as expected, incompatible with binary-0.8 accidental bootstrapping, cache poisoned (cached setup-1.18 is actually setup-1.22), incompatible with binary-0.8 without -j1 works as expected, compatible with binary-0.8

@ezyang
Copy link
Contributor

ezyang commented Jan 28, 2016

OK, so there is a subtlety here regarding whether or not the cache is poisoned in the second column of your table. In the build-type: Simple, hs-source-dirs: . without e2bf243, it is true that the cached setup will have the same version as its name. However, this setup will be built by compiling the files in the source directory manually, rather than just linking against a Cabal library in the database. I suppose that if you are not doing development on Cabal, then this "poisoning" is harmless. If you are doing development, you might accidentally install a "bad" setup script which has the right version, but some local changes you made which you did not want.

But this only matters in the case of build-type: Custom, correct?

Quite the opposite: cabal-install only attempts to globally cache the Setup script when it's NOT build-type: Custom. (Think about it; it makes no sense to cache a Custom setup script!) Which is why we've never noticed this bug until now.

It is certainly true that build-type: Simple, hs-source-dirs: src and e2bf243 solve the problem. But I have two countervailing concerns: (1) how easy is it to bootstrap and up-to-date Cabal/cabal-install from a default shipped Cabal (I think this solution makes it slightly harder due to the -j1 workaround), and (2) can I bootstrap Cabal (so that the test-suite tricks with reading the LocalBuildInfo work) (can't do it when it's in src #3085 ). Also I don't like having to type src to get at files haha.

@23Skidoo
Copy link
Member

Quite the opposite: cabal-install only attempts to globally cache the Setup script when it's NOT build-type: Custom. (Think about it; it makes no sense to cache a Custom setup script!)

OK, I think we agree that for build-type: Simple we should compile the setup script with -i instead of -i -i.. It will also fully fix the binary-0.8 issue for the users of up-to-date cabal-install versions.

Then if we revert the hs-source-dirs: src change we'll have the following situation:

build-type: Custom build-type: Simple, hs-source-dirs: .
cabal-install without e2bf243 and b453d30 works as expected, incompatible with binary-0.8 accidental bootstrapping, benign cache poisoning (cached setup-1.22 is setup-1.22), incompatible with binary-0.8 (without -j1)
cabal-install with e2bf243 and b453d30 works as expected, incompatible with binary-0.8 works as expected, compatible with binary-0.8

So we won't have to worry about cache poisoning and the "unrecognized options" error. And we can tell people running into the binary-0.8 error when upgrading to either do ghc-pkg hide binary-0.8 or run cabal install -j1 Cabal && cabal install cabal-install.

@ezyang
Copy link
Contributor

ezyang commented Jan 29, 2016

or run cabal install -j1 Cabal && cabal install cabal-install

Just to be totally clear, this workaround only helps for the build-type: Simple, hs-source-dirs: . situation.

Another problem with the binary-0.8 hide workaround, @hvr mentioned that some Travis buildbots get binary-0.8 installed by distro packages, in which case you can't actually hide the package easily, since it is in the global database.

@23Skidoo
Copy link
Member

Just to be totally clear, this workaround only helps for the build-type: Simple, hs-source-dirs: . situation.

Yep, and given that we now have new Cabal releases with build-type: Simple, hs-source-dirs: ., it should work for the majority of users.

23Skidoo added a commit that referenced this issue Jan 30, 2016
23Skidoo added a commit that referenced this issue Jan 30, 2016
23Skidoo added a commit that referenced this issue Jan 30, 2016
@23Skidoo
Copy link
Member

Reverted the hs-source-dirs: src change and changed D.C.SetupWrapper to pass -i -i. to GHC only when compiling Custom setup scripts. Now we only need to cut new cabal-install releases and then this can be marked as fixed.

@rthomas
Copy link

rthomas commented Jan 31, 2016

I've released cabal-install 1.22.8.0, 1.20.2.0 and 1.18.2.0, so I'll close this issue.

@rthomas rthomas closed this as completed Jan 31, 2016
@23Skidoo
Copy link
Member

Thanks to everyone involved!

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