hpack

hpack copied to clipboard

I tried to apply recipe #409 to executables but it does not seem to work.

This is the respective section of my package.yaml (it does not have a library section):

executables:
  LR:
    main: LR.hs
    # Exclude some files according to recipe https://github.com/sol/hpack/issues/409
    when:
      condition: false
      other-modules:
        BugLRec
        CYK
        LBNF.ErrM
        LBNF.Skel
        LBNF.Test

The generated .cabal file still contains all these modules:

executable LR
  main-is: LR.hs
  other-modules:
      BugLRec
      CFG
      CharacterTokenGrammar
      CYK
      DebugPrint
      LBNF.Abs
      LBNF.ErrM
      LBNF.Lex
      LBNF.Par
      LBNF.Print
      LBNF.Skel
      LBNF.Test
      ParseTable
      ParseTable.Pretty
      Saturation
      SetMaybe
      Util
      Paths_LR_demo

hpack seems to ignore other-modules in the when section altogether.

@andreasabel it took me a while to see what's going on here. The issue is that in your example other-modules is a YAML string, not a list. If you change it to a list it should work:

executables:
  LR:
    main: LR.hs
    # Exclude some files according to recipe https://github.com/sol/hpack/issues/409
    when:
      condition: false
      other-modules:
        - BugLRec
        - CYK
        - LBNF.ErrM
        - LBNF.Skel
        - LBNF.Test

In places where a list of strings is expected, Hpack treats a string literal as a singleton list. It's convenient, but this issue demonstrates how this can be problematic. If would always require lists, your example would result in a parse error, which would be preferable in this case.

I'm not yet sure if, and if yes what we want to do about it. Input certainly welcome!

Ah, I see. I guess I fell into the traps because other-modules: looks exactly like in cabal-files and there, a list does not need dashes.

If parsing does not fail for my mistake, then it should fail in a checking phase.

• I suppose Hpack should check whether modules mentioned in the yaml file actually exist, and if not, at least throw a warning, if not an error.

• It could also check that the modules are valid filenames.

But why isn't omitting the dashes a parse error in the first place? E.g. in this tutorial, https://docs.ansible.com/ansible/latest/reference_appendices/YAMLSyntax.html, I would have to write either

other-modules: |

or

other-modules: >

if I wanted a string-literal spanning several lines.

But why isn't omitting the dashes a parse error in the first place?

$ echo -e "foo:\n  foo\n  bar" | yaml2json -
{"foo":"foo bar"}

Apparently indentation can be used to indicated continuation of a string literal.

check whether modules mentioned in the yaml file actually exist, and if not, at least throw a warning, if not an error.

I'm not sure what I think about doing existence checks in general as I think it collides with one of our design goals: "Give the user 100% control when needed"

Possibly, we could warn if exposed-modules is a string literal (not a list) that consists of multiple words.

Possibly, we could warn if exposed-modules is a string literal (not a list) that consists of multiple words.

Or you words these string literals.

Unfortunately, I couldn't try it myself. I searched the codebase for 30 min to find the parser for fields like other-modules, but I suppose it isn't easy to spot due to some Aeson/Yaml magic (like data LibrarySection ... deriving (..., FromValue), I guess).

Anyway, module lists/strings coming from the parser could be subjected to words avoid blunders like mine.

Which yaml2json is this?

$ echo -e "foo:\n  foo\n  bar" | yaml2json -
{"foo":"foo bar"}

I got one via npm install -g yaml2json (https://www.npmjs.com/package/yaml2json) and this one does not like your example, giving a parse error:

Error: hash not properly dedented, near "bar\n"

(But it neither seems understand the | nor the > syntax, either.)

Which yaml2json is this?

I think it's the one coming with the yaml package (which uses the C implementation, which I think is widely used). The first online tool I tried exposes the same behavior, but this could be due to it using the same underlying C code, not sure: https://onlineyamltools.com/convert-yaml-to-json?input=foo%3A%0A%20%20foo%0A%20%20bar

Or you words these string literals.

I'm not too eager to add more syntax variation for the same thing. I'm puzzled if we could turn it into a parse error? Or possibly warn first and later make it a hard failure.

Unfortunately, I couldn't try it myself. I searched the codebase for 30 min

Don't worry, it will be fast for me to implement, probably faster even than doing a code review.

if we could turn it into a parse error? Or possibly warn first and later make it a hard failure.

Yes, this is probably better.
"Warn" because of users having mistakenly put in modules with spaces in their names? I'd say this is always meaningless, so a hard error makes sense. However, there is a small probability of disrupting existing build processes, so a hard error would require a major version bump.

"Warn" because of users having mistakenly put in modules with spaces in their names?

Yes, I think right now something like exposed-modules: Foo Bar Baz will produce a valid .cabal file that correctly references Foo.hs, Bar.hs and Baz.hs. So rejecting invalid module names is a breaking change.

https://github.com/sol/hpack/pull/426 rejects spaces in module names.

"Warn" because of users having mistakenly put in modules with spaces in their names?

Yes, I think right now something like exposed-modules: Foo Bar Baz will produce a valid .cabal file that correctly references Foo.hs, Bar.hs and Baz.hs.

Ah, I wasn't aware of this. So basically a modules entry with space-separated modules is something that hpack misunderstands (when doing its own logic), but by passing it verbatim to cabal it accidentally works. In that sense, using words would be the more conservative change (restoring hpack's understanding of things).