Use cabal-install-parsers and Cabal parser #13

maksbotan · 2020-06-05T22:48:08Z

This is a draft PR to provoke discussion.

I came across http://hackage.haskell.org/package/cabal-install-parsers-0.3.0.1/docs/Cabal-Project.html, which was made for haskell-ci, and I thought that it could be a good idea to use this parser here instead of hand-written one.

What do you think?

Avi-D-coder · 2020-06-05T23:11:54Z

Thanks,
I'm very much in favor of this.
It was discussed in #4

I take a look tomorrow.

Avi-D-coder · 2020-06-05T23:13:42Z

@jneira, @fendor, if you could both take a look as well

Avi-D-coder · 2020-06-06T19:15:36Z

So this is the output of this pr on hls repo.

cradle:
  cabal:
    - path: "./src"
      component: "lib:haskell-language-server"

    - path: "./exe/Main.hs"
      component: "haskell-language-server:exe:haskell-language-server"

    - path: "./exe/Wrapper.hs"
      component: "haskell-language-server:exe:haskell-language-server-wrapper"

    - path: "./test/utils"
      component: "haskell-language-server:test:func-test"

    - path: "./test/functional"
      component: "haskell-language-server:test:func-test"

    - path: "ghcide/src"
      component: "lib:ghcide"

    - path: "ghcide/test/preprocessor/Main.hs"
      component: "ghcide:exe:ghcide-test-preprocessor"

    - path: "ghcide/exe/Main.hs"
      component: "ghcide:exe:ghcide"

    - path: "ghcide/test/cabal"
      component: "ghcide:test:ghcide-tests"

    - path: "ghcide/test/exe"
      component: "ghcide:test:ghcide-tests"

    - path: "ghcide/test/src"
      component: "ghcide:test:ghcide-tests"

master outputs this:

cradle:
  cabal:
    - path: "./src"
      component: "lib:haskell-language-server"

    - path: "./exe/Main.hs"
      component: "haskell-language-server:exe:haskell-language-server"

    - path: "./exe/Arguments.hs"
      component: "haskell-language-server:exe:haskell-language-server"

    - path: "./exe/Wrapper.hs"
      component: "haskell-language-server:exe:haskell-language-server-wrapper"

    - path: "./exe/Arguments.hs"
      component: "haskell-language-server:exe:haskell-language-server-wrapper"

    - path: "./test/functional"
      component: "haskell-language-server:test:func-test"

    - path: "ghcide/src"
      component: "lib:ghcide"

    - path: "ghcide/src-ghc86"
      component: "lib:ghcide"

    - path: "ghcide/src-ghc88"
      component: "lib:ghcide"

    - path: "ghcide/src-ghc810"
      component: "lib:ghcide"

    - path: "ghcide/test/preprocessor/Main.hs"
      component: "ghcide:exe:ghcide-test-preprocessor"

    - path: "ghcide/exe/Main.hs"
      component: "ghcide:exe:ghcide"

    - path: "ghcide/exe/Utils.hs"
      component: "ghcide:exe:ghcide"

    - path: "ghcide/exe/Arguments.hs"
      component: "ghcide:exe:ghcide"

    - path: "ghcide/exe/Paths_ghcide.hs"
      component: "ghcide:exe:ghcide"

    - path: "ghcide/test/cabal"
      component: "ghcide:test:ghcide-tests"

    - path: "ghcide/test/exe"
      component: "ghcide:test:ghcide-tests"

    - path: "ghcide/test/src"
      component: "ghcide:test:ghcide-tests"

maksbotan · 2020-06-06T19:19:34Z

Yes, I did not yet implement other-modules for exes, tests and benchmarks, there's TODO item for that in the code.

By the way, what do you do in the case there are multiple hs-source-dirs in the executable? Generate cartesian product of source dirs and modules?

Avi-D-coder · 2020-06-06T20:13:35Z

Maybe I have not experimented with that case.

jneira · 2020-06-07T14:49:51Z

src/Hie/Locate.hs

-          mapM (\p -> if takeExtension p == ".cabal" then pure [p] else cfs p) cd
-      pure $ concat cf
+  case rights [cp, cl] of
+    -- FIXME parse cabal files w/o project file


Maybe we could use here: parseProject "cabal.project" "packages: ./*.cabal ./*/*.cabal"
That is the default value in cabal-install for discovering packages when there is no exist a explicit cabal.project

jneira · 2020-06-07T20:33:30Z

Maybe a case not taken in account:

PS D:\dev\ws\haskell\cabal-test> gen-hie
gen-hie.exe: BadPackageLocation "."
PS D:\dev\ws\haskell\cabal-test> cat .\cabal.project
packages:  .

