From 37396c7f8b484282936a9925b8e65217789209b9 Mon Sep 17 00:00:00 2001
From: Fred Hebert <mononcqc@ferd.ca>
Date: Tue, 1 Sep 2020 09:42:04 -0400
Subject: [PATCH] Fix Recursive Behaviour Lookup Regression

This was due to a subtle mistake in the new code in
https://github.com/erlang/rebar3/pull/2322 which optimized the search
behaviour of files not found by EPP. The patch mistakenly always kept
the top-level src/ directory for an app even if it did a lookup in
recursive ones.

This patch tracks and maintains the proper subdirectory hierarchy. To
keep performance adequate, the lists:keyfind/3 function is used, which
is also now a NIF to the same extent as lists:member/2; manual lookups
would likely end up reducing performance, particularly in deep
hierarchies.

A test case has been added to track the regression.
Reported by @elbrujohalcon
---
 src/rebar_compiler_epp.erl   |  8 +++---
 test/rebar_compile_SUITE.erl | 48 ++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/src/rebar_compiler_epp.erl b/src/rebar_compiler_epp.erl
index 5334ea15..53fb3986 100644
--- a/src/rebar_compiler_epp.erl
+++ b/src/rebar_compiler_epp.erl
@@ -101,9 +101,9 @@ resolve(_Name, Fs, []) ->
     {false, Fs};
 resolve(Name, Fs, [Dir | Tail]) ->
     {NewFs, Files} = list_directory(Dir, Fs),
-    case lists:member(Name, Files) of
-        true ->
-            {{true, filename:join(Dir, Name)}, NewFs};
+    case lists:keyfind(Name, 2, Files) of
+        {FullDir, _} ->
+            {{true, filename:join(FullDir, Name)}, NewFs};
         false ->
             resolve(Name, NewFs, Tail)
     end.
@@ -136,7 +136,7 @@ list_directory(Dir, Cache) ->
                                     %% ignore all but *.erl files
                                     case filename:extension(File) =:= ".erl" of
                                         true ->
-                                            {DirCache, [File | Files]};
+                                            {DirCache, [{Dir, File} | Files]};
                                         false ->
                                             Acc
                                     end
diff --git a/test/rebar_compile_SUITE.erl b/test/rebar_compile_SUITE.erl
index 9362596b..d2717f54 100644
--- a/test/rebar_compile_SUITE.erl
+++ b/test/rebar_compile_SUITE.erl
@@ -20,6 +20,7 @@ all() ->
      recompile_when_opts_included_hrl_changes,
      recompile_when_foreign_included_hrl_changes,
      recompile_when_foreign_behaviour_changes,
+     recompile_when_recursive_behaviour_changes,
      recompile_when_opts_change, recompile_when_dag_opts_change,
      dont_recompile_when_opts_dont_change, dont_recompile_yrl_or_xrl,
      delete_beam_if_source_deleted,
@@ -890,6 +891,53 @@ recompile_when_foreign_behaviour_changes(Config) ->
 
     ?assert(ModTime =/= NewModTime).
 
+recompile_when_recursive_behaviour_changes(Config) ->
+    AppDir = ?config(apps, Config),
+    AppsDir = filename:join([AppDir, "apps"]),
+
+    Name1 = rebar_test_utils:create_random_name("app1_"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(filename:join(AppsDir, Name1),
+                                Name1, Vsn, [kernel, stdlib]),
+
+    ExtraSrc = <<"-module(test_behaviour_include).\n"
+                 "-export([main/0]).\n"
+                 "-behaviour(app1_behaviour).\n"
+                 "main() -> 1.\n">>,
+
+    Behaviour = <<"-module(app1_behaviour).\n"
+                  "-callback main() -> term().\n">>,
+    ok = filelib:ensure_dir(filename:join([AppsDir, Name1, "src", "dummy"])),
+    ok = filelib:ensure_dir(filename:join([AppsDir, Name1, "src", "sub", "dummy"])),
+    BehaviourFile = filename:join([AppsDir, Name1, "src", "sub", "app1_behaviour.erl"]),
+    ok = file:write_file(filename:join([AppsDir, Name1, "src", "test_behaviour_include.erl"]), ExtraSrc),
+    ok = file:write_file(BehaviourFile, Behaviour),
+
+    rebar_test_utils:run_and_check(Config, [], ["compile"], {ok, [{app, Name1}]}),
+
+    EbinDir = filename:join([AppDir, "_build", "default", "lib", Name1, "ebin"]),
+    {ok, Files} = rebar_utils:list_dir(EbinDir),
+    ModTime = [filelib:last_modified(filename:join([EbinDir, F]))
+               || F <- Files,
+                  filename:extension(F) == ".beam",
+                  filename:basename(F) =/= "app1_behaviour.beam"],
+
+    timer:sleep(1000),
+
+    NewBehaviour = <<"-module(app1_behaviour).\n"
+                     "-callback main(_) -> term().\n">>,
+    ok = file:write_file(BehaviourFile, NewBehaviour),
+
+    rebar_test_utils:run_and_check(Config, [], ["compile"], {ok, [{app, Name1}]}),
+
+    {ok, NewFiles} = rebar_utils:list_dir(EbinDir),
+    NewModTime = [filelib:last_modified(filename:join([EbinDir, F]))
+                  || F <- NewFiles,
+                     filename:extension(F) == ".beam",
+                     filename:basename(F) =/= "app1_behaviour.beam"],
+
+    ?assert(ModTime =/= NewModTime).
+
 recompile_when_opts_change(Config) ->
     AppDir = ?config(apps, Config),