Merge pull request #1486 from ferd/src_dir-is-respected

Add respect for src_dirs option in app_discover and app_info

src/rebar_app_discover.erl

@@ -7,8 +7,10 @@
          find_unbuilt_apps/1,
          find_apps/1,
          find_apps/2,
+         find_apps/3,
          find_app/2,
-         find_app/3]).
+         find_app/3,
+         find_app/4]).
 
 -include("rebar.hrl").
 -include_lib("providers/include/providers.hrl").
@@ -20,7 +22,9 @@
 do(State, LibDirs) ->
     BaseDir = rebar_state:dir(State),
     Dirs = [filename:join(BaseDir, LibDir) || LibDir <- LibDirs],
-    Apps = find_apps(Dirs, all),
+    RebarOpts = rebar_state:opts(State),
+    SrcDirs = rebar_dir:src_dirs(RebarOpts, ["src"]),
+    Apps = find_apps(Dirs, SrcDirs, all),
     ProjectDeps = rebar_state:deps_names(State),
     DepsDir = rebar_dir:deps_dir(State),
     CurrentProfiles = rebar_state:current_profiles(State),
@@ -179,32 +183,44 @@ reset_hooks(Opts) ->
                         rebar_opts:set(OptsAcc, Key, [])
                 end, Opts, [post_hooks, pre_hooks, provider_hooks, artifacts]).
 
-%% @doc find the directories for all apps
--spec all_app_dirs([file:name()]) -> [file:name()].
+%% @private find the directories for all apps, while detecting their source dirs
+%% Returns the app dir with the respective src_dirs for them, in that order,
+%% for every app found.
+-spec all_app_dirs([file:name()]) -> [{file:name(), [file:name()]}].
 all_app_dirs(LibDirs) ->
     lists:flatmap(fun(LibDir) ->
-                          app_dirs(LibDir)
+                        SrcDirs = find_config_src(LibDir, ["src"]),
+                        app_dirs(LibDir, SrcDirs)
                   end, LibDirs).
 
-%% @doc find the directories based on the library directories
--spec app_dirs([file:name()]) -> [file:name()].
-app_dirs(LibDir) ->
-    Path1 = filename:join([LibDir,
-                           "src",
-                           "*.app.src"]),
-
-    Path2 = filename:join([LibDir,
-                           "src",
-                           "*.app.src.script"]),
-
-    Path3 = filename:join([LibDir,
-                           "ebin",
-                           "*.app"]),
+%% @private find the directories for all apps based on their source dirs
+%% Returns the app dir with the respective src_dirs for them, in that order,
+%% for every app found.
+-spec all_app_dirs([file:name()], [file:name()]) -> [{file:name(), [file:name()]}].
+all_app_dirs(LibDirs, SrcDirs) ->
+    lists:flatmap(fun(LibDir) -> app_dirs(LibDir, SrcDirs) end, LibDirs).
+
+%% @private find the directories based on the library directories.
+%% Returns the app dir with the respective src_dirs for them, in that order,
+%% for every app found.
+%%
+%% The function returns the src directories since they might have been
+%% detected in a top-level loop and we want to skip further detection
+%% starting now.
+-spec app_dirs([file:name()], [file:name()]) -> [{file:name(), [file:name()]}].
+app_dirs(LibDir, SrcDirs) ->
+    Paths = lists:append([
+        [filename:join([LibDir, SrcDir, "*.app.src"]),
+         filename:join([LibDir, SrcDir, "*.app.src.script"])]
+        || SrcDir <- SrcDirs
+    ]),
+    EbinPath = filename:join([LibDir, "ebin", "*.app"]),
 
     lists:usort(lists:foldl(fun(Path, Acc) ->
-                                    Files = filelib:wildcard(ec_cnv:to_list(Path)),
-                                    [app_dir(File) || File <- Files] ++ Acc
-                            end, [], [Path1, Path2, Path3])).
+                                Files = filelib:wildcard(ec_cnv:to_list(Path)),
+                                [{app_dir(File), SrcDirs}
+                                 || File <- Files] ++ Acc
+                            end, [], [EbinPath | Paths])).
 
 %% @doc find all apps that haven't been built in a list of directories
 -spec find_unbuilt_apps([file:filename_all()]) -> [rebar_app_info:t()].
@@ -222,16 +238,32 @@ find_apps(LibDirs) ->
 %% app info records.
 -spec find_apps([file:filename_all()], valid | invalid | all) -> [rebar_app_info:t()].
 find_apps(LibDirs, Validate) ->
