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

Adding a haskell-gi-overloading package to hackage #107

Closed
hamishmack opened this issue Jun 27, 2017 · 18 comments
Closed

Adding a haskell-gi-overloading package to hackage #107

hamishmack opened this issue Jun 27, 2017 · 18 comments

Comments

@hamishmack
Copy link
Collaborator

I would like to start uploading Leksah to hackage again, but I really don't like that it will take something like 5x longer to build from hackage than it does from github because of the use of overloading.

The problem is there is no way for leksah to indicate in leksah.cabal that it does not want overloading to be enabled (since there is no cabal.project file). I don't like the idea of splitting the gi-* packages and creating lots of orphans.

So what if we added a haskell-gi-overloading package to hackage. This would have two branches 0.0.0.0 (meaning no overloading support) and 1.x.x.x (yes we want overloading). We don't need to put anything in these packages (it might need one module so it is not completely empty), but perhaps there is some code in haskell-gi-base only needed when using overloading that would make sense to put in the 1.x.x.x branch.

For cabal files that do not use overloading (like leksah.cabal) we would add:

build-depends: haskell-gi-overloading <1.0

One (probably bad) way to make it work would be to change the gi-*.cabal files to include:

if flag(overloaded-methods)
  build-depends: haskell-gi-overloading >=1.0 && <1.1
if flag(overloaded-properties)
  build-depends: haskell-gi-overloading >=1.0 && <1.1
if flag(overloaded-signals)
  build-depends: haskell-gi-overloading >=1.0 && <1.1

The solver should switch off all the flags as they are set to automatic. I suspect it will have little impact on the speed of the cabal solver for packages without the <1.0 constraint, but switching off those three automatic flags on so many gi-* packages is likely to be really bad for the solver.

Instead could the code generation in the custom setup of the gi-* packages ask Cabal for the version of haskell-gi-overloading that is in the build plan (not the one in setup-depends of course) and disable output of the overloading code if the version is <1.0? So the flag settings would just have no impact when the version of haskell-gi-overloading in the build plan was <1.0.

If that is not possible, could we output #if MIN_VERSION_haskell_gi_overloading(1,0,0) around the offending overloading code?

@garetxe
Copy link
Collaborator

garetxe commented Jun 28, 2017

I completely agree with the goal, but I am not so sure about the design.

As a possibility, would it be workable to include the autogenerated bindings without overloading as part of the leksah package? That is, include a bit of custom code that generates the bindings without overloading as part of the leksah source tree. The binding generator is already a library, so this should be possible in principle, I think.

@hamishmack
Copy link
Collaborator Author

As a possibility, would it be workable to include the autogenerated bindings without overloading as part of the leksah package?

I don't think that would work:

  • How would we use libraries that depend on haskell-gi (for instance gi-gtk-hs)?

  • Building leksah would require rebuilding the bindings for each new version of leksah. I guess we could put them in a leksah-gi package, but that would not be shared with anything else (so it would still be more likely to require building). I guess we could have no-overload-gi-* packages, but that seems wrong to me.

Leksah is not the only place this problem comes up by the way. @luigy spent quite some time trying to figure out why his reflex application was taking so long to load in ghci. It turned out he was using jsaddle-webkit2gtk to run it.

Most people using jsaddle-webkit2gtk in their executable will not be using any haskell-gi code directly (so they won't care about overloading). There will be a few who might use haskell-gi and some of those will want overloading. So it would be nice to add something like this to jsaddle-webkit2gtk.cabal to disable overloading by default:

flag gi-overloading
  default: False

...

  if flag(gi-overloading)
      build-depends: haskell-gi-overloading >=1.0
  else
      build-depends: haskell-gi-overloading <1.0

If you have a package that needs overloading it can include build-depends: haskell-gi-overloading >=1.0 and if you use it with jsaddle-webkit2gtk the solver should get the flag setting right.

@luigy
Copy link

luigy commented Jul 1, 2017

I completely agree with the goal, but I am not so sure about the design.

Would be great to find a solution for this that doesn't require manual configuration :)

Leksah is not the only place this problem comes up by the way. @luigy spent quite some time trying to figure out why his reflex application was taking so long to load in ghci.

Indeed, the slow down was obscure to debug and not immediately obvious that overloading in some other package was causing it. In the team I encountered this the speed was disregarded as normal and I would of thought the same if I hadn't seen an app with overloading turned off.

@garetxe
Copy link
Collaborator

garetxe commented Jul 5, 2017

I don't think that would work:

Thanks for the explanation, I agree with what you say.

Instead could the code generation in the custom setup of the gi-* packages ask Cabal for the version of haskell-gi-overloading that is in the build plan (not the one in setup-depends of course) and disable output of the overloading code if the version is <1.0

I think I like this option best, but I am not sure how it would interact with cabal. Does cabal hash the whole build plan, or only the dependencies for a given package? The scenario that worries me is that gi-gtk is installed in the global package store due to some package needing it, but it gets built without overloading. Then another package asks for the same version, but with overloading. Will cabal reuse the already installed version, or install a new one?

If that is not possible, could we output #if MIN_VERSION_haskell_gi_overloading(1,0,0) around the offending overloading code?

Wouldn't this have the same problem as the previous option?

In any case, doing the changes in haskell-gi is probably not too hard, whichever way we decide to go, but I would like to understand a bit better the pros and cons. Maybe we could ask the cabal people for advice?

@garetxe
Copy link
Collaborator

garetxe commented Jul 5, 2017

By the way, there is a related cabal issue: haskell/cabal#2821. We may want to chime in there.

Also, the root problem is the slowness in ghc. I wouldn't be surprised if there is some low-hanging fruit that makes the situation more acceptable. I don't have any time to dig into this right now, but perhaps some profiling leads to some quick results here.

@hamishmack
Copy link
Collaborator Author

I think I like this option best, but I am not sure how it would interact with cabal. Does cabal hash the whole build plan, or only the dependencies for a given package? The scenario that worries me is that gi-gtk is installed in the global package store due to some package needing it, but it gets built without overloading. Then another package asks for the same version, but with overloading. Will cabal reuse the already installed version, or install a new one?

I was hoping that since each gi-* package would have build-depends: haskell-gi-overloading <2.0 a change in the version of haskell-gi-overloading would trigger the code generator in each gi-* to run again.

I think we should consider deprecating the flags and having overloading support on whenever the build plan includes haskell-gi-overloading >=1.0. That way the flags would have no impact at all and could eventually be removed.

This would simplify reasoning about the problem. For instance it would be clear that any package with build-depends: haskell-gi-overloading >=1.0 requires overloading.

By the way, there is a related cabal issue: haskell/cabal#2821. We may want to chime in there.

One problem with that is how do we express the constraint that all gi-* packages should have the same overloading flag settings?

I guess we could probably have a single overloading flag on haskell-gi-base and have that control the generation of the overloading code. Then packages could specify the requirements for that flag.

I think using a haskell-gi-overloading package is the best solution that will work now and we can update to something better when it is available.

@garetxe
Copy link
Collaborator

garetxe commented Jul 6, 2017

I think using a haskell-gi-overloading package is the best solution that will work now and we can update to something better when it is available.

Yes, I agree. I am still not entirely happy about the approach, and would prefer to fix the root cause, but better having something working now.

Would you like sending a pull request? You need to modify https://github.com/haskell-gi/haskell-gi/blob/master/lib/Data/GI/CodeGen/CabalHooks.hs. Otherwise I can try to do it early next week (I am traveling this week).

Just one comment about the default behavior: I think that it is better for newcomers to have access to the overloaded API, so in the case that no explicit dependency is given in the user's cabal file it would be good to default to include overloading. I think that this would be automatic in your design, but just wanted to bring it up.

@hamishmack
Copy link
Collaborator Author

Would you like sending a pull request?

Sorry, I was flat out working on improving support for project files (cabal.project and stack.yaml) in Leksah.

Otherwise I can try to do it early next week (I am traveling this week).

That would be awesome if you have time (I still have a few things I would like to tidy up in Leksah).

Just one comment about the default behavior: I think that it is better for newcomers to have access to the overloaded API, so in the case that no explicit dependency is given in the user's cabal file it would be good to default to include overloading. I think that this would be automatic in your design, but just wanted to bring it up.

Agreed. The default should be to use haskell-gi-overloading-1.0.0.0. Hackage based builds should pick it because it is the highest version number, but also it should be the only version that goes in stackage. If stackage user wants to avoid the overloading they can add haskell-gi-overloading-0.0.0.0 as a manual override in whatever build too they are using (eg. stack.yaml extra-deps).

I would like to change the default in jsaddle-webkit2gtk though. So jsaddle-webkit2gtk would default to build-depends: haskell-gi-overloading <1.0. I will do it with a automatic flag so people can still include overloading if they want it. I expect 90% of users of jsaddle-webkit2gtk will not call any Gtk functions directly from there code (so they will not need overloading), but they are likely to use ghci and as @luigy found it is a bit painful with overloading switched on.

@garetxe
Copy link
Collaborator

garetxe commented Jul 28, 2017

This should now be implemented in ca3d4b2. There are some small differences with respect to the design sketched above. Mainly, instead of generating different code depending on whether the flag was set, the overloading code is now wrapped by a CPP conditional #ifdef ENABLE_OVERLOADING.

The main reason for this is that we can avoid adding magic in the Setup.hs file, and instead can add the following to the gi-*.cabal files:

Flag enable-overloading
      Description: Enable support for overloaded labels.
      Default: True

library:
      if flag(enable-overloading)
         cpp-options: -DENABLE_OVERLOADING
         build-depends: haskell-gi-overloading == 1.0.*

which should achieve the same as the design above, but it should be more transparent.

Could you please check whether the above works for you? I have not yet uploaded new versions of the gi-* packages to hackage, but will do it as soon as you confirm that the above works.

garetxe pushed a commit that referenced this issue Jul 28, 2017
This is useful in particular for wrapping with #ifdef ENABLE_OVERLOADING
blocks of code associated with overloading, as in #107.
@hamishmack
Copy link
Collaborator Author

Could you please check whether the above works for you?

It seems to work very nicely! I generated the haskell-gi bindings packages then added them all to leksah's cabal.project and it built nice and fast and the dist-newstyle/cache/plan.json file has lots of "flags":{"enable-overloading":false}.

We will need two versions of a haskell-gi-overloading package though (I did not see it in haskell-gi repo). Then we can add build-depends: haskell-gi-overloading ==0.0.* to leksah.cabal and make sure it picks the right flags still.

@garetxe
Copy link
Collaborator

garetxe commented Jul 30, 2017

It seems to work very nicely!

Awesome! :)

We will need two versions of a haskell-gi-overloading package though (I did not see it in haskell-gi repo).

I was planning on uploading to hackage basically what's in https://github.com/haskell-gi/haskell-gi/tree/master/overloading as version 1.0 (with a little bit of polishing), and the same thing with version 0.0.

@garetxe
Copy link
Collaborator

garetxe commented Jul 30, 2017

haskell-gi-overloading versions 0.0 and 1.0 are in hackage now.

@garetxe
Copy link
Collaborator

garetxe commented Jul 30, 2017

I have just released the new versions of the gi-* packages to hackage with the support for haskell-gi-overloading, so I am closing this. Please feel free to reopen, or start a new issue, if you find that anything does not work as expected.

@garetxe garetxe closed this as completed Jul 30, 2017
@hamishmack
Copy link
Collaborator Author

@peti would it make sense to add the haskell-gi-overloading 0.0 version of this package to nixpkgs in configuration-hackage2nix.yaml. Something like:

extra-packages:
...
  - haskell-gi-overloading == 0.0       # required when using -f-enable-overloadin with haskell-gi

I presume then we would be able to use something like haskell-gi-overloading = haskell-gi-overloading_0_0 when we want to disable overloading.

@peti
Copy link

peti commented Aug 28, 2017

This is rather perplexing. Why does one need to add a dependency in order to disable a feature?

@hamishmack
Copy link
Collaborator Author

This is rather perplexing. Why does one need to add a dependency in order to disable a feature?

The details are in this issue, but the TLDR is that it allows us to put build-depends: haskell-gi-overloading <1.0 in leksah.cabal to force the feature to be disabled in all the haskell-gi packages used by leksah.

@peti
Copy link

peti commented Aug 29, 2017

Yes, I've seen that. It still makes very little sense to me. Or rather, it feels like an abuse of the package system to do something that's supposed to be done with Cabal flags.

Anyhow, it's probably not my concern. If you want to add haskell-gi-overloading-0.0 to Nixpkgs, then that's fine IMHO.

@hamishmack
Copy link
Collaborator Author

Or rather, it feels like an abuse of the package system to do something that's supposed to be done with Cabal flags.

Agreed, but you can't specify a dependency on a particular cabal flag setting. In this case it is very important that the flag is set the same for all haskell-gi packages (or you get very confusing build errors).

Another option might have been to split the overloaded functions out of the haskell-gi packages. Unfortunately that would create a record number of orphans.

hamishmack added a commit to hamishmack/nixpkgs that referenced this issue Sep 4, 2017
Using this version turns off overloading in all the haskell-gi packages.
See haskell-gi/haskell-gi#107 for details.
markwright added a commit to gentoo-haskell/gentoo-haskell that referenced this issue May 22, 2018
It seems that leksah really does want this empty package:
haskell-gi/haskell-gi#107
Disable overloading with only 0.0 for leksah
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

No branches or pull requests

4 participants