Another round of refactoring to yield better flexiblity in the base_compiler system

7 files changed, 158 insertions, 150 deletions

diff --git a/src/rebar_base_compiler.erl b/src/rebar_base_compiler.erl
index b6172c2..e1312a9 100644
--- a/src/rebar_base_compiler.erl
+++ b/src/rebar_base_compiler.erl
@@ -28,58 +28,68 @@
 
 -include("rebar.hrl").
 
--export([run/8]).
+-export([run/4, run/7, run/8]).
 
 
 %% ===================================================================
 %% Public API
 %% ===================================================================
 
-run(Config, SourceDir, SourceExt, TargetDir, TargetExt,
-    FirstFiles, CompileFn, Opts) ->
-    SourceExtRe = ".*\\" ++ SourceExt ++ [$$],
+run(Config, FirstFiles, RestFiles, CompileFn) ->
+    %% Compile the first files in sequence
+    compile_each(FirstFiles, Config, CompileFn),
 
-    %% Options:
-    %% recurse_source_dir
-    %% needs_compile_checks - [ fun/2 ]
-    Recursive = proplists:get_bool(recurse_source_dir, Opts),
-
-    %% Find all the source files we can
-    FoundFiles = filelib:fold_files(SourceDir, SourceExtRe, Recursive,
-                                 fun(F, Acc) -> [F | Acc] end, []),
-
-    %% Construct two lists of targets. "FirstTargets" is the list of files which
-    %% must be compiled first and in strict order; "RestTargets" is all remaining files
-    %% that may be compiled in any order.
-    FirstTargets = [{Fs, target_file(Fs, SourceDir, SourceExt, TargetDir, TargetExt)} ||
-                       Fs <- FirstFiles],
-    RestTargets = [{Fs, target_file(Fs, SourceDir, SourceExt, TargetDir, TargetExt)} ||
-                      Fs <- drop_each(FirstFiles, FoundFiles)],
-
-    %% Setup list of functions which determine if a file needs compilation or not. By
-    %% default we just check the last modified date
-    NeedsCompileFns = [ fun check_source_lastmod/3 ] ++
-        rebar_config:get(Config, needs_compile_checks, []),
-
-    %% Compile the first targets in sequence
-    compile_each(FirstTargets, Config, NeedsCompileFns, CompileFn),
-
-    %% Spin up workers
-    case RestTargets of
+    %% Spin up workers for the rest of the files
+    case RestFiles of
         [] ->
             ok;
         _ ->
             Self = self(),
-            F = fun() -> compile_worker(Self, Config, NeedsCompileFns, CompileFn) end,
+            F = fun() -> compile_worker(Self, Config, CompileFn) end,
             Pids = [spawn_monitor(F) || _I <- lists:seq(1,3)],
-            compile_queue(Pids, RestTargets)
+            compile_queue(Pids, RestFiles)
     end.
 
+run(Config, FirstFiles, SourceDir, SourceExt, TargetDir, TargetExt, Compile3Fn) ->
+    run(Config, FirstFiles, SourceDir, SourceExt, TargetDir, TargetExt,
+        Compile3Fn, [check_last_mod]).
+
+run(Config, FirstFiles, SourceDir, SourceExt, TargetDir, TargetExt,
+    Compile3Fn, Opts) ->
+    %% Convert simple extension to proper regex
+    SourceExtRe = ".*\\" ++ SourceExt ++ [$$],
+
+    %% Find all possible source files
+    FoundFiles = rebar_utils:find_files(SourceDir, SourceExtRe),
+
+    %% Remove first files from found files
+    RestFiles = [Source || Source <- FoundFiles,
+                           lists:member(Source, FirstFiles) == false],
+
+    %% Check opts for flag indicating that compile should check lastmod
+    CheckLastMod = proplists:get_bool(check_last_mod, Opts),
+
+    run(Config, FirstFiles, RestFiles,
+        fun(S, C) ->
+                Target = target_file(S, SourceDir, SourceExt, TargetDir, TargetExt),
+                simple_compile_wrapper(S, Target, Compile3Fn, C, CheckLastMod)
+        end).
+
 
 %% ===================================================================
 %% Internal functions
 %% ===================================================================
 