-    rebar_utils:filtermap(fun(AppDir) ->
-                                  find_app(AppDir, Validate)
-                          end, all_app_dirs(LibDirs)).
+    rebar_utils:filtermap(
+      fun({AppDir, AppSrcDirs}) ->
+            find_app(rebar_app_info:new(), AppDir, AppSrcDirs, Validate)
+      end,
+      all_app_dirs(LibDirs)
+    ).
+
+%% @doc for each directory passed, with the configured source directories,
+%% find all apps according to the validity rule passed in.
+%% Returns all the related app info records.
+-spec find_apps([file:filename_all()], [file:filename_all()], valid | invalid | all) -> [rebar_app_info:t()].
+find_apps(LibDirs, SrcDirs, Validate) ->
+    rebar_utils:filtermap(
+      fun({AppDir, AppSrcDirs}) ->
+            find_app(rebar_app_info:new(), AppDir, AppSrcDirs, Validate)
+      end,
+      all_app_dirs(LibDirs, SrcDirs)
+    ).
 
 %% @doc check that a given app in a directory is there, and whether it's
 %% valid or not based on the second argument. Returns the related
 %% app info record.
 -spec find_app(file:filename_all(), valid | invalid | all) -> {true, rebar_app_info:t()} | false.
 find_app(AppDir, Validate) ->
-    find_app(rebar_app_info:new(), AppDir, Validate).
+    SrcDirs = find_config_src(AppDir, ["src"]),
+    find_app(rebar_app_info:new(), AppDir, SrcDirs, Validate).
 
 %% @doc check that a given app in a directory is there, and whether it's
 %% valid or not based on the second argument. Returns the related
@@ -239,9 +271,29 @@ find_app(AppDir, Validate) ->
 -spec find_app(rebar_app_info:t(), file:filename_all(), valid | invalid | all) ->
     {true, rebar_app_info:t()} | false.
 find_app(AppInfo, AppDir, Validate) ->
+    %% if no src dir is passed, figure it out from the app info, with a default
+    %% of src/
+    AppOpts = rebar_app_info:opts(AppInfo),
+    SrcDirs = rebar_dir:src_dirs(AppOpts, ["src"]),
+    find_app(AppInfo, AppDir, SrcDirs, Validate).
+
+%% @doc check that a given app in a directory is there, and whether it's
+%% valid or not based on the second argument. The third argument includes
+%% the directories where source files can be located. Returns the related
+%% app info record.
+-spec find_app(rebar_app_info:t(), file:filename_all(),
+               [file:filename_all()], valid | invalid | all) ->
+    {true, rebar_app_info:t()} | false.
+find_app(AppInfo, AppDir, SrcDirs, Validate) ->
     AppFile = filelib:wildcard(filename:join([AppDir, "ebin", "*.app"])),
-    AppSrcFile = filelib:wildcard(filename:join([AppDir, "src", "*.app.src"])),
-    AppSrcScriptFile = filelib:wildcard(filename:join([AppDir, "src", "*.app.src.script"])),
+    AppSrcFile = lists:append(
+        [filelib:wildcard(filename:join([AppDir, SrcDir, "*.app.src"]))
+         || SrcDir <- SrcDirs]
+    ),
+    AppSrcScriptFile = lists:append(
+        [filelib:wildcard(filename:join([AppDir, SrcDir, "*.app.src.script"]))
+         || SrcDir <- SrcDirs]
+    ),
     try_handle_app_file(AppInfo, AppFile, AppDir, AppSrcFile, AppSrcScriptFile, Validate).
 
 %% @doc find the directory that an appfile has
@@ -358,3 +410,16 @@ enable(State, AppInfo) ->
 -spec to_atom(binary()) -> atom().
 to_atom(Bin) ->
     list_to_atom(binary_to_list(Bin)).
+
+%% @private when looking for unknown apps, it's possible they have a
+%% rebar.config file specifying non-standard src_dirs. Check for a
+%% possible config file and extract src_dirs from it.
+find_config_src(AppDir, Default) ->
+    case rebar_config:consult(AppDir) of
+        [] ->
+            Default;
+        Terms ->
+            %% TODO: handle profiles I guess, but we don't have that info
+            proplists:get_value(src_dirs, Terms, Default)
+    end.
+

src/rebar_app_info.erl