jneira · 2020-06-07T20:50:41Z

It seems it does not take in account conditionals (imo it is one of the keys reasons to use Cabal):

For a .cabal file with a lib stanza like (a very common pattern):

library
  exposed-modules:     Lib
  build-depends:       base >=4.12 && <4.15
                     , bytestring
  hs-source-dirs:      src
  default-language:    Haskell2010
  if os(windows)
    hs-source-dirs:    win-src
  else
    hs-source-dirs:    nix-src

the hie.yaml generated is):

cradle:
  cabal:
    - path: "./src"
      component: "lib:cabal-test"

    - path: "./app/Main.hs"
      component: "cabal-test:exe:cabal-test"

    - path: "./test"
      component: "cabal-test:test:cabal-test-test"

Not sure how could we handle conditionals:

include all paths and let the user remove the ones that breaks the ide
- with a comment warning them
include conditional paths but commented, to make easier select the good ones
???

maksbotan · 2020-06-07T21:33:08Z

Thanks for the comments, @jneira!
Unfortunately, I did not have the time to look into it more on the weekend.

Re conditionals. As far as I understand, for every open file hie/hls would try to find matching prefix. Than I suppose that it's the right idea to generate all possible prefixes. In the normal scenario, user won't open files that belong to other conditionals, and therefore those prefixes will never be matched.

I believe that resolving of the conditionals does not belong to this project.

But of course, I may be wrong, I'm just a bystander :)

jneira

Re conditionals. As far as I understand, for every open file hie/hls would try to find matching prefix. Than I suppose that it's the right idea to generate all possible prefixes. In the normal scenario, user won't open files that belong to other conditionals, and therefore those prefixes will never be matched.

Yeah, that would be my preferred option but i am not sure if include all of them, always, could interfere in some cases with the ide.

I believe that resolving of the conditionals does not belong to this project.

Well, as i wrote in my comment i think that is one of the main points of include Cabal and cabal-install-parsers. This project dont have to fully resolve conditionals but imo it should resolve or analyze them up to being able to generate a reliable hie.yaml.

maksbotan · 2020-06-07T21:56:38Z

How about we come up with a definition of reliable first, then? :)
I don't think I'm familiar with hie internals enough at the moment to know that...

jneira · 2020-06-08T06:01:09Z

Well, reliable can be pretty ambiguous term, fortunately we already have two alternatives that can be used a bounds of what can be a reliable way to get a hie-bios configuration:

the actual custom parser as lower bound
using cabal-helper a higher bound, see WIP: Cabal helper loader #11

