CC Macro name conflict when two packages have the same name

[guibou]

580ea82 is a unit test reproducing the issue

Problem statment

The haskell package name of an haskell_library is based on the bazel rule name, so //tests:bytestring and //tests/cpp_macro_conflict:bytestring have the same name bytestring.

When enabling haskell extension CPP, ghc is exporting macros VERSION_pkgname (e.g. VERSION_bytestring), this results in macro name conflicts when a haskell_library or haskell_binary have two deps with the same bazel name.

Using code from 580ea82, I got this error when building with bazel build //tests/cpp_macro_conflict/...

/run/user/1000/ghc8953_0/ghc_2.h:13:0: error:
     error: "VERSION_bytestring" redefined [-Werror]
     #define VERSION_bytestring ""
     
   |
13 | #define VERSION_bytestring ""
   | ^

/run/user/1000/ghc8953_0/ghc_2.h:7:0: error:
     note: this is the location of the previous definition
     #define VERSION_bytestring "0.10.8.2"
     
  |
7 | #define VERSION_bytestring "0.10.8.2"
  | ^

/run/user/1000/ghc8953_0/ghc_2.h:14:0: error:
     error: "MIN_VERSION_bytestring" redefined [-Werror]
     #define MIN_VERSION_bytestring(major1,major2,minor) (\
     
   |
14 | #define MIN_VERSION_bytestring(major1,major2,minor) (\
   | ^

/run/user/1000/ghc8953_0/ghc_2.h:8:0: error:
     note: this is the location of the previous definition
     #define MIN_VERSION_bytestring(major1,major2,minor) (\
     
  |
8 | #define MIN_VERSION_bytestring(major1,major2,minor) (\
  | ^

Note: this is an error due to -Warn, but usually that's just annoying warnings.

One simple workaround is to tell user to avoid package name conflict when using CPP.

Discussion

Version name is useful for library from the outside world (i.e. hackage) because they may implement different behaviors depending on some version of their dependencies, which are also coming from the outside world. These external libraries can still be built inside our bazel repository (i.e. using Hazel). These external libraries depends on these macro and on the correct package name. For them we need to export the version macro and keep the official package name. External libraries have a version number (and hazel exposes it using version attribute).

Libraries defined in a bazel repository may not be concerned by version number, that the point of a mega repo, you control all your dependencies and everything must be coherent inside the mega repo. Hence users won't need these macros for local libraries and we can disable them.

The undocumented option -fno-macro-version (https://ghc.haskell.org/trac/ghc/ticket/11763) works for all the macros during one build operation. This does not allow a per-package choice. For example, if a rule depends on both the "hackage" bytestring and a local bytestring, we cannot enable macro generation for the former and not for the later.

Design proposal

Each package controls if its macro version will be exported when the package is used as a dependency. Using for example an export_macros attribute. The exported macro depends on the version attribute.

Users will still have to change their naming scheme but only if they want to export the version macro of two packages with the same name.

For example:

# This is the hackage "bytestring" library
haskell_library(
   name = "bytestring",
   version = "1.2.3",
   export_macros = True, # can be deduced from the presence of `version`
)

# This is a local library
haskell_library(
   name = "foo",
   export_macros = False, # can be deduced from the absence of `version` 
)

haskell_library(
   name = "bar",
   deps = [":bytestring", ":foo"],
)

Here, bar will be built with version macro for bytestring but not for foo.

Implementation details

We'll have to use -fno-macro-version for all build and then manually export the needed macros.

We'll have to manually replicate the macro export mechanisms, AFAIK there is no flag to tell GHC to export the version macro per package.

Alternative design 0

We can tell user that, if they want to use CPP macro, they need to avoid package name conflict. They will need to change their naming scheme, but there is no work from our side (except documentation).

Simple, but forces the user to change its naming scheme.

Design 1: disable all macros on a rule

This is the solution with the simplest implementation, using -fno-macro-version, we can disable all the macro generation during the rule execution. For example:

# This is the hackage "bytestring" library, it needs the version macro to build
haskell_library(
   name = "bytestring",
   deps = [":text", ":lens"], # others dependencies from hackage
   enable_version_macro = True,
)

# This is a local library which does not need versino macro to build
haskell_library(
   name = "foo",
   enable_version_macro = False
)

Here bytestring will be built with the macro version for lens and text. However foo will be built without any macro version.

I don't like this approach, because disabling all the macro for a build changes nothing compared to the current status of rules_haskell, except that it will disable the warnings.

[guibou]

After a discussion with @mboes, we'll go with design 1. That one should be simple to implement and will fix the warning issue.

This solution is based on the invariant that inside a mono repository you don't care about version number of your dependencies for local packages, because the version number is implitly fixed by the repository.

The design will be as such:

• Any rule with a version attribute will exposes version macro during compilation. This way hazel will continue to work. @judah, can you confirm?
• Any rule without a version attribute will not exposes version macro during compilation.

The designed expose_version_macro is for now implicitly defined based on the content of the version attribute. Later it may be exposed to the user .

[judah]

@guibou that approach sounds good and should work fine for Hazel.