+simple_compile_wrapper(Source, Target, Compile3Fn, Config, false) ->
+    Compile3Fn(Source, Target, Config);
+simple_compile_wrapper(Source, Target, Compile3Fn, Config, true) ->
+    case filelib:last_modified(Target) < filelib:last_modified(Source) of
+        true ->
+            Compile3Fn(Source, Target, Config);
+        false ->
+            skipped
+    end.
+
 target_file(SourceFile, SourceDir, SourceExt, TargetDir, TargetExt) ->
     %% Remove all leading components of the source dir from the file -- we want
     %% to maintain the deeper structure (if any) of the source file path
@@ -97,46 +107,25 @@ remove_common_path1(FilenameParts, _) ->
     filename:join(FilenameParts).
 
 
-drop_each([], List) ->
-    List;
-drop_each([Member | Rest], List) ->
-    drop_each(Rest, lists:delete(Member, List)).
-
-
-needs_compile(_SourceFile, _TargetFile, _Config, []) ->
-    false;
-needs_compile(SourceFile, TargetFile, Config, [Fn | Rest]) ->
-    case Fn(SourceFile, TargetFile, Config) of
-        true ->
-            true;
-        false ->
-            needs_compile(SourceFile, TargetFile, Config, Rest)
-    end.
-
-check_source_lastmod(SourceFile, TargetFile, _Config) ->
-    filelib:last_modified(TargetFile) < filelib:last_modified(SourceFile).
-
-compile(Source, Target, Config, NeedsCompileFns, CompileFn) ->
-    case needs_compile(Source, Target, Config, NeedsCompileFns) of
-        true ->
-            ok = filelib:ensure_dir(Target),
-            CompileFn(Source, Target, Config);
-        false ->
+compile(Source, Config, CompileFn) ->
+    case CompileFn(Source, Config) of
+        ok ->
+            ok;
+        skipped ->
             skipped
     end.
 
 
-
-compile_each([], _Config, _NeedsCompileFns, _CompileFn) ->
+compile_each([], _Config, _CompileFn) ->
     ok;
-compile_each([{Source, Target} | Rest], Config, NeedsCompileFns, CompileFn) ->
-    case compile(Source, Target, Config, NeedsCompileFns, CompileFn) of
+compile_each([Source | Rest], Config, CompileFn) ->
+    case compile(Source, Config, CompileFn) of
         ok ->
             ?CONSOLE("Compiled ~s\n", [Source]);
         skipped ->
             ?INFO("Skipped ~s\n", [Source])
     end,
-    compile_each(Rest, Config, NeedsCompileFns, CompileFn).
+    compile_each(Rest, Config, CompileFn).
 
 
 
@@ -149,8 +138,8 @@ compile_queue(Pids, Targets) ->
                 [] ->
                     Worker ! empty,
                     compile_queue(Pids, Targets);
-                [{Source, Target} | Rest] ->
-                    Worker ! {compile, Source, Target},
+                [Source | Rest] ->
+                    Worker ! {compile, Source},
                     compile_queue(Pids, Rest)
             end;
 
@@ -176,17 +165,17 @@ compile_queue(Pids, Targets) ->
             ?FAIL
     end.
 
-compile_worker(QueuePid, Config, NeedsCompileFns, CompileFn) ->
+compile_worker(QueuePid, Config, CompileFn) ->
     QueuePid ! {next, self()},
     receive
-        {compile, Source, Target} ->
-            case catch(compile(Source, Target, Config, NeedsCompileFns, CompileFn)) of
+        {compile, Source} ->
+            case catch(compile(Source, Config, CompileFn)) of
                 ok ->
                     QueuePid ! {compiled, Source},
-                    compile_worker(QueuePid, Config, NeedsCompileFns, CompileFn);
+                    compile_worker(QueuePid, Config, CompileFn);
                 skipped ->
                     QueuePid ! {skipped, Source},
-                    compile_worker(QueuePid, Config, NeedsCompileFns, CompileFn);
+                    compile_worker(QueuePid, Config, CompileFn);
                 Error ->
                     QueuePid ! {fail, Error},
                     ok
diff --git a/src/rebar_core.erl b/src/rebar_core.erl
index e250a99..e372983 100644
--- a/src/rebar_core.erl
+++ b/src/rebar_core.erl
@@ -237,8 +237,8 @@ apply_commands([Command | Rest], Modules, Config, ModuleFile) ->
                 {error, failed} ->
                     ?FAIL;
                 Other ->
