rules_haskell icon indicating copy to clipboard operation
rules_haskell copied to clipboard

Remove duplicate configuration in MODULE.bazel

Open avdv opened this issue 2 years ago • 2 comments

The configuration variables such as test_repl_ghci_args are duplicated inside the MODULE.bazel file because it is not possible to load them at the moment (see bazelbuild/bazel#17880). For the same reason it is not possible to rely on is_windows to configure the cabalops variable so the windows toolchains are declared separately in this file.

Perhaps another approach could be for the module extension/tags to accept a label to a JSON file holding this configuration? (Either way, out of scope for this PR)

Originally posted by @aherrmann in https://github.com/tweag/rules_haskell/pull/1914#discussion_r1246305014

avdv avatar Oct 16 '23 12:10 avdv

See e.g. https://github.com/bazelbuild/bazel/issues/17880#issuecomment-1483112863

I think we could indeed add an attribute which accepts a label to a json file which would be an object that accepts properties corresponding to the current attributes of the bindist extension, e.g. instead of

haskell_toolchains.bindists(
    cabalopts = test_cabalopts,
    ghcopts = test_ghcopts,
    haddock_flags = test_haddock_flags,
    repl_ghci_args = test_repl_ghci_args,
)

one could write:

haskell_toolchains.bindists(json_config = "//:bindist.json")

and have a bindist.json file in workspace root:

{
    "cabalopts": [...],
    "ghcopts": [...],
    "haddock_flags": [...],
    "repl_ghci_args": [...],

}

Is this what you had in mind, @aherrmann?

Should we try to combine these if both, the attribute and the json file are given or error out?

avdv avatar Oct 16 '23 13:10 avdv

@avdv Thanks for giving this its dedicated ticket!

Yes, that's what I had in mind.

Should we try to combine these if both, the attribute and the json file are given or error out?

That is a good question. In the spirit of small incremental steps I would say start with the simpler solution, which is to only allow one or the other and error out if both are provided. If anyone actually has a use-case to combine both, then we can see how to best do that in that context.

Either way, I would not block the release and first BCR upload on this issue. I think this can be handled later.

aherrmann avatar Oct 16 '23 14:10 aherrmann