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

Build failure with HLS-hlint-plugin #3839

Closed
Vekhir opened this issue Oct 11, 2023 · 11 comments
Closed

Build failure with HLS-hlint-plugin #3839

Vekhir opened this issue Oct 11, 2023 · 11 comments
Labels
component: hls-hlint-plugin status: needs triage type: support User support tickets, questions, help with setup etc.

Comments

@Vekhir
Copy link
Contributor

Vekhir commented Oct 11, 2023

Your environment

OS: Arch Linux
Kernel: Linux 6.5.6-arch2-1
Built from source (u = updated version):
GHC: 9.2.8 (u)
haskell-apply-refact: 0.11.0.0 (u)
haskell-hls-hlint-plugin: 1.1.1.0
haskell-ghc-exactprint: 1.5.0 (u)
haskell-hls-plugin-api: 1.6.0.0
haskell-ghc-lib-parser: 9.2.8.20230729 (u)
haskell-ghc-lib-parser-ex: 9.2.1.1 (u)

For brevity, all other packages dependent on GHC have been rebuilt aswell. I can provide a complete list of changes on request.

What's wrong?

HLS-Hlint-Plugin in version 1.1.1.0 should work with haskell-apply-refact 0.11.0.0. Instead, however, there are build errors (see below), so the build fails. Due to the various version constraints, I can't cleanly separate the effects of updating GHC, apply-refact, and the other components. Updating apply-refact to 0.13.0.0 has no effect, and updating hls-hlint-plugin to 1.1.2.0 also has no effect.

I don't have this issue with any other plugin.

Do you have an idea where to look for potential issues or solutions? Has this been fixed in later versions? I'm mainly looking for a patch to backport rather than updating HLS, as I'm for example constrained by fourmolu to stay below 2.3.0.0.

Debug information

During building of HLS-Hlint-Plugin, the following series of errors occurs:

Configuring hls-hlint-plugin-1.1.1.0...
Preprocessing library for hls-hlint-plugin-1.1.1.0..
Building library for hls-hlint-plugin-1.1.1.0..
[1 of 1] Compiling Ide.Plugin.Hlint ( src/Ide/Plugin/Hlint.hs, dist/build/Ide/Plugin/Hlint.dyn_o )

src/Ide/Plugin/Hlint.hs:300:38: error:
    • Couldn't match expected type ‘ParsedSource -> b’
                  with actual type ‘ModuleEx’
    • The function ‘createModuleEx’ is applied to two value arguments,
        but its type ‘Located HsModule -> ModuleEx’ has only one
      In the first argument of ‘Right’, namely
        ‘(createModuleEx anns (applyParseFlagsFixities modu))’
      In the expression:
        Right (createModuleEx anns (applyParseFlagsFixities modu))
    • Relevant bindings include
        createModule :: ParsedModule -> Either a b
          (bound at src/Ide/Plugin/Hlint.hs:300:13)
    |
300 |             createModule pm = Right (createModuleEx anns (applyParseFlagsFixities modu))
    |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

src/Ide/Plugin/Hlint.hs:300:53: error:
    • Couldn't match type ‘()’ with ‘GenLocated SrcSpan HsModule’
      Expected: Located HsModule
        Actual: ApiAnns
    • In the first argument of ‘createModuleEx’, namely ‘anns’
      In the first argument of ‘Right’, namely
        ‘(createModuleEx anns (applyParseFlagsFixities modu))’
      In the expression:
        Right (createModuleEx anns (applyParseFlagsFixities modu))
    |
300 |             createModule pm = Right (createModuleEx anns (applyParseFlagsFixities modu))
    |                                                     ^^^^

src/Ide/Plugin/Hlint.hs:313:42: error:
    • Data constructor not in scope: InfixL
    • Perhaps you meant ‘Infix’ (imported from Development.IDE.GHC.Compat)
    |
313 |                     f LeftAssociative  = InfixL
    |                                          ^^^^^^

src/Ide/Plugin/Hlint.hs:314:42: error:
    • Data constructor not in scope: InfixR
    • Perhaps you meant ‘Infix’ (imported from Development.IDE.GHC.Compat)
    |
314 |                     f RightAssociative = InfixR
    |                                          ^^^^^^

src/Ide/Plugin/Hlint.hs:315:42: error:
    • Data constructor not in scope: InfixN
    • Perhaps you meant ‘Infix’ (imported from Development.IDE.GHC.Compat)
    |
315 |                     f NotAssociative   = InfixN
    |                                          ^^^^^^

src/Ide/Plugin/Hlint.hs:608:35: error:
    • Couldn't match expected type: Rigidity -> t
                  with actual type: Language.Haskell.GHC.ExactPrint.ExactPrint.EPOptions
                                      Data.Functor.Identity.Identity ()
    • The function ‘deltaOptions’ is applied to one value argument,
        but its type ‘Language.Haskell.GHC.ExactPrint.ExactPrint.EPOptions
                        Data.Functor.Identity.Identity ()’
        has none
      In the expression: deltaOptions RigidLayout
      In an equation for ‘rigidLayout’:
          rigidLayout = deltaOptions RigidLayout
    • Relevant bindings include
        rigidLayout :: t (bound at src/Ide/Plugin/Hlint.hs:608:21)
    |
608 |                 let rigidLayout = deltaOptions RigidLayout
    |                                   ^^^^^^^^^^^^^^^^^^^^^^^^

src/Ide/Plugin/Hlint.hs:610:45: error:
    • Couldn't match type: IO Refact.Compat.Module
                     with: b0 -> IO (Refact.Compat.Module, t1)
      Expected: Refact.Compat.Module
                -> b0 -> IO (Refact.Compat.Module, t1)
        Actual: Refact.Compat.Module -> IO Refact.Compat.Module
    • In the first argument of ‘uncurry’, namely ‘Refact.applyFixities’
      In the first argument of ‘mapM’, namely
        ‘(uncurry Refact.applyFixities)’
      In the first argument of ‘($)’, namely
        ‘mapM (uncurry Refact.applyFixities)’
    |
610 |                     ExceptT $ mapM (uncurry Refact.applyFixities)
    |                                             ^^^^^^^^^^^^^^^^^^^^

src/Ide/Plugin/Hlint.hs:611:31: error:
    • Couldn't match expected type: t0
                                    -> Either String (Refact.Compat.Module, b0)
                  with actual type: Either a0 ParsedSource
    • The function ‘postParseTransform’
      is applied to two value arguments,
        but its type ‘Either
                        a0 ([GHC.Parser.Annotation.LEpaComment], DynFlags, ParsedSource)
                      -> Either a0 ParsedSource’
        has only one
      In the second argument of ‘($)’, namely
        ‘postParseTransform (Right (anns, [], dflags, modu)) rigidLayout’
      In the second argument of ‘($)’, namely
        ‘mapM (uncurry Refact.applyFixities)
           $ postParseTransform (Right (anns, [], dflags, modu)) rigidLayout’
    |
611 |                             $ postParseTransform (Right (anns, [], dflags, modu)) rigidLayout
    |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

src/Ide/Plugin/Hlint.hs:611:57: error:
    • Couldn't match expected type: ([GHC.Parser.Annotation.LEpaComment],
                                     DynFlags, ParsedSource)
                  with actual type: (ApiAnns, [a1], DynFlags, ParsedSource)
    • In the first argument of ‘Right’, namely
        ‘(anns, [], dflags, modu)’
      In the first argument of ‘postParseTransform’, namely
        ‘(Right (anns, [], dflags, modu))’
      In the second argument of ‘($)’, namely
        ‘postParseTransform (Right (anns, [], dflags, modu)) rigidLayout’
    |
611 |                             $ postParseTransform (Right (anns, [], dflags, modu)) rigidLayout
    |                                                         ^^^^^^^^^^^^^^^^^^^^^^^^