-                    ?ERROR("~p failed while processing ~s: ~p", [Command, Dir, Other]),
-                    ?FAIL
+                    ?ABORT("~p failed while processing ~s: ~s",
+                           [Command, Dir, io_lib:print(Other, 1,80,-1)])
             end
     end.
 
diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl
index 4e083ab..ee51cd0 100644
--- a/src/rebar_erlc_compiler.erl
+++ b/src/rebar_erlc_compiler.erl
@@ -29,8 +29,7 @@
 -export([compile/2,
          clean/2]).
 
- %% make available for rebar_eunit until there is a better option
--export([hrls_check/3]).
+-export([doterl_compile/2]).
 
 -include("rebar.hrl").
 
@@ -39,16 +38,10 @@
 %% ===================================================================
 
 compile(Config, _AppFile) ->
-    rebar_base_compiler:run(Config, "src", ".erl", "ebin", ".beam",
-                            rebar_config:get_list(Config, erl_first_files, []),
-                            fun compile_erl/3,
-                            [recurse_source_dir,
-                             {needs_compile_checks, [fun hrls_check/3]}]),
-
-    rebar_base_compiler:run(Config, "mibs", ".mib", "priv/mibs", ".bin",
-                            rebar_config:get_list(Config, mib_first_files, []),
-                            fun compile_mib/3,
-                            []).
+    doterl_compile(Config, "ebin"),
+    rebar_base_compiler:run(Config, rebar_config:get_list(Config, mib_first_files, []),
+                            "mibs", ".mib", "priv/mibs", ".bin",
+                            fun compile_mib/3).
 
 clean(_Config, _AppFile) ->
     %% TODO: This would be more portable if it used Erlang to traverse
@@ -68,66 +61,97 @@ clean(_Config, _AppFile) ->
 
 
 %% ===================================================================
-%% Internal functions
+%% .erl Compilation API (externally used by only eunit)
 %% ===================================================================
 
-hrls_check(Source, Target, Config) ->
-    TargetLastMod = filelib:last_modified(Target),
-    lists:any(fun(I) -> TargetLastMod < filelib:last_modified(I) end,
-              list_hrls(Source, Config)).
+doterl_compile(Config, Outdir) ->
+    FirstErls = rebar_config:get_list(Config, erl_first_files, []),
+    RestErls  = [Source || Source <- rebar_utils:find_files("src", ".*.erl"),
+                           lists:member(Source, FirstErls) == false],
+    rebar_base_compiler:run(Config, FirstErls, RestErls,
+                            fun(S, C) -> internal_erl_compile(S, C, Outdir) end).
 
 
-list_hrls(Src, Config) ->
-    case epp:open(Src, include_path(Src, Config)) of
+%% ===================================================================
+%% Internal functions
+%% ===================================================================
+
+include_path(Source, Config) ->
+    ErlOpts = rebar_config:get(Config, erl_opts, []),
+    [filename:dirname(Source)] ++ proplists:get_all_values(i, ErlOpts).
+
+inspect(Source, IncludePath) ->
+    ModuleDefault = filename:basename(Source, ".erl"),
+    case epp:open(Source, IncludePath) of
         {ok, Epp} ->
-            %% check include for erlang files
-            extract_includes(Epp, Src);
-        _ ->
-            false
+            inspect_epp(Epp, ModuleDefault, []);
+        {error, Reason} ->
+            ?DEBUG("Failed to inspect ~s: ~p\n", [Source, Reason]),
+            {ModuleDefault, []}
     end.
 
-
-extract_includes(Epp, Src) ->
+inspect_epp(Epp, Module, Includes) ->
     case epp:parse_erl_form(Epp) of
-        {ok, {attribute, 1, file, {Src, 1}}} ->
-            extract_includes(Epp, Src);
+        {ok, {attribute, _, module, ActualModule}} when is_list(ActualModule) ->
+            %% If the module name includes package info, we get a list of atoms...
+            case is_list(ActualModule) of
+                true ->
+                    ActualModuleStr = string:join([atom_to_list(P) || P <- ActualModule], ".");
+                false ->
+                    ActualModuleStr = atom_to_list(ActualModule)
+            end,
+            inspect_epp(Epp, ActualModuleStr, Includes);
+        {ok, {attribute, 1, file, {Module, 1}}} ->
+            inspect_epp(Epp, Module, Includes);
         {ok, {attribute, 1, file, {IncFile, 1}}} ->
-            [IncFile|extract_includes(Epp, Src)];
-        {ok, _} ->
-            extract_includes(Epp, Src);
+            inspect_epp(Epp, Module, [IncFile | Includes]);
         {eof, _} ->
             epp:close(Epp),
-            [];
-        {error, _Error} ->
-            extract_includes(Epp, Src)
+            {Module, Includes};
+        _ ->
+            inspect_epp(Epp, Module, Includes)
     end.
 
-include_path(Source, Config) ->
-    [filename:dirname(Source) | compile_opts(Config, i)].
-
-compile_opts(Config, Key) ->
-    rebar_config:get_list(Config, Key, []).
-
-compile_erl(Source, Target, Config) ->
-    Opts = [{i, "include"}, {outdir, filename:dirname(Target)}, report, return] ++
-            compile_opts(Config, erl_opts),
-    case compile:file(Source, Opts) of
-        {ok, _, []} ->
-            ok;
-        {ok, _, _Warnings} ->
-            %% We got at least one warning -- if fail_on_warning is in options, fail
-            case lists:member(fail_on_warning, Opts) of
-                true ->
-                    ?FAIL;
-                false ->
-                    ok
+needs_compile(Source, Target, Hrls) ->
+    TargetLastMod = filelib:last_modified(Target),
+    lists:any(fun(I) -> TargetLastMod < filelib:last_modified(I) end,
+              [Source] ++ Hrls).
+
+
+internal_erl_compile(Source, Config, Outdir) ->
+    %% Determine the target name and includes list by inspecting the source file
+    {Module, Hrls} = inspect(Source, include_path(Source, Config)),
+
+    %% Construct the target filename
+    Target = filename:join([Outdir | string:tokens(Module, ".")]) ++ ".beam",
+
+    %% If the file needs compilation, based on last mod date of includes or
+    %% the target,
+    case needs_compile(Source, Target, Hrls) of
+        true ->
+            Opts = [{i, "include"}, {outdir, filename:dirname(Target)}, report, return] ++
+                rebar_config:get(Config, erl_opts, []),
+            case compile:file(Source, Opts) of
+                {ok, _, []} ->
+                    ok;
+                {ok, _, _Warnings} ->
+                    %% We got at least one warning -- if fail_on_warning is in options, fail
+                    case lists:member(fail_on_warning, Opts) of
+                        true ->
+                            ?FAIL;
+                        false ->
+                            ok
+                    end;
+                _ ->
+                    ?FAIL
             end;
-        _ ->
-            ?FAIL
+        false ->
+            skipped
     end.
 
 compile_mib(Source, _Target, Config) ->
-    Opts = [{outdir, "priv/mibs"}, {i, ["priv/mibs"]}] ++ compile_opts(Config, mib_opts),
+    Opts = [{outdir, "priv/mibs"}, {i, ["priv/mibs"]}] ++
+        rebar_config:get(Config, mib_opts, []),
     case snmpc:compile(Source, Opts) of
         {ok, _} ->
             ok;
diff --git a/src/rebar_erlydtl_compiler.erl b/src/rebar_erlydtl_compiler.erl
index ecdc8c0..9a40f67 100644
--- a/src/rebar_erlydtl_compiler.erl
+++ b/src/rebar_erlydtl_compiler.erl
@@ -82,10 +82,10 @@
 
 compile(Config, _AppFile) ->
     DtlOpts = erlydtl_opts(Config),
-    rebar_base_compiler:run(Config, option(doc_root, DtlOpts), option(source_ext, DtlOpts),
+    rebar_base_compiler:run(Config, [],
+                            option(doc_root, DtlOpts), option(source_ext, DtlOpts),
                             option(out_dir, DtlOpts), option(module_ext, DtlOpts) ++ ".beam",
-                            [], fun compile_dtl/3,
-                            [{needs_compile_checks, [fun referenced_dtls/3]}]).
+                            fun compile_dtl/3, [{check_last_mod, false}]).
 
 
 %% ===================================================================
@@ -128,7 +128,7 @@ referenced_dtls1(Step, Config, Seen) ->
                              sets:union(New, Seen))
     end.
 
-compile_dtl(Source, _Target, Config) ->
+compile_dtl(Source, Target, Config) ->
     case code:which(erlydtl) of
         non_existing ->
             ?CONSOLE(
@@ -140,6 +140,7 @@ compile_dtl(Source, _Target, Config) ->
                "===============================================~n~n", []),
             ?FAIL;
         _ ->
