REBAR_SRC_DIRS not available in hook

[MarkoMin]

Based on #2847 , I've found that REBAR_SRC_DIRS is empty. All other variables are actually exposed, but that one is set to "".

To reproduce, simply put this into config:

{pre_hooks,[
    {compile, "echo $REBAR_SRC_DIRS"}
  ]}.

and then run rebar3 compile and see it outputs empty string

Wanted behavior: have a : separated list of source directories (including extra ones).

I'm trying to finish docs about those env variables and this one is problematic.

[ferd]

The gotcha here is that while the erl compiler defaults to src, but this is only for the Erlang compiler (and yrl and xrl as well). Other compilers (eg. plugins or the mib compiler) would specify other directories

The REBAR_SRC_DIRS option comes from the options that can be passed to override default directories more than the basic ones, which are empty lists. The parameter is configurable globally, but specific compilers override it.

I don't think that it would hurt at this point to just pass in the defaults in rebar_dir:all_src_dirs(RebarOpts, ["src"], []) so it defaults to Erlang compilation values, I think that would match most people's expectations for these values.

[MarkoMin]

I don't think that it would hurt at this point to just pass in the defaults in rebar_dir:all_src_dirs(RebarOpts, ["src"], []) so it defaults to Erlang compilation values, I think that would match most people's expectations for these values.

I also think so. I see REBAR_SRC_DIRS as a convenience, not necessity - user can do whatever he wants with REBAR_ROOT_DIR, we're not limiting the user in his actions.

What I'd also like to have is to respect recursive option while fetching src_dirs and extra_src_dirs.

I'll make a PR when I find some time

EDIT: Almost every variable can change after plugin compilation, so values actually depend on when you call it. I'll include a note on that in the docs

[ferd]

The recursive options are likely going to be a bit of a mess, because they change between the src and the extra_src directories.

[MarkoMin]

I wonder if this is the BaseDir which we want in both cases. IMO, both REBAR_APP_DIRS and REBAR_SRC_DIRS should point to either original app/src dirs or to the built ones. REBAR_APP_DIRS points to the built app directories which is totally fine, but if we resolve "src" with "_build/default" as the base dir, we'll get nonexisting path. If we want REBAR_SRC_DIRS to point to the built src directories we need to resolve it with rebar_dir:deps_dir(rebar_dir:deps_dir(State), AppName) as a base dir.

Considering recursive, best we can do without breaking the API in rebar_dir is to append /* at the end of the recursive src_dirs and extra_src_dirs.

[ferd]

For the src directories within the deps, I think it should be filename:join([rebar_dir:deps_dir(State), AppName]) (which is equivalent to what you posted, yes). An option would also be to just return the relative paths so the user can choose where to plug it, but that might break compatibility. I sort of like that one, but it would be a pain if you have multiple entries within the subdirectory.

The whole recursive aspect is an incredible pitfall (see root dir extra source paths for example. I think we are somewhat painted in a corner -- which may not be surprising since the recursion settings were turned on accidentally early on and then when we noticed people already depended on it. The rules are inconsistently applied depending on the context to optimize for breaking the fewest things possible.

I don't know we'll have a great solution out of it either way. If I recall properly, the src_dirs are recursive, but the extra_src_dirs are not by default, but that can be overridden too. So based on how the options are passed and which directory is being analyzed for the hooks, you may actually get funny results there that are not widely consistent.

One thing to note is that we search for files recursively within the src_dirs (by default) but we don't necessarily consider all the subdirectories to be source files, it really depends on what they contain. I'm saying this because it's possible that people have src_dirs like src/ with many languages in them (erl, ex, js) and so marking the recursively should return all directories and subdirectories in there, which may be more than desired for deep structures.

Anyway, as a tl;dr:

• I'm fine with the path being made relative to the app in _build or the src_dirs paths
• I'm fine with the path for src_dirs being purely relative and leave some extra processing to do by the user
• I don't know that we should expand the recursiveness of src directories within that path set but I can be swayed.