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

Remove duplicate configuration in MODULE.bazel #1976

Open
avdv opened this issue Oct 16, 2023 · 2 comments
Open

Remove duplicate configuration in MODULE.bazel #1976

avdv opened this issue Oct 16, 2023 · 2 comments

Comments

@avdv
Copy link
Member

avdv commented Oct 16, 2023

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 #1914 (comment)

@avdv
Copy link
Member Author

avdv commented Oct 16, 2023

See e.g. bazelbuild/bazel#17880 (comment)

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?

@aherrmann
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants