Add -main-is argument to _build_haskell_module.

[pranaysashank]

when we have modules attr in haskell_binary, main_function is not respected which results in linking error when we try to build

[ylecornec]

Thanks for the PR,

One issue (that should not be too hard to fix) is that the ctx.attr.main_file attribute only exists for haskell_binary rules (and not haskell_library) but the build_haskell_modules function is called in both cases (this makes the CI fail).

Other than that, the fix works on this example that uses the main_file,main_function and modules attributes of haskell_test.

However I am not familiar with the module system. @facundominguez, is there a better way to address this maybe ?

[pranaysashank]

Pushed a fix for the failure.

[facundominguez]

Thanks @pranaysashank! Looks like a reasonable way to fix it. There is still some disagreement to fix with the documentation. Also, could we have a test added for this?

[pranaysashank]

I added a test for this custom main_function usage, and I am getting strange errors. This is how I am running the test

 bazel test //tests/haskell_module/binary-custom-main:Test

and I'm getting the error

ERROR: /home/pranaysashank/src/tweag/rules_haskell/tests/haskell_module/binary-custom-main/BUILD.bazel:10:13: in _haskell_test rule //tests/haskell_module/binary-custom-main:Test: 
Traceback (most recent call last):
       File "/home/pranaysashank/src/tweag/rules_haskell/haskell/private/haskell_impl.bzl", line 97, column 39, in haskell_test_impl
               return _haskell_binary_common_impl(ctx, is_test = True)
       File "/home/pranaysashank/src/tweag/rules_haskell/haskell/private/haskell_impl.bzl", line 212, column 40, in _haskell_binary_common_impl
               module_map = determine_module_names(srcs_files, not main_as_haskell_module, ctx.attr.main_function, ctx.file.main_file)
       File "/home/pranaysashank/src/tweag/rules_haskell/haskell/private/path_utils.bzl", line 126, column 21, in determine_module_names
               fail("""\
Error in fail: main_file = "tests/haskell_module/binary-custom-main/TestBin.hs" is not listed in the "srcs" attribute.
ERROR: Analysis of target '//tests/haskell_module/binary-custom-main:Test' failed; build aborted: Analysis of target '//tests/haskell_module/binary-custom-main:Test' failed
INFO: Elapsed time: 0.045s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (1 packages loaded, 2 targets configured)
FAILED: Build did NOT complete successfully (1 packages loaded, 2 targets configured)

Am I missing something or is this another bug?

[facundominguez]

It looks to me like a bug, or a yet unsupported feature. No test in tests/haskell_module uses main_file so far.

[googleson78]

So there are at least two bugs here:

• infer_main_module is either "buggy" (it's getting passed custom_main, which is not a valid module name which gets checked, hence it returns Main) or getting called with the wrong arg
• is_main_as_haskell_module is returning False, even though (assuming from the name) it should return True in this situation

Still investigating

[googleson78]

Upon further inspection, the following assumption is currently made (and not documented as far as I can tell):

• if main_file is present, we're not building with haskell_modules

determine_module_names looks for the main module in srcs only when that happens.

[googleson78]

I think it would be fair to just document this and say "if you're building with modules, you must write the fully qualified name of the main function in main_function".

[googleson78]

If we want to support a specifying main_function being not fully qualified and in a file that's not Main.hs, I guess we could go the route of adding main_module which points to a haskell_module target. But tbh I'm not seeing the benefit of using main_file/main_module when you can already fully qualify main_function, at least in the cases I'm thinking of. Perhaps there are some other considerations I'm missing?

[pranaysashank]

If we change to always require a fully qualified main_function, do we try to get a module name for usages like this https://github.com/tweag/rules_haskell/pull/1740/files#diff-500f449c248c36e74ead8385c8dd2b98bb9244e21fafd0be214b0eb927c99abbR157 or is it okay to just pass -main-is argument to all modules. I thought it's not a good idea to always pass the option as the modules can be from a different package.

[pranaysashank]

if main_file is present, we're not building with haskell_modules

where do you see that assumption?

[pranaysashank]

BTW, in my project where I needed to use this, this fix does work. It's only in the test case above that it doesn't

[googleson78]

where do you see that assumption?

path_utils.bzl, line 122 - https://github.com/pranaysashank/rules_haskell/blob/f0af91817ea66972c05b48b471fd58e5aae195f5/haskell/private/path_utils.bzl#L122

[pranaysashank]

I think it would be fair to just document this and say "if you're building with modules, you must write the fully qualified name of the main function in main_function".

Is this the current behaviour for haskell_binary? Without this PR, fully qualifying main_function still doesn't work because we don't pass -main-is to ghc

Edit:
I misunderstood your comment before and yes main_function should always be qualified when main_file is not Main and that was the bug in the test case.

[pranaysashank]

But tbh I'm not seeing the benefit of using main_file/main_module when you can already fully qualify main_function, at least in the cases I'm thinking of. Perhaps there are some other considerations I'm missing?

haskell_module allows module_name to be empty, so we need main_file in those cases. I pushed a new commit that tries to guess module_name from src attribute of haskell_module, if that looks good enough, we can merge this PR

[googleson78]

The buildifier test has failed (bazel test //:buildifier_test). You should run bazel run //:buildifier-fix and commit the fix it generated.

EDIT: fixed this

[facundominguez]

LGTM!

[facundominguez]

LGTM!

I created #1806 to deal with main_file.