facebook / buck2

Build system, successor to Buck
https://buck2.build/
Apache License 2.0
3.33k stars 194 forks source link

Conflicting inputs on erlang build of Opentelemetry #596

Open srstrong opened 2 months ago

srstrong commented 2 months ago

Hi,

We use open-telemetry as a third party dependency (https://github.com/open-telemetry/opentelemetry-erlang), but are hitting a problem with the conflicting inputs check in erlang_application. The issue is caused by the opentelemetry_api_experimental and the opentelemetry_experimental both having include files names "otel_metrics.hrl". I get that might not be best practice, but I think the reasoning is that the otel_metrics.hrl inside opentelemetry_experimental is not part of the public API, so in principal outside code shouldn't care what that file is called.

For my rules, I have specified the hols in opentelemetry_experimental as part of "srcs", so they should be private:

erlang_application(
    name = "opentelemetry_experimental",
    srcs = glob(["opentelemetry/apps/opentelemetry_experimental/src/*.erl", "opentelemetry/apps/opentelemetry_experimental/include/*.hrl"]),
    version = "1.2.1",
   ...

and for opentelemetry_experimental_api the hrls are listed in "includes":

erlang_application(
    name = "opentelemetry_api_experimental",
    srcs = glob(["opentelemetry/apps/opentelemetry_api_experimental/src/*.erl"]), 
    includes = glob(["opentelemetry/apps/opentelemetry_api_experimental/include/*.hrl"]),

Would it be reasonable for the conflict checker to take into account the private vs public distinction?

TheGeorge commented 2 months ago

We haven't gotten a great solution for the division between public and private includes yet. The guidance for now is to ensure that header files are named identifiable by their file name regardless of whether they are residing in the public includes/ directory, or in a private source directory.

The issue is as following:

The last point essentially lifts all private headers to be public as well. Ideally, we would like to enforce a better separation, and say that public headers cannot include private ones. But in practice, this would break alot of code. So we error on the side of caution and enforce uniqueness in the name.

However, I think we could add a strict mode, on application level, that enforces a better separation.