Difficulty including file from a dependency, for the dependency

[kennedyjosh]

Environment

% ./rebar3 report compile
Rebar3 report
 version 3.16.1
 generated at 2021-08-26T20:51:42+00:00
=================
Task: compile
Entered as:
  compile
-----------------
Operating System: x86_64-apple-darwin20.4.0
ERTS: Erlang/OTP 23 [erts-11.2.2.4] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [hipe] [dtrace]
Root Directory: /usr/local/Cellar/erlang@23/23.3.4.5_1/lib/erlang
Library directory: /usr/local/Cellar/erlang@23/23.3.4.5_1/lib/erlang/lib
-----------------
Loaded Applications:
bbmustache: 1.10.0
certifi: 2.6.1
cf: 0.3.1
common_test: 1.20.2.2
compiler: 7.6.9.1
crypto: 4.9.0.2
cth_readable: 1.5.1
dialyzer: 4.3.1.1
edoc: 0.12
erlware_commons: 1.5.0
eunit: 2.6
eunit_formatters: 0.5.0
getopt: 1.0.1
hipe: 4.0.1
inets: 7.3.2.1
kernel: 7.3.1.2
providers: 1.8.1
public_key: 1.10.0.1
relx: 4.4.0
sasl: 4.0.2
snmp: 5.8.0.1
ssl_verify_fun: 1.1.6
stdlib: 3.14.2.2
syntax_tools: 2.5
tools: 3.4.4

-----------------
Escript path: /Users/josh/Code/project/rebar3
Providers:
  app_discovery as clean clean compile compile compile cover ct cut deps dialyzer do docs edoc escriptize eunit get-deps help install install_deps key list lock new owner path pkgs publish release relup repo report repos retire revert search shell state tar tree unlock update upgrade upgrade upgrade user version xref 

My project, project, has a dependency, dep0. The (simplified) file structure is as follows:

project
  | _build
    | default
      | lib
        | dep0
          | include
            | proto
              | out.hrl
          | proto
            | out.proto
          | src
            | proto
              | out.erl
          | rebar.config
  | include
  | src
  | test
  rebar.config

Current behaviour

I compile project using rebar3 compile. The proto compiler works (successfully compiling out.erl and out.hrl where they belong) but I get an error:

_build/default/lib/dep0/src/proto/out.erl:48: can't find include file "out.hrl"

dep0 compiles successfully on its own (aka, not as a dependency). The config file for dep0 includes the include/proto dir:

{erl_opts, [nowarn_deprecated_function,
            warn_export_all,
            {lager_truncation_size, 15360}, %% 15KB
            {i, "include/proto"},
            {i, "_build/default/plugins/gpb/include"}]}.

In addition, I have tried adding the include/proto dir to project's config:

{erl_opts, [nowarn_deprecated_function,
            {i, "include"},
            {i, "include/proto"},
            %% ...
            {i, "_build/default/lib/dep0/include"},
            {i, "_build/default/lib/dep0/include/proto"},
            %% ...
]}.

Expected behaviour

I expect compilation to succeed as I have told rebar (in the dependency and the main project) where to find the file its complaining about, but compilation fails when it cannot find the file.

[ferd]

to include a given library from gpb, you only need to declare them as -include_lib("gpb/include/name_of_file.hrl"). and by having it in deps, the project will find them. There should be no need to specify the erl_opts for them. The app-local files can be included with -include("myfile.hrl"). (which will search in both src/ and include/ for them) from within the app, and the external ones from another lib with -include_lib("libname/include/file.hrl")..

[kennedyjosh]

to include a given library from gpb, you only need to declare them as -include_lib("gpb/include/name_of_file.hrl"). and by having it in deps, the project will find them. There should be no need to specify the erl_opts for them.

I have the gpb file included in erl_opts because the auto-generated protobuf file includes it like -include("gpb.hrl"). and I don't want to mess with the protobuf compiler to change this. It works, so I see no need to mess with it now.

The app-local files can be included with -include("myfile.hrl"). (which will search in both src/ and include/ for them) from within the app, and the external ones from another lib with -include_lib("libname/include/file.hrl")..

Yes, I have the app-local file included like so: -include("out.hrl"). However, this file is actually in include/proto, not just include, and this seems to be causing the problem. Its not a problem when I run the app on its own, I think because I have the erl_opt to include include/proto, but it is a problem when that library is used as a dependency. Basically, I am not trying to include the out.hrl file from the main project, it is only used in the dependency, and it is the dependency that does not compile (but only when being compiled as a dependency from the main project; it works when I compile the dependency as a top-level app). Does that make more sense?

[ferd]

Yeah I'm not quite sure why it would work when the dependency is at the top level but not as a dependency. We handle absolute paths on behalf of all applications. The one known case of issues there is when the top-level project contains a .hrl file with the same name as one of the subprojects, and that's a limitation of the Erlang compiler itself (which always looks in ./first, regardless of where you call it from).