@@ -248,13 +248,12 @@ set(AppInfo=#app_info_t{opts=Opts}, Key, Value) ->
 
 %% @doc finds the .app.src file for an app, if any.
 -spec app_file_src(t()) -> file:filename_all() | undefined.
-app_file_src(#app_info_t{app_file_src=undefined, dir=Dir, name=Name}) ->
-    AppFileSrc = filename:join([ec_cnv:to_list(Dir), "src", ec_cnv:to_list(Name)++".app.src"]),
-    case filelib:is_file(AppFileSrc) of
-        true ->
-            AppFileSrc;
-        false ->
-            undefined
+app_file_src(#app_info_t{app_file_src=undefined, dir=Dir, name=Name, opts=Opts}) ->
+    CandidatePaths = [filename:join([ec_cnv:to_list(Dir), Src, ec_cnv:to_list(Name)++".app.src"])
+                      || Src <- rebar_opts:get(Opts, src_dirs, ["src"])],
+    case lists:dropwhile(fun(Path) -> not filelib:is_file(Path) end, CandidatePaths) of
+        [] -> undefined;
+        [AppFileSrc|_] -> AppFileSrc
     end;
 app_file_src(#app_info_t{app_file_src=AppFileSrc}) ->
     ec_cnv:to_list(AppFileSrc).

src/rebar_prv_plugins.erl

@@ -34,14 +34,17 @@ do(State) ->
     GlobalConfigFile = rebar_dir:global_config(),
     GlobalConfig = rebar_state:new(rebar_config:consult_file(GlobalConfigFile)),
     GlobalPlugins = rebar_state:get(GlobalConfig, plugins, []),
+    GlobalSrcDirs = rebar_state:get(GlobalConfig, src_dirs, ["src"]),
     GlobalPluginsDir = filename:join([rebar_dir:global_cache_dir(rebar_state:opts(State)), "plugins", "*"]),
-    GlobalApps = rebar_app_discover:find_apps([GlobalPluginsDir], all),
+    GlobalApps = rebar_app_discover:find_apps([GlobalPluginsDir], GlobalSrcDirs, all),
     display_plugins("Global plugins", GlobalApps, GlobalPlugins),
 
+    RebarOpts = rebar_state:opts(State),
+    SrcDirs = rebar_dir:src_dirs(RebarOpts, ["src"]),
     Plugins = rebar_state:get(State, plugins, []),
     PluginsDir = filename:join(rebar_dir:plugins_dir(State), "*"),
     CheckoutsDir = filename:join(rebar_dir:checkouts_dir(State), "*"),
-    Apps = rebar_app_discover:find_apps([CheckoutsDir, PluginsDir], all),
+    Apps = rebar_app_discover:find_apps([CheckoutsDir, PluginsDir], SrcDirs, all),
     display_plugins("Local plugins", Apps, Plugins),
     {ok, State}.
 

test/rebar_src_dirs_SUITE.erl

@@ -11,12 +11,16 @@
          src_dirs_in_erl_opts/1,
          extra_src_dirs_in_erl_opts/1,
          src_dirs_at_root_and_in_erl_opts/1,
+         dupe_src_dirs_at_root_and_in_erl_opts/1,
          extra_src_dirs_at_root_and_in_erl_opts/1,
          build_basic_app/1,
          build_multi_apps/1,
-         src_dir_takes_precedence_over_extra/1]).
+         src_dir_takes_precedence_over_extra/1,
+         src_dir_checkout_dep/1,
+         app_src_info/1]).
 
 -include_lib("common_test/include/ct.hrl").
+-include_lib("eunit/include/eunit.hrl").
 
 suite() ->
     [].
@@ -35,8 +39,11 @@ end_per_testcase(_, _Config) -> ok.
 all() ->
     [src_dirs_at_root, extra_src_dirs_at_root,
      src_dirs_in_erl_opts, extra_src_dirs_in_erl_opts,
-     src_dirs_at_root_and_in_erl_opts, extra_src_dirs_at_root_and_in_erl_opts,
-     build_basic_app, build_multi_apps, src_dir_takes_precedence_over_extra].
+     src_dirs_at_root_and_in_erl_opts,
+     dupe_src_dirs_at_root_and_in_erl_opts,
+     extra_src_dirs_at_root_and_in_erl_opts,
+     build_basic_app, build_multi_apps, src_dir_takes_precedence_over_extra,
+     src_dir_checkout_dep, app_src_info].
 
 src_dirs_at_root(Config) ->
     AppDir = ?config(apps, Config),
@@ -93,15 +100,47 @@ extra_src_dirs_in_erl_opts(Config) ->
 src_dirs_at_root_and_in_erl_opts(Config) ->
     AppDir = ?config(apps, Config),
 
-    Name = rebar_test_utils:create_random_name("app1_"),
+    Name = rebar_test_utils:create_random_name("src_dirs_root_erlopts_"),
     Vsn = rebar_test_utils:create_random_vsn(),
     rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]),
 
     RebarConfig = [{erl_opts, [{src_dirs, ["foo", "bar"]}]}, {src_dirs, ["baz", "qux"]}],
 
+    %% move the .app.src file to one of the subdirs, out of src/
+    filelib:ensure_dir(filename:join([AppDir, "qux", "fake"])),
+    rebar_file_utils:mv(filename:join([AppDir, "src", Name ++ ".app.src"]),
+                        filename:join([AppDir, "qux", Name ++ ".app.src"])),
+
     {ok, State} = rebar_test_utils:run_and_check(Config, RebarConfig, ["compile"], return),
 