+            %% TODO: Check last mod on target and referenced DTLs here..
             DtlOpts = erlydtl_opts(Config),
             %% ensure that doc_root and out_dir are defined,
             %% using defaults if necessary
diff --git a/src/rebar_eunit.erl b/src/rebar_eunit.erl
index 74c58af..1a3a45f 100644
--- a/src/rebar_eunit.erl
+++ b/src/rebar_eunit.erl
@@ -51,12 +51,9 @@ eunit(Config, _File) ->
     %% Make sure ?EUNIT_DIR/ directory exists (tack on dummy module)
     ok = filelib:ensure_dir(?EUNIT_DIR ++ "/foo"),
 
-    %% Compile all erlang from src/ into ?EUNIT_DIR
-    rebar_base_compiler:run(Config, "src", ".erl", ?EUNIT_DIR, ".beam",
-                            rebar_config:get_list(Config, erl_first_files, []),
-                            fun compile_erl/3,
-                            [recurse_source_dir,
-                             {needs_compile_checks, [fun rebar_erlc_compiler:hrls_check/3]}]),
+    %% Compile erlang code to ?EUNIT_DIR, using a tweaked config
+    %% with appropriate defines for eunit
+    rebar_erlc_compiler:doterl_compile(eunit_config(Config), ?EUNIT_DIR),
 
     %% Build a list of all the .beams in ?EUNIT_DIR -- use this for cover
     %% and eunit testing. Normally you can just tell cover and/or eunit to
@@ -127,7 +124,7 @@ clean(_Config, _File) ->
 %% Internal functions
 %% ===================================================================
 
-compile_erl(Source, Target, Config) ->
+eunit_config(Config) ->
     case is_quickcheck_avail() of
         true ->
             EqcOpts = [{d, 'EQC'}];
@@ -137,15 +134,9 @@ compile_erl(Source, Target, Config) ->
 
     ErlOpts = rebar_config:get_list(Config, erl_opts, []),
     EunitOpts = rebar_config:get_list(Config, eunit_compile_opts, []),
-    Opts = [{i, "include"}, {outdir, filename:dirname(Target)},
-            {d, 'TEST'}, debug_info, report] ++
+    Opts = [{d, 'TEST'}, debug_info] ++
         ErlOpts ++ EunitOpts ++ EqcOpts,
-    case compile:file(Source, Opts) of
-        {ok, _} ->
-            ok;
-        error ->
-            ?FAIL
-    end.
+    rebar_config:set(Config, erl_opts, Opts).
 
 is_quickcheck_avail() ->
     case erlang:get(is_quickcheck_avail) of
diff --git a/src/rebar_lfe_compiler.erl b/src/rebar_lfe_compiler.erl
index 78a2c3d..4451530 100644
--- a/src/rebar_lfe_compiler.erl
+++ b/src/rebar_lfe_compiler.erl
@@ -38,9 +38,8 @@
 
 compile(Config, _AppFile) ->
     FirstFiles = rebar_config:get_list(Config, lfe_first_files, []),
-    rebar_base_compiler:run(Config, "src", ".lfe", "ebin", ".beam",
-                            FirstFiles,
-                            fun compile_lfe/3, []).
+    rebar_base_compiler:run(Config, FirstFiles, "src", ".lfe", "ebin", ".beam",
+                            fun compile_lfe/3).
 
 
 %% ===================================================================
diff --git a/src/rebar_utils.erl b/src/rebar_utils.erl
index 05f9349..9ecbbe6 100644
--- a/src/rebar_utils.erl
+++ b/src/rebar_utils.erl
@@ -31,7 +31,8 @@
          get_os/0,
          sh/2, sh/3,
          sh_failfast/2,
-	 now_str/0]).
+         find_files/2,
+         now_str/0]).
 
 -include("rebar.hrl").
 
@@ -80,6 +81,9 @@ sh(Command, Env, Dir) ->
 sh_failfast(Command, Env) ->
     sh(Command, Env).
 
+find_files(Dir, Regex) ->
+    filelib:fold_files(Dir, Regex, true, fun(F, Acc) -> [F | Acc] end, []).
+
 now_str() ->
     {{Year, Month, Day}, {Hour, Minute, Second}} = calendar:local_time(),
     lists:flatten(io_lib:format("~4b/~2..0b/~2..0b ~2..0b:~2..0b:~2..0b",