Fix xref compile hook on deps

If xref analysis is being run by a dependency during its compilation
phase, the xref provider will try to add that deps' parents to the check
job while the ebin/ directories for them do not exist. This causes a
crash.

This patch makes it so directories are only added if they are existing,
preventing failure on any compile hook for dependencies and allowing
successful compilation as a best effort.

9 files changed, 187 insertions, 4 deletions

diff --git a/src/rebar_prv_xref.erl b/src/rebar_prv_xref.erl
index c4e72e7..51154b5 100644
--- a/src/rebar_prv_xref.erl
+++ b/src/rebar_prv_xref.erl
@@ -97,8 +97,11 @@ prepare(State) ->
                              rebar_state:get(State, xref_warnings, false)},
                             {verbose, rebar_log:is_verbose(State)}]),
 
-    [{ok, _} = xref:add_directory(xref, rebar_app_info:ebin_dir(App))
-     || App <- rebar_state:project_apps(State)],
+    [{ok, _} = xref:add_directory(xref, Dir)
+     || App <- rebar_state:project_apps(State),
+        %% the directory may not exist in rare cases of a compile
+        %% hook of a dep running xref prior to the full job being done
+        Dir <- [rebar_app_info:ebin_dir(App)], filelib:is_dir(Dir)],
 
     %% Get list of xref checks we want to run
     ConfXrefChecks = rebar_state:get(State, xref_checks,
diff --git a/test/rebar_xref_SUITE.erl b/test/rebar_xref_SUITE.erl
index 09f73a7..d1c071b 100644
--- a/test/rebar_xref_SUITE.erl
+++ b/test/rebar_xref_SUITE.erl
@@ -9,7 +9,8 @@
          end_per_testcase/2,
          all/0,
          xref_test/1,
-         xref_ignore_test/1]).
+         xref_ignore_test/1,
+         xref_dep_hook/1]).
 
 -include_lib("common_test/include/ct.hrl").
 -include_lib("eunit/include/eunit.hrl").
@@ -28,6 +29,15 @@ init_per_suite(Config) ->
 end_per_suite(_Config) ->
     ok.
 
+init_per_testcase(xref_dep_hook, Config) ->
+    Src = filename:join([?config(data_dir, Config), "recursive"]),
+    Dst = filename:join([?config(priv_dir, Config), "recursive"]),
+    ok = rebar_file_utils:cp_r([Src], Dst),
+    GlobalDir = filename:join([?config(priv_dir, Config), "cache"]),
+    State = rebar_state:new([{base_dir, filename:join([Dst, "_build"])}
+                            ,{global_rebar_dir, GlobalDir}
+                            ,{root_dir, Dst}]),
+    [{apps, Dst}, {state, State} | Config];
 init_per_testcase(Case, Config) ->
     UpdConfig = rebar_test_utils:init_rebar_state(Config),
     AppDir = ?config(apps, UpdConfig),
@@ -48,7 +58,7 @@ end_per_testcase(_, _Config) ->
     ok.
 
 all() ->
-    [xref_test, xref_ignore_test].
+    [xref_test, xref_ignore_test, xref_dep_hook].
 
 %% ===================================================================
 %% Test cases
@@ -70,6 +80,9 @@ xref_ignore_test(Config) ->
     Result = rebar3:run(rebar_state:new(State, RebarConfig, AppDir), ["xref"]),
     verify_results(xref_ignore_test, Name, Result).
 
+xref_dep_hook(Config) ->
+    rebar_test_utils:run_and_check(Config, [], ["compile"], {ok, []}).
+
 %% ===================================================================
 %% Helper functions
 %% ===================================================================
diff --git a/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/rebar.config b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/rebar.config
new file mode 100644
index 0000000..cf48edf
--- /dev/null
+++ b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/rebar.config
@@ -0,0 +1,12 @@
+{erl_opts, [debug_info]}.
+{deps, []}.
+
+{xref_checks,[
+    undefined_function_calls,
+    undefined_functions,
+    locals_not_used,
+    deprecated_function_calls,
+    deprecated_functions
+]}.
+
+{provider_hooks, [{post, [{compile, xref}]}]}.
diff --git a/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/src/rebar_issue1.app.src b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/src/rebar_issue1.app.src
new file mode 100644
index 0000000..b935082
--- /dev/null
+++ b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/src/rebar_issue1.app.src
@@ -0,0 +1,16 @@
+{application, rebar_issue1,
+ [{description, "An OTP application"},
+  {vsn, "0.1.0"},
+  {registered, []},
+  {mod, { rebar_issue1_app, []}},
+  {applications,
+   [kernel,
+    stdlib
+   ]},
+  {env,[]},
+  {modules, []},
+
+  {maintainers, []},
+  {licenses, []},
+  {links, []}
+ ]}.
diff --git a/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/src/rebar_issue1_app.erl b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/src/rebar_issue1_app.erl
new file mode 100644
index 0000000..78c88c1
--- /dev/null
+++ b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/src/rebar_issue1_app.erl
@@ -0,0 +1,26 @@
+%%%-------------------------------------------------------------------
+%% @doc rebar_issue1 public API
+%% @end
+%%%-------------------------------------------------------------------
+
+-module(rebar_issue1_app).
+
+-behaviour(application).
+
+%% Application callbacks
+-export([start/2, stop/1]).
+
+%%====================================================================
+%% API
+%%====================================================================
+
+start(_StartType, _StartArgs) ->
+    rebar_issue1_sup:start_link().
+
+%%--------------------------------------------------------------------
+stop(_State) ->
+    ok.
+
+%%====================================================================
+%% Internal functions
+%%====================================================================
diff --git a/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/src/rebar_issue1_sup.erl b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/src/rebar_issue1_sup.erl
new file mode 100644
index 0000000..6e5a9f8
--- /dev/null
+++ b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue1/src/rebar_issue1_sup.erl
@@ -0,0 +1,35 @@
+%%%-------------------------------------------------------------------
+%% @doc rebar_issue1 top level supervisor.
+%% @end
+%%%-------------------------------------------------------------------
+
+-module(rebar_issue1_sup).
+
+-behaviour(supervisor).
+
+%% API
+-export([start_link/0]).
+
+%% Supervisor callbacks
+-export([init/1]).
+
+-define(SERVER, ?MODULE).
+
+%%====================================================================
+%% API functions
+%%====================================================================
+
+start_link() ->
+    supervisor:start_link({local, ?SERVER}, ?MODULE, []).
+
+%%====================================================================
+%% Supervisor callbacks
+%%====================================================================
+
+%% Child :: {Id,StartFunc,Restart,Shutdown,Type,Modules}
+init([]) ->
+    {ok, { {one_for_all, 0, 1}, []} }.
+
+%%====================================================================
+%% Internal functions
+%%====================================================================
diff --git a/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue2/src/rebar_issue2.app.src b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue2/src/rebar_issue2.app.src
new file mode 100644
index 0000000..59ffa35
--- /dev/null
+++ b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue2/src/rebar_issue2.app.src
@@ -0,0 +1,17 @@
+{application, rebar_issue2,
+ [{description, "An OTP application"},
+  {vsn, "0.1.0"},
+  {registered, []},
+  {mod, { rebar_issue2_app, []}},
+  {applications,
+   [kernel,
+    stdlib,
+    rebar_issue1
+   ]},
+  {env,[]},
+  {modules, []},
+
+  {maintainers, []},
+  {licenses, []},
+  {links, []}
+ ]}.
diff --git a/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue2/src/rebar_issue2_app.erl b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue2/src/rebar_issue2_app.erl
new file mode 100644
index 0000000..968966c
--- /dev/null
+++ b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue2/src/rebar_issue2_app.erl
@@ -0,0 +1,26 @@
+%%%-------------------------------------------------------------------
+%% @doc rebar_issue2 public API
+%% @end
+%%%-------------------------------------------------------------------
+
+-module(rebar_issue2_app).
+
+-behaviour(application).
+
+%% Application callbacks
+-export([start/2, stop/1]).
+
+%%====================================================================
+%% API
+%%====================================================================
+
+start(_StartType, _StartArgs) ->
+    rebar_issue2_sup:start_link().
+
+%%--------------------------------------------------------------------
+stop(_State) ->
+    ok.
+
+%%====================================================================
+%% Internal functions
+%%====================================================================
diff --git a/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue2/src/rebar_issue2_sup.erl b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue2/src/rebar_issue2_sup.erl
new file mode 100644
index 0000000..3673548
--- /dev/null
+++ b/test/rebar_xref_SUITE_data/recursive/apps/rebar_issue2/src/rebar_issue2_sup.erl
@@ -0,0 +1,35 @@
+%%%-------------------------------------------------------------------
+%% @doc rebar_issue2 top level supervisor.
+%% @end
+%%%-------------------------------------------------------------------
+
+-module(rebar_issue2_sup).
+
+-behaviour(supervisor).
+
+%% API
+-export([start_link/0]).
+
+%% Supervisor callbacks
+-export([init/1]).
+
+-define(SERVER, ?MODULE).
+
+%%====================================================================
+%% API functions
+%%====================================================================
+
+start_link() ->
+    supervisor:start_link({local, ?SERVER}, ?MODULE, []).
+
+%%====================================================================
+%% Supervisor callbacks
+%%====================================================================
+
+%% Child :: {Id,StartFunc,Restart,Shutdown,Type,Modules}
+init([]) ->
+    {ok, { {one_for_all, 0, 1}, []} }.
+
+%%====================================================================
+%% Internal functions
+%%====================================================================