src/Ide/Plugin/Hlint.hs:612:56: error:
    • Couldn't match expected type: t1 -> IO String
                  with actual type: IO String
    • The function ‘Refact.applyRefactorings'’
      is applied to four value arguments,
        but its type ‘Maybe (Int, Int)
                      -> [[Refact.Refactoring Refact.SrcSpan]]
                      -> Refact.Compat.Module
                      -> IO String’
        has only three
      In the first argument of ‘withRuntimeLibdir’, namely
        ‘(Refact.applyRefactorings' position commands anns' modu')’
      In the second argument of ‘(<$>)’, namely
        ‘withRuntimeLibdir
           (Refact.applyRefactorings' position commands anns' modu')’
    • Relevant bindings include
        modu' :: t1 (bound at src/Ide/Plugin/Hlint.hs:609:25)
    |
612 |                 liftIO $ (Right <$> withRuntimeLibdir (Refact.applyRefactorings' position commands anns' modu'))
    |                                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@Vekhir Vekhir added status: needs triage type: support User support tickets, questions, help with setup etc. labels Oct 11, 2023
@fendor
Copy link
Collaborator

fendor commented Oct 11, 2023

Hi, thank you for your bug report!

You seem to try to build an almost one year version of that plugin... We don't support such a building from source.

Do you have an idea where to look for potential issues or solutions? Has this been fixed in later versions?

It likely has been fixed, as hls-hlint-plugin that ships with 2.3.0.0 uses apply-refact-0.13.0.0.

You can take a look at the plan.json for your GHC version https://downloads.haskell.org/~hls/haskell-language-server-2.3.0.0/plan_json.tar, it tells you which versions we use.

I'm mainly looking for a patch to backport rather than updating HLS, as I'm for example constrained by fourmolu to stay below 2.3.0.0.

I think you need to be a bit more specific, what are you trying to do? After 2.0.0.0, it gets easier to see which plugin and ghcide versions are compatible, since we changed the version scheme to use lockstep versioning.

@Vekhir
Copy link
Contributor Author

Vekhir commented Oct 11, 2023

Hi, I'll expand on the context of what I want to achieve.

The overall aim

On Arch Linux, the official GHC package is at 9.0.2. While the ultimate goal is to update that to 9.6 (or since recently 9.8), the current stepping stone towards that is an update to 9.2. This means that all packages depending on GHC must be compatible with 9.2 (or more precisely, 9.2.8). This is a hard requirement.
Given the requirement for GHC 9.2, I must find compatible package versions and update or patch them where necessary. For most packages, updating isn't an issue and patching enhances upstream compatibility. But it involves slightly more than 1200 packages as they are all dynamically linked to each other, so I want to keep changes minimal.

The specific issue with haskell-language-server

For one thing, updating haskell-language-server means updating a whole lot of packages (around 25 IIRC), so if I can avoid that, I will. On the other hand, all these packages have themselves requirements which ultimately trace back to GHC and must all be compatible with GHC 9.2.
To give a specific example with hls-hlint-plugin 2.3.0.0:
hls-hlint-plugin 2.3.0.0 requires haskell-language-server 2.3.0.0. This means updating all plugins, including hls-fourmolu-plugin. The latter plugin itself requires fourmolu 0.14 (ignore the bounds shown on Hackage, they are wrong). That requires ghc-lib-parser 9.6 which I have to admit I've forgotten the reason, but I can't update it to 9.6. I was there before.

And I want to keep the changes in terms of packages as minimal as possible. A patch to get this plugin working is much easier than going through dozens of packages again.

What I'd like from you

Now, all of the above is mostly a me-problem. Where I'm slightly confused though, is that the hls-hlint-plugin.cabal on Hackage (source for 1.1.1.0) advertises apply-refact ^>= 0.11.0.0 for GHC >= 9.2, but that configuration fails with above errors.
So maybe you can draw on your experience in this project and with your colleagues, on whether this issue happened before and how it was fixed, or if it hasn't, provide me guidance on how to fix it.
And just to be clear, the affected code still exists. I'm not using ghc-lib-parser, but GHC directly, so I get into the #ifndef HLINT_ON_GHC_LIB codepath. The exact configuration can be found in the official PKGBUILD.

How to proceed

Thinking about it, while I can't update all packages consistently, I might be able to update hls-hlint-plugin and the required packages anyway and live with the inconsistency temporarily. This way I can figure out for you whether the issue still persists. Maybe it's a corner case.
What do you think?

@Vekhir
Copy link
Contributor Author

Vekhir commented Oct 12, 2023

@fendor I have now rebuilt the packages in order to update hls-hlint-plugin to 2.4.0.0. The same error occurs with both apply-refact v0.11.0.0 and v0.13.0.0. ghc-exactprint is still at 1.5.0 as that's the latest version supporting GHC 9.2.

I guess this issue now qualifies as a support request for a supported version.

@fendor
Copy link
Collaborator

fendor commented Oct 12, 2023

You say, our build is broken with GHC 9.2.8? Our CI is succeeding with apply-refact-0.13.0.0.

You can see the constraint any.apply-refact ==0.13.0.0 and the GHC version is 9.2.8. (I included some logs that can no longer be viewed, and my comment doesn't seem relevant any more)

@Vekhir
Copy link
Contributor Author

Vekhir commented Oct 12, 2023

You say, our build is broken with GHC 9.2.8?

No, not at all, as I said, the package I'm building has some modifications, and those are probably not reflected in your CI. The main one being that the cpp-option -DHLINT_ON_GHC_LIB is removed as there are other compatibility issues and errors with that.
Of course, the plugin should still work without it - but it does not. So I came here to request your help with finding the issue and a solution. That's part of the reason I chose the support template instead of the bug template. I'm sorry for my miscommunication.

Just FYI, the link is already expired, so I was never able to access it.

@Vekhir
Copy link
Contributor Author

Vekhir commented Oct 16, 2023

Effectively prior to 2bd9ab4, hls-hlint-plugin had a flag to build with the GHC libraries instead of using ghc-lib, and to that end conditionally unset the cpp-option -DHLINT_ON_GHC_LIB. The change set the flag to use ghc-lib by default, though disabling it had no effect and was removed shortly after in the commit 344323b.
The codepath not using -DHLINT_ON_GHC_LIB (i.e. not using ghc-lib-parser) can't be used without heavy modifications since then and therefore hasn't been tested in CI. There have been some attempts at still maintaining it (notably c14cbdb and 065957e), but they are incomplete and don't alleviate the slow bitrot, so this branch is effectively unmaintained.

AFAICT, the codepath not using -DHLINT_ON_GHC_LIB is unsupported, unmaintained, and broken.
For anyone else hitting the errors in the initial post, I recommend not to insist on using this codepath and instead switch to the known and tested codepath using ghc-lib-parser. This requires a rebuild of hlint to also use ghc-lib-parser, aswell as potentially a rebuild of ghc-lib-parser-ex. Newer versions of both default to that configuration. More infos can be found in hlint#1376.

@fendor Would it be reasonable to remove the guarded codepaths? This makes hacks like overriding the .cabal file insufficient, so users employing them are more likely to be aware of the necessity to switch (if there are any left by now).
Keeping the dead code doesn't provide any benefits in return, especially considering the decision in #2867.

-- Vekhir

@fendor
Copy link
Collaborator

fendor commented Oct 16, 2023

Your analysis seems correct to me. Removal is justified but the PR #3757 wants to add support for the guarded code path again, afaict.

If #3757 is merged, and we test the code path in CI again, would this also work for you?

@Vekhir
Copy link
Contributor Author

Vekhir commented Oct 16, 2023

If the codepath works again with that PR it would be fine with me. For now, I've switched to using ghc-lib-parser as that's the one currently working, and I'll keep an eye out for the PR. Whether and when the Arch package is going to switch back will be decided when the PR is included in a release.
Thanks for your input.

@Vekhir Vekhir closed this as completed Oct 16, 2023
@Vekhir
Copy link
Contributor Author

Vekhir commented Feb 27, 2024

The decision has been made to backport the PR #3757 to the Arch package of hls-hlint-plugin to avoid the switch to ghc-lib-parser and the associated costs.
@fendor The linked PR essentially co-opts the existing codepath since ghc and ghc-lib-parser by design have a similar API. The obsolete codepath is actually removed rather than restored. This makes backporting easy. My next aim is getting the PR merged.

@michaelpj
Copy link
Collaborator

Note that we still won't be testing the non-ghc-lib-parser route. Personally I think it's unwise to go that way, but if you're willing to maintain it we can leave it in.

@Vekhir
Copy link
Contributor Author

Vekhir commented Feb 27, 2024

Since all Arch packages are built against a single GHC version, the benefits of ghc-lib-parser are limited. Due to API similarities, there is no expected increase in maintainance burden.
The previous code tried to be clever and take advantage of direct GHC, leading to brittle code. I see no such drawback with this new approach.

We'll do our own checks and maintainance, so your burden should be additionally minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: hls-hlint-plugin status: needs triage type: support User support tickets, questions, help with setup etc.
Projects
None yet
Development

No branches or pull requests

3 participants