Skip to content

Commit

Permalink
installedCabalVersion: Don't special-case Cabal anymore.
Browse files Browse the repository at this point in the history 
It uses 'build-type: Simple' now.
  • Loading branch information
@23Skidoo
23Skidoo committed Jan 25, 2016
1 parent e0320da commit e2bf243
Showing 1 changed file with 0 additions and 2 deletions.
2 changes: 0 additions & 2 deletions cabal-install/Distribution/Client/SetupWrapper.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,6 @@ externalSetupMethod verbosity options pkg bt mkargs = do
installedCabalVersion :: SetupScriptOptions -> Compiler -> ProgramConfiguration
-> IO (Version, Maybe UnitId
,SetupScriptOptions)
installedCabalVersion options' _ _ | packageName pkg == PackageName "Cabal" =
return (packageVersion pkg, Nothing, options')
installedCabalVersion options' compiler conf = do
index <- maybeGetInstalledPackages options' compiler conf
let cabalDep = Dependency (PackageName "Cabal") (useCabalVersion options')
Expand Down

8 comments on commit e2bf243

@DanielG
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this change broke installing older Cabals which still have build-type:Custom with cabal-install>=1.24 wouldn't it have been better to just check the build-type of the Cabal library being built or even just the version?

I don't understand all the details of #3003 so I'm probably missing something though.

@23Skidoo
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it fail?

@DanielG
Copy link
Collaborator

@DanielG DanielG commented on e2bf243 Sep 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I assumed this limitation of this patch was known.

Basically cabal-install assumes that the version of Cabal it chooses to compile Setup.hs with will be the one its going to talk to. So if cabal-install chooses to compile Cabal-1.22.6's Setup.hs (which still uses build-type:custom) with Cabal-1.24 it will try to use flags such as --dynlibdir which 1.22.6 doesn't know about causing an error.

See at the bottom of https://gitlab.com/dxld/cabal-helper/-/jobs/33264680.

@23Skidoo
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I think we can apply such a patch (see #4787), but bootstrapping-enabled Cabals will still break like in #3003 (e.g. when binary-0.8 is installed).

@DanielG
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine with me as long as an install from a plain GHC bindist works :)

@hvr
Copy link
Member

@hvr hvr commented on e2bf243 Sep 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically cabal-install assumes that the version of Cabal it chooses to compile Setup.hs with will be the one its going to talk to.

is this only for the old-build codepaths or does it apply to new-build as well?

@DanielG
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have only tested with old-build so I don't know.

@23Skidoo
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this only for the old-build codepaths or does it apply to new-build as well?

For both, since new-build also uses setupWrapper.

Please sign in to comment.