In general you should be able to use -include("proto/out.hrl"). from within the app and -include_lib("dep0/include/proto/out.hrl"). as far as I can tell, and plenty of applications do this on a routine basis, without needing to specify the paths in erl_opts (we add the standard ones for you in https://github.com/erlang/rebar3/blob/master/src/rebar_compiler_erl.erl#L86-L88), and without having the files coming from gpb.

I unfortunately don't have a project to reproduce that offhand, so debugging is going to be hard outside of what I know about how we've wired up the compiler. Especially if the file exists already and it's not a plugin configuration issue, there might be something else that's odd about the project going on.

[tsloughter]

Yea, can you try to recreate in an example repo? There should be no need for gpb of course, the issue sounds like how to reference a header file under a subdir of src.

[tomas-abrahamsson]

I have the gpb file included in erl_opts because the auto-generated protobuf file includes it like -include("gpb.hrl").

Gpb has the option include_as_lib to generate this as -include_lib("gpb/include/gpb.hrl"). in case it would help.

[kennedyjosh]

I created a reproducible example here: https://github.com/kennedyjosh/proj

[ferd]

In _build/default/lib/dep/src/proto/noise_server.erl, if I change line 48 and apply this diff:

diff --git a/src/proto/noise_server.erl b/src/proto/noise_server.erl
index db4d884..bf8d60d 100644
--- a/src/proto/noise_server.erl
+++ b/src/proto/noise_server.erl
@@ -45,7 +45,7 @@
 -export([get_protos_by_pkg_name_as_fqbin/1]).
 -export([gpb_version_as_string/0, gpb_version_as_list/0]).

--include("noise_server.hrl").
+-include("proto/noise_server.hrl").
 -include("gpb.hrl").

then everything works fine. The search is relative to the top of the include directory, so just specify the path as such. If it's in include/proto/noise_server.hrl, then the include is to proto/noise_server.hrl.

[kennedyjosh]

I was hoping for an alternative method, as that file is auto-generated by gpb. To make that change, I would have to go and edit gpb's behavior...

[ferd]

Ah yeah, I see. The issue then is that the {i, "src/proto"} added to erl_opts is passed as-is to the compiler. So when the dep is used as a top-level application, it works, but when it's moved to a dep, that path no longer exists.

I guess we'd have to look for the shape of the path to make it relative if it's not absolute, but this would in turn break the gpb includes (which point to _build/... as a relative path from the root). So it might end up requiring a check of 'if the directory does not exist, then make it absolute based on the app's relative path, and if that doesn't work again then go back to the original one', but that's very messy. Will have to think on it.

[tsloughter]

It would be a good fix for gpb to have :)

@tomas-abrahamsson what do you think?

@ferd I suppose we should consider adding all subdirs of src/ to the i options?

[ferd]

@tsloughter I think we posted concurrently. See my new comment on the issue there. But yeah an alternative would be for gpb to account for its own o_hrl config when generating its includes, but even that gives no guarantees.

[tomas-abrahamsson]

I was toying with a new gpb option, {include_mod_hrl_prepend, string()}, to insert a string just before the mod.hrl in the generated -include("mod.hrl"). So one could add {include_mod_hrl_prepend, "proto/"} to gpb_opts. Would this be one viable solution to the problem?

But I'm unsure if it would be better instead to reuse the o_hrl directory. I'm a little worried though that doing that might break builds in existing projects, at least if unconditionally doing so. So I'm leaning towards the option I toyed with.

[ferd]

To no small extent I think the fix might be ours to make. If someone just specifies {i, "myincludes/"} they would expect that to work without having to specify a full path. The only gotcha is being careful about not breaking projects such as yours using _build/.../ in there and accidentally messing with backwards compat.

[kennedyjosh]

Thanks for all the feedback. I was able to solve the issue by adding {i, "_build/default/lib/dep/include/proto"} to the erl_opts in the dependency's rebar.config. This seems like more of a band-aid fix than something permanent, but it works.

[tsloughter]

Yea, that is a band-aid and not a permanent fix.

Curious, why not just generate to src and include instead of the proto subdir?

@ferd not sure what you mean about {i, "myincludes/"}, I just mean we add src/ to the includes, why not also add src/*. I don't think that would break any ones projects.

Hm, probably have to make sure src/subdir/ is only included when compiling a file under src/subdir, otherwise src/mod.erl would potentially have its -include("header.hrl"). resolve to src/subdir/header.hrl instead of src/header.hrl or include/header.hrl.

Not that anyone should be having headers with the same name in a project... but I'm sure it exists.

[ferd]

@tsloughter what I'm saying is that regardless of recursivity, {i, "myincludes"} will work at the project root, but will fail when the dep is in _build because we pass the path as-is to the compiler, and the compiler is invoked from the project root and it's directory-sensitive. When we auto-inject src/ and include/, we do it by prepending the path to the app, but that doesn't get done for custom includes in erl_opt.

This, fundamentally, means that non-default paths risk being broken there when a top-level app is moved to a dep and uses non-standard includes.