Merge pull request #2313 from ferd/fix-recursive-dirs

Fix directory recursion in compiler

src/rebar3.erl

@@ -430,7 +430,18 @@ test_state(State) ->
     ProfileOpts = proplists:get_value(test, Profiles, []),
     ErlOpts = proplists:get_value(erl_opts, ProfileOpts, []),
     TestOpts = safe_define_test_macro(ErlOpts),
-    [{extra_src_dirs, ["test"]}, {erl_opts, TestOpts}].
+    %% Only define the test directory if it wasn't set by the user already,
+    %% otherwise our definition may clash with theirs
+    Extras = rebar_opts:get(Opts, extra_src_dirs, []),
+    ExtrasTest = proplists:get_value(extra_src_dirs, ProfileOpts, []),
+    IsDefined = lists:any(fun({"test", _}) -> true
+                          ;  ("test") -> true
+                          ;  (_) -> false
+                          end, Extras ++ ExtrasTest),
+    case IsDefined of
+        true -> [];
+        false -> [{extra_src_dirs, [{"test", [{recursive, false}]}]}]
+    end ++ [{erl_opts, TestOpts}].
 
 -spec safe_define_test_macro([any()]) -> [any()] | [{'d',atom()} | any()].
 safe_define_test_macro(Opts) ->

src/rebar_compiler.erl

@@ -354,10 +354,24 @@ clean_(CompilerMod, AppInfo, _Label) ->
     ok.
 
 annotate_extras(AppInfo) ->
-    ExtraDirs = rebar_dir:extra_src_dirs(rebar_app_info:opts(AppInfo), []),
-    OldSrcDirs = rebar_app_info:get(AppInfo, src_dirs, ["src"]),
+    AppOpts = rebar_app_info:opts(AppInfo),
+    ExtraDirs = rebar_dir:extra_src_dirs(AppOpts, []),
+    OldSrcDirs = rebar_dir:src_dirs(AppOpts, ["src"]),
+    %% Re-annotate the directories with non-default options if it is the
+    %% case; otherwise, later down the line, the options get dropped with
+    %% profiles. All of this must be done with the rebar_dir functionality
+    %% which properly tracks and handles the various legacy formats for
+    %% recursion setting (erl_opts vs. dir options and profiles)
+    ExtraDirsOpts = [case rebar_dir:recursive(AppOpts, Dir) of
+                         false -> {Dir, [{recursive, false}]};
+                         true -> Dir
+                     end || Dir <- ExtraDirs],
+    OldSrcDirsOpts = [case rebar_dir:recursive(AppOpts, Dir) of
+                          false -> {Dir, [{recursive, false}]};
+                          true -> Dir
+                      end || Dir <- OldSrcDirs],
     AppDir = rebar_app_info:dir(AppInfo),
-    lists:map(fun(Dir) ->
+    lists:map(fun({DirOpt, Dir}) ->
         EbinDir = filename:join(rebar_app_info:out_dir(AppInfo), Dir),
         %% need a unique name to prevent lookup issues that clobber entries
         AppName = unicode:characters_to_binary(
@@ -365,15 +379,15 @@ annotate_extras(AppInfo) ->
         ),
         AppInfo0 = rebar_app_info:name(AppInfo, AppName),
         AppInfo1 = rebar_app_info:ebin_dir(AppInfo0, EbinDir),
-        AppInfo2 = rebar_app_info:set(AppInfo1, src_dirs, [Dir]),
-        AppInfo3 = rebar_app_info:set(AppInfo2, extra_src_dirs, OldSrcDirs),
+        AppInfo2 = rebar_app_info:set(AppInfo1, src_dirs, [DirOpt]),
+        AppInfo3 = rebar_app_info:set(AppInfo2, extra_src_dirs, OldSrcDirsOpts),
         add_to_includes( % give access to .hrl in app's src/
             AppInfo3,
             [filename:join([AppDir, D]) || D <- OldSrcDirs]
         )
     end,
-    [ExtraDir || ExtraDir <- ExtraDirs,
-                 filelib:is_dir(filename:join(AppDir, ExtraDir))]
+    [T || T = {_DirOpt, ExtraDir} <- lists:zip(ExtraDirsOpts, ExtraDirs),
+          filelib:is_dir(filename:join(AppDir, ExtraDir))]
     ).
 
 find_source_files(BaseDir, SrcExt, SrcDirs, Opts) ->

test/rebar_compile_SUITE.erl

@@ -38,7 +38,7 @@ all() ->
      recompile_when_parse_transform_as_opt_changes,
      recompile_when_parse_transform_inline_changes,
      regex_filter_skip, regex_filter_regression,
-     recursive, no_recursive,
+     recursive, no_recursive, extra_recursion,
      always_recompile_when_erl_compiler_options_set,
      dont_recompile_when_erl_compiler_options_env_does_not_change,
      recompile_when_erl_compiler_options_env_changes,
@@ -2500,6 +2500,40 @@ no_recursive(Config) ->
     ?assert(false==lists:member("rec.beam",Files2)),
     ok.
 
+extra_recursion(Config) ->
+    AppDir = ?config(apps, Config),
+
+    Name = rebar_test_utils:create_random_name("app1_"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]),
+    rebar_test_utils:write_src_file(filename:join([AppDir, "src", "src2"]), "rec.erl"),
+    rebar_test_utils:write_src_file(filename:join([AppDir, "test", "test2"]), "rectest.erl"),
+
+    %% Default for src directories: recursive
+    %% default for extra_src directories: non-recursive
+    RebarConfig1 = [],
+    rebar_test_utils:run_and_check(Config, RebarConfig1, ["as", "test", "compile"],
+                                   {ok, [{app, Name}]}),
+    EbinDir = filename:join([AppDir, "_build", "test", "lib", Name, "ebin"]),
+    {ok, Files1} = rebar_utils:list_dir(EbinDir),
+    ?assert(lists:member("rec.beam", Files1)),
+    file:delete(filename:join(EbinDir, "rec.beam")),
+
+    TestEbinDir = filename:join([AppDir, "_build", "test", "lib", Name, "test"]),
+    {ok, TestFiles1} = rebar_utils:list_dir(TestEbinDir),
+    ?assertNot(lists:member("rectest.beam", TestFiles1)),
+
+    RebarConfig2 = [{src_dirs,[{"src",[{recursive,false}]}]},
+                    {extra_src_dirs, [{"test", [{recursive, true}]}]}],
+    rebar_test_utils:run_and_check(Config, RebarConfig2, ["as", "test", "compile"],
+                                   {ok, [{app, Name}]}),
+    {ok, Files2} = rebar_utils:list_dir(EbinDir),
+    ?assertNot(lists:member("rec.beam",Files2)),
+
+    {ok, TestFiles2} = rebar_utils:list_dir(TestEbinDir),
+    ?assert(lists:member("rectest.beam", TestFiles2)),
+    ok.
+
 regex_filter_skip(Config) ->
     AppDir = ?config(apps, Config),
     Name = rebar_test_utils:create_random_name("regex_skip"),