-    ["bar", "baz", "foo", "qux"] = rebar_dir:src_dirs(rebar_state:opts(State), []).
+    ["bar", "baz", "foo", "qux"] = rebar_dir:src_dirs(rebar_state:opts(State), []),
+    rebar_test_utils:run_and_check(Config, RebarConfig, ["compile"],
+                                   {ok, [{app, Name}]}),
+    ok.
+
+dupe_src_dirs_at_root_and_in_erl_opts(Config) ->
+    AppDir = ?config(apps, Config),
+
+    Name = rebar_test_utils:create_random_name("dupe_src_dirs_root_erlopts_"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]),
+
+    RebarConfig = [{erl_opts, [{src_dirs, ["foo", "bar"]}]}, {src_dirs, ["baz", "qux"]}],
+
+    %% move the .app.src file to one of the subdirs, out of src/
+    filelib:ensure_dir(filename:join([AppDir, "qux", "fake"])),
+    filelib:ensure_dir(filename:join([AppDir, "foo", "fake"])),
+    Src1 = filename:join([AppDir, "qux", Name ++ ".app.src"]),
+    Src2 = filename:join([AppDir, "foo", Name ++ ".app.src"]),
+    rebar_file_utils:mv(filename:join([AppDir, "src", Name ++ ".app.src"]),
+                        Src1),
+    %% Then copy it over to create a conflict with dupes
+    file:copy(Src1, Src2),
+
+    {error, {rebar_prv_app_discovery, {multiple_app_files, [Src2, Src1]}}} =
+      rebar_test_utils:run_and_check(Config, RebarConfig, ["compile"], return),
+
+    ok.
 
 extra_src_dirs_at_root_and_in_erl_opts(Config) ->
     AppDir = ?config(apps, Config),
@@ -236,3 +275,52 @@ src_dir_takes_precedence_over_extra(Config) ->
     [{application, _, KVs}] = App,
     Mods = proplists:get_value(modules, KVs),
     true = lists:member(extra, Mods).
+
+src_dir_checkout_dep(Config) ->
+    AppDir = ?config(apps, Config),
+    AppName = rebar_test_utils:create_random_name("src_dir_checkout_app"),
+    DepName = rebar_test_utils:create_random_name("src_dir_checkout_dep"),
+    AtomDep = list_to_atom(DepName),
+
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(AppDir, AppName, Vsn, [kernel, stdlib]),
+    RebarConfig = [{deps, [AtomDep]}],
+
+    DepDir = filename:join([?config(checkouts, Config), DepName]),
+    ct:pal("checkouts dir: ~p", [DepDir]),
+    rebar_test_utils:create_app(DepDir, DepName, Vsn, [kernel, stdlib]),
+
+
+    %% move the .app.src file to one of the subdirs, out of src/
+    rebar_file_utils:mv(filename:join([DepDir, "src"]),
+                        filename:join([DepDir, "qux"])),
+    DepRebarConfig = [{erl_opts, [{src_dirs, ["foo", "bar"]}]},
+                      {src_dirs, ["baz", "qux"]}],
+    file:write_file(filename:join([DepDir, "rebar.config"]),
+                    io_lib:format("~p.~n~p.~n", DepRebarConfig)),
+
+    rebar_test_utils:run_and_check(
+        Config, RebarConfig, ["compile"],
+        {ok, [{checkout, DepName}, {app, AppName}]}
+    ),
+    ok.
+
+app_src_info(Config) ->
+    PrivDir = ?config(priv_dir, Config),
+    AppName1 = rebar_test_utils:create_random_name("app_src_info"),
+    AppDir1 = filename:join(PrivDir, AppName1),
+    {ok, Info1} = rebar_app_info:new(AppName1, "1.0.0", AppDir1),
+    AppSrc1 = filename:join([AppDir1, "src", AppName1 ++ ".app.src"]),
+    ok = filelib:ensure_dir(AppSrc1),
+    ok = file:write_file(AppSrc1, "[]."),
+    ?assertEqual(AppSrc1, rebar_app_info:app_file_src(Info1)),
+
+    AppName2 = rebar_test_utils:create_random_name("app_src_info"),
+    AppDir2 = filename:join(PrivDir, AppName2),
+    {ok, Info2Tmp} = rebar_app_info:new(AppName2, "1.0.0", AppDir2),
+    Info2 = rebar_app_info:set(Info2Tmp, src_dirs, ["foo", "bar", "baz"]),
+    AppSrc2 = filename:join([AppDir2, "bar", AppName2 ++ ".app.src"]),
+    ok = filelib:ensure_dir(AppSrc2),
+    ok = file:write_file(AppSrc2, "[]."),
+    ?assertEqual(AppSrc2, rebar_app_info:app_file_src(Info2)),
+    ok.