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

haskell-cabal-bounds: New expression #2311

Merged
merged 2 commits into from
May 1, 2014
Merged

Conversation

bennofs
Copy link
Contributor

@bennofs bennofs commented Apr 18, 2014

No description provided.

@@ -2870,6 +2877,10 @@ let result = let callPackage = x : y : modifyPrio (newScope result.finalReturn x

cabal2nix = callPackage ../development/tools/haskell/cabal2nix {};

cabalBounds = callPackage ../development/tools/haskell/cabal-bounds {
cabal = self.cabal_1_18;
Copy link
Contributor

Choose a reason for hiding this comment

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

What's going on here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why I added this line anymore, everything seems to work fine without this. Removed it.

@ocharles
Copy link
Contributor

There seems to be quite a bit of hacking going on here - could you explain what this is?

@bennofs
Copy link
Contributor Author

bennofs commented Apr 25, 2014

I need to depend on Cabal_1_18_1_3 because cabal-bounds update reads the setup-config file, whose format is dependent on the version of Cabal. This is also the reason why I patch the setup-config file of the tests in the postPatch hook.

@@ -119,6 +119,13 @@ let result = let callPackage = x : y : modifyPrio (newScope result.finalReturn x
glibcLocales = if pkgs.stdenv.isLinux then pkgs.glibcLocales else null;
};

cabal_1_18 = callPackage ../build-support/cabal {
Copy link
Member

Choose a reason for hiding this comment

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

Why does this expression duplicate most of the arguments to cabal instead of overriding the one that wants to change, i.e. Cabal?

Copy link
Member

Choose a reason for hiding this comment

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

Also, why does cabal_1_18 exist in the first place? No-one seems to use it?

@bennofs
Copy link
Contributor Author

bennofs commented Apr 28, 2014

I just upgraded to version 0.4.1, whose tests require Cabal 1.20 to work, so I removed the hack and disabled testing.

@@ -2870,6 +2870,10 @@ let result = let callPackage = x : y : modifyPrio (newScope result.finalReturn x

cabal2nix = callPackage ../development/tools/haskell/cabal2nix {};

cabalBounds = callPackage ../development/tools/haskell/cabal-bounds {
Cabal = self.Cabal_1_18_1_3;
Copy link
Member

Choose a reason for hiding this comment

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

Is that override still required with a disabled test suite?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, cabal bounds requires Cabal >= 1.18 (compile error on older versions)

Copy link
Member

Choose a reason for hiding this comment

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

So what happens on newer compilers? It would be nice if the old Cabal version wouldn't have to be built unnecessarily when compiling the package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If overriding Cabal is not the right thing to do, what are you suggesting could I do instead?

Copy link
Member

Choose a reason for hiding this comment

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

You could use pkgs.stdenv.lib.versionOlder "x.y" ghc.version to override Cabal only iff GHC has a version that's older than the one we need. On platforms that have Cabal >= 1.18, you can pass null instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, fixed

EDIT: For some reason, it still tries to build Cabal ...
EDIT2: It should work now

@bennofs
Copy link
Contributor Author

bennofs commented Apr 30, 2014

Rebased on master, so it should now merge without conflicts

peti added a commit that referenced this pull request May 1, 2014
haskell-cabal-bounds: New expression
@peti peti merged commit 9795203 into NixOS:master May 1, 2014
@bennofs
Copy link
Contributor Author

bennofs commented May 1, 2014

@peti Can you merge the commit the I just added?

@bennofs
Copy link
Contributor Author

bennofs commented May 1, 2014

Ah, I found out why versionAtLeast didn't work. The argument order for versionAtLeast is opposite ... how confusing.

@peti
Copy link
Member

peti commented May 1, 2014

Pushed 8310c6a.

@bennofs
Copy link
Contributor Author

bennofs commented May 1, 2014

Thanks :)

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

Successfully merging this pull request may close these issues.

3 participants