We have take in account that one of the goals of the library is to being used as the implicit configuration with no hie.yaml (see haskell/haskell-language-server#138). There would be no hie.yaml file to correct so imo it would need a configuration that takes in account common stanzas and conditionals.

More context (apart of the pr linked by @Avi-D-coder above):

Use implicit-hie haskell/hie-bios#178

What would be the improvements in the generated config using this pr instead the actual custom parser? does it handle common stanzas?

jneira · 2020-06-08T06:24:44Z

does it handle common stanzas?

It does, although using them within executable components generates a little bit strange file.

For this cabal file

common mycommon
  hs-source-dirs:      common-src

library
  import:               mycommon
  exposed-modules:     Lib
  build-depends:       base >=4.12 && <4.15
                     , bytestring
  hs-source-dirs:      src
  default-language:    Haskell2010
  if os(windows)
    hs-source-dirs:    win-src
  else
    hs-source-dirs:    nix-src

executable cabal-test
  import:              mycommon
  main-is:             Main.hs
  build-depends:       base >=4.12 && <4.15
                     , cabal-test
                     , bytestring
  hs-source-dirs:      app
  default-language:    Haskell2010

that is the output of this version:

cradle:
  cabal:
    - path: "./common-src"
      component: "lib:cabal-test"

    - path: "./src"
      component: "lib:cabal-test"

    - path: "./common-src/Main.hs"
      component: "cabal-test:exe:cabal-test"

    - path: "./app/Main.hs"
      component: "cabal-test:exe:cabal-test"

    - path: "./test"
      component: "cabal-test:test:cabal-test-test"

Observe the .common-src/Main.hs, that likely will not exist

Otoh, the actual lib version includes some condition handling!

cradle:
  cabal:
    - path: "./src"
      component: "lib:cabal-test"

    - path: "./win-src"
      component: "lib:cabal-test"

    - path: "./nix-src"
      component: "lib:cabal-test"

    - path: "./app/Main.hs"
      component: "cabal-test:exe:cabal-test"

    - path: "./test"
      component: "cabal-test:test:cabal-test-test"

So not handling them would be even a regression.

jneira · 2020-06-08T06:36:58Z

For completeness, this is the hie-yaml using cabal-helper (with #4):

cradle:
  cabal:
    - path: "common-src\"
      component: "lib:cabal-test"

    - path: "src\"
      component: "lib:cabal-test"

    - path: "win-src\"
      component: "lib:cabal-test"

    - path: "common-src\"
      component: "cabal-test:exe:cabal-test"

    - path: "app\"
      component: "cabal-test:exe:cabal-test"

    - path: "test\"
      component: "cabal-test:test:cabal-test-test"

As expected, it resolves the conditions with the actual flags values (i am on windows) and handles common stanzas the right way.
But i think we could get a similar result using cabal-install-parsers and this pr as base.

maksbotan · 2020-06-08T08:00:57Z

What would be the improvements in the generated config using this pr instead the actual custom parser? does it handle common stanzas?

My primary motivation was to make implicit-hie "automatically" support all of cabal's syntax. I thought of this after making PR #12.

Otoh, the actual lib version includes some condition handling!

Right, I haven't yet done traversing of conditional tree. I probably should just use flattenPackageDescription as you (?) proposed in #4.

jneira · 2020-06-08T08:15:17Z

My primary motivation was to make implicit-hie "automatically" support all of cabal's syntax. I thought of this after making PR #12.

That is a really good point.

Right, I haven't yet done traversing of conditional tree. I probably should just use flattenPackageDescription as you (?) proposed in #4.

Well, it is a possible path; it resolves conditions without having to set flags values (you would need a full config execution to get them). Afaik it merges all branches so maybe it gives us what we need.
Another approximation could be copy part of its logic, focusing in the elements of interest to compute the paths/component pair (hs-source-dir, *-modules, etc) and adapting to our needs.

Avi-D-coder · 2022-10-19T05:18:23Z

Superseded by #48

WIP use cabal-install-parsers

7f8f1f0

Avi-D-coder requested review from fendor and Avi-D-coder June 5, 2020 23:11

jneira reviewed Jun 7, 2020

View reviewed changes

jneira mentioned this pull request Jun 15, 2020

Converge on implicit cradle discovery mechanism haskell/haskell-language-server#38

Closed

Avi-D-coder force-pushed the master branch from 44024eb to ff475f6 Compare November 12, 2020 03:52

jneira mentioned this pull request Nov 25, 2020

Empty other-modules leads to wrong output #30

Closed

Avi-D-coder closed this Oct 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use cabal-install-parsers and Cabal parser #13

Use cabal-install-parsers and Cabal parser #13

maksbotan commented Jun 5, 2020

Avi-D-coder commented Jun 5, 2020

Avi-D-coder commented Jun 5, 2020

Avi-D-coder commented Jun 6, 2020

maksbotan commented Jun 6, 2020

Avi-D-coder commented Jun 6, 2020

jneira Jun 7, 2020

jneira commented Jun 7, 2020 •

edited

Loading

jneira commented Jun 7, 2020 •

edited

Loading

maksbotan commented Jun 7, 2020

jneira left a comment •

edited

Loading

maksbotan commented Jun 7, 2020

jneira commented Jun 8, 2020 •

edited

Loading

jneira commented Jun 8, 2020 •

edited

Loading

jneira commented Jun 8, 2020

maksbotan commented Jun 8, 2020

jneira commented Jun 8, 2020

Avi-D-coder commented Oct 19, 2022

Use cabal-install-parsers and Cabal parser #13

Use cabal-install-parsers and Cabal parser #13

Conversation

maksbotan commented Jun 5, 2020

Avi-D-coder commented Jun 5, 2020

Avi-D-coder commented Jun 5, 2020

Avi-D-coder commented Jun 6, 2020

maksbotan commented Jun 6, 2020

Avi-D-coder commented Jun 6, 2020

jneira Jun 7, 2020

Choose a reason for hiding this comment

jneira commented Jun 7, 2020 • edited Loading

jneira commented Jun 7, 2020 • edited Loading

maksbotan commented Jun 7, 2020

jneira left a comment • edited Loading

Choose a reason for hiding this comment

maksbotan commented Jun 7, 2020

jneira commented Jun 8, 2020 • edited Loading

jneira commented Jun 8, 2020 • edited Loading

jneira commented Jun 8, 2020

maksbotan commented Jun 8, 2020

jneira commented Jun 8, 2020

Avi-D-coder commented Oct 19, 2022

jneira commented Jun 7, 2020 •

edited

Loading

jneira commented Jun 7, 2020 •

edited

Loading

jneira left a comment •

edited

Loading

jneira commented Jun 8, 2020 •

edited

Loading

jneira commented Jun 8, 2020 •

edited

Loading