Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Turn private library into common stanza #96

Closed
wants to merge 2 commits into from

Conversation

fendor
Copy link
Contributor

@fendor fendor commented Feb 8, 2020

Private libraries have a bunch of bugs in cabal currently:

and devs of haskell-ide-engine are encountering this bug frequently.
To mitigate this issue, remove usage of private libraries and use
a common stanza to have the same re-use as before.

This change can be undone when the mentioned issues have been resolved.

closes #94
cc @jneira

import: build-deps, extensions
exposed-modules:
other-modules:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise we would expose these modules everywhere, did I get this right?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah that's right.

Copy link
Owner

Choose a reason for hiding this comment

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

The exposed-modules: Symlink below should also be other-modules.

import: build-deps, extensions
exposed-modules:
other-modules:
Copy link
Owner

Choose a reason for hiding this comment

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

Yeah that's right.

Private libraries have a bunch of bugs in cabal currently:
* haskell/cabal#6211
* haskell/cabal#6483

and devs of haskell-ide-engine are encountering this bug frequently.
To mitigate this issue, remove usage of private libraries and use
a common stanza to have the same re-use as before.

This change can be undone when the mentioned issues have been resolved.
@fendor fendor force-pushed the common-stanza-over-private-lib branch from 78ea5b3 to 3f58f52 Compare February 8, 2020 12:27
Copy link
Owner

@DanielG DanielG left a comment

Choose a reason for hiding this comment

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

Great, thanks.

@@ -186,7 +183,6 @@ test-suite ghc-session
build-depends: ghc < 8.9 && >= 8.0.2
, pretty-show < 1.9 && >= 1.8.1
, cabal-helper
Copy link
Collaborator

Choose a reason for hiding this comment

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

this works with cabal build --enable-tests? i was giving a try and it failed for me in ghc-session with:

tests\GhcSession.hs:71:31: error:
    • Couldn't match expected type ‘CabalHelper.Compiletime.Types.Programs’
                  with actual type ‘Programs’
      NB: ‘Programs’
            is defined in ‘CabalHelper.Compiletime.Types’
                in package ‘cabal-helper-1.0.0.0’
          ‘CabalHelper.Compiletime.Types.Programs’
            is defined in ‘CabalHelper.Compiletime.Types’
    • In the first argument of ‘modProgs’, namely ‘defaultPrograms’
      In the expression: modProgs defaultPrograms
      In the expression:
        let
          ?verbose = const False
          ?progs = modProgs defaultPrograms
        in action
   |
71 |             ?progs = modProgs defaultPrograms
   |  

So i was about to remove cabal-helperfrom build-depends and add the public lib modules as src ones

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didnt test, sorry, will do immediately

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems non-trivial as this depends on both the internal common stanza and cabal-helper. they need to depend on the same common-stanza, which cabal doesnt seem to be able to prove (which sounds like the exact reason for private libraries).

Copy link
Collaborator

@jneira jneira Feb 8, 2020

Choose a reason for hiding this comment

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

So i was about to remove cabal-helper from build-depends and add the public lib modules as src ones

Have you tried this?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah looks you'll have to the main cabal-helper library into a stanza too :/

import: build-deps, extensions
exposed-modules:
other-modules:
Copy link
Owner

Choose a reason for hiding this comment

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

The exposed-modules: Symlink below should also be other-modules.

@@ -186,7 +183,6 @@ test-suite ghc-session
build-depends: ghc < 8.9 && >= 8.0.2
, pretty-show < 1.9 && >= 1.8.1
, cabal-helper
Copy link
Owner

Choose a reason for hiding this comment

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

Yeah looks you'll have to the main cabal-helper library into a stanza too :/

@DanielG
Copy link
Owner

DanielG commented Feb 8, 2020

I got it building over in #97 just had to make the main lib a common section too.

I'm pretty all this will blow up the build times by like x5 but let's see what CI says :)

@DanielG DanielG closed this Feb 8, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider replace temporary private lib with a common stanza to avoid cabal bug
3 participants