consolidate app validation and exist checks

src/rebar_app_discover.erl

@@ -5,9 +5,7 @@
          find_unbuilt_apps/1,
          find_apps/1,
          find_apps/2,
-         find_app/2,
-         validate_application_info/1,
-         validate_application_info/2]).
+         find_app/2]).
 
 -include_lib("providers/include/providers.hrl").
 
@@ -104,14 +102,14 @@ find_app(AppDir, Validate) ->
                        end,
             case Validate of
                 valid ->
-                    case validate_application_info(AppInfo2) of
+                    case rebar_app_utils:validate_application_info(AppInfo2) of
                         true ->
                             {true, AppInfo2};
                         _ ->
                             false
                     end;
                 invalid ->
-                    case validate_application_info(AppInfo2) of
+                    case rebar_app_utils:validate_application_info(AppInfo2) of
                         true ->
                             false;
                         _ ->
@@ -157,46 +155,3 @@ create_app_info(AppDir, AppFile) ->
         _ ->
             error
     end.
-
--spec validate_application_info(rebar_app_info:t()) -> boolean().
-validate_application_info(AppInfo) ->
-    validate_application_info(AppInfo, rebar_app_info:app_details(AppInfo)).
-
-validate_application_info(AppInfo, AppDetail) ->
-    EbinDir = rebar_app_info:ebin_dir(AppInfo),
-    case rebar_app_info:app_file(AppInfo) of
-        undefined ->
-            false;
-        AppFile ->
-            case get_modules_list(AppFile, AppDetail) of
-                {ok, List} ->
-                    has_all_beams(EbinDir, List);
-                _Error ->
-                    ?PRV_ERROR({module_list, AppFile})
-            end
-    end.
-
--spec get_modules_list(file:filename_all(), proplists:proplist()) ->
-                              {ok, list()} |
-                              {warning, Reason::term()} |
-                              {error, Reason::term()}.
-get_modules_list(AppFile, AppDetail) ->
-    case proplists:get_value(modules, AppDetail) of
-        undefined ->
-            {warning, {invalid_app_file, AppFile}};
-        ModulesList ->
-            {ok, ModulesList}
-    end.
-
--spec has_all_beams(file:filename_all(), list()) -> true | providers:error().
-has_all_beams(EbinDir, [Module | ModuleList]) ->
-    BeamFile = filename:join([EbinDir,
-                              ec_cnv:to_list(Module) ++ ".beam"]),
-    case filelib:is_file(BeamFile) of
-        true ->
-            has_all_beams(EbinDir, ModuleList);
-        false ->
-            ?PRV_ERROR({missing_module, Module})
-    end;
-has_all_beams(_, []) ->
-    true.

src/rebar_app_info.erl

@@ -224,7 +224,7 @@ state(#app_info_t{state=State}) ->
 
 -spec valid(t()) -> boolean().
 valid(AppInfo=#app_info_t{valid=undefined}) ->
-    case rebar_app_discover:validate_application_info(AppInfo) of
+    case rebar_app_utils:validate_application_info(AppInfo) of
         true ->
             true;
         _ ->

src/rebar_app_utils.erl

@@ -28,13 +28,13 @@
 
 -export([find/2,
          find/3,
-         is_app_dir/0, is_app_dir/1,
          is_app_src/1,
          app_src_to_app/1,
-         load_app_file/2,
-         app_vsn/2]).
+         validate_application_info/1,
+         validate_application_info/2]).
 
 -include("rebar.hrl").
+-include_lib("providers/include/providers.hrl").
 
 %% ===================================================================
 %% Public API
@@ -51,35 +51,6 @@ find(Name, Vsn, Apps) ->
                               andalso rebar_app_info:original_vsn(App) =:= Vsn
                   end, Apps).
 
--spec is_app_dir() -> {true, file:name()} | false.
-is_app_dir() ->
-    is_app_dir(rebar_dir:get_cwd()).
-
--spec is_app_dir(file:name()) -> {true, file:name()} | false.
-is_app_dir(Dir) ->
-    SrcDir = filename:join([Dir, "src"]),
-    AppSrc = filename:join([SrcDir, "*.app.src"]),
-    case filelib:wildcard(AppSrc) of
-        [AppSrcFile] ->
-            {true, AppSrcFile};
-        [] ->
-            EbinDir = filename:join([Dir, "ebin"]),
-            App = filename:join([EbinDir, "*.app"]),
-            case filelib:wildcard(App) of
-                [AppFile] ->
-                    {true, AppFile};
-                [] ->
-                    false;
-                _ ->
-                    ?ERROR("More than one .app file in ~s~n", [EbinDir]),
-                    false
-            end;
-        _ ->
-            ?ERROR("More than one .app.src file in ~s~n", [SrcDir]),
-            false
-    end.
-
-
 is_app_src(Filename) ->
     %% If removing the extension .app.src yields a shorter name,
     %% this is an .app.src file.
@@ -91,58 +62,49 @@ app_src_to_app(Filename) ->
     filelib:ensure_dir(AppFile),
     AppFile.
 
-app_vsn(Config, AppFile) ->
-    case load_app_file(Config, AppFile) of
-        {ok, Config1, _, AppInfo} ->
-            AppDir = filename:dirname(filename:dirname(AppFile)),
-            rebar_utils:vcs_vsn(Config1, get_value(vsn, AppInfo, AppFile),
-                                AppDir);
-        {error, Reason} ->
-            ?ABORT("Failed to extract vsn from ~s: ~p\n",
-                   [AppFile, Reason])
-    end.
+-spec validate_application_info(rebar_app_info:t()) -> boolean().
+validate_application_info(AppInfo) ->
+    validate_application_info(AppInfo, rebar_app_info:app_details(AppInfo)).
 
-load_app_file(_State, undefined) -> {error, missing_app_file};
-load_app_file(State, Filename) ->
-    AppFile = {app_file, Filename},
-    case rebar_state:get(State, {appfile, AppFile}, undefined) of
+validate_application_info(AppInfo, AppDetail) ->
+    EbinDir = rebar_app_info:ebin_dir(AppInfo),
+    case rebar_app_info:app_file(AppInfo) of
         undefined ->
-            case consult_app_file(Filename) of
-                {ok, [{application, AppName, AppData}]} ->
-                    State1 = rebar_state:set(State,
-                                             {appfile, AppFile},
-                                             {AppName, AppData}),
-                    {ok, State1, AppName, AppData};
-                {error, _} = Error ->
-                    Error;
-                Other ->
-                    {error, {unexpected_terms, Other}}
-            end;
-        {AppName, AppData} ->
-            {ok, State, AppName, AppData}
+            false;
+        AppFile ->
+            case get_modules_list(AppFile, AppDetail) of
+                {ok, List} ->
+                    has_all_beams(EbinDir, List);
+                _Error ->
+                    ?PRV_ERROR({module_list, AppFile})
+            end
     end.
 
 %% ===================================================================
 %% Internal functions
 %% ===================================================================
 
-%% In the case of *.app.src we want to give the user the ability to
-%% dynamically script the application resource file (think dynamic version
-%% string, etc.), in a way similar to what can be done with the rebar
-%% config. However, in the case of *.app, rebar should not manipulate
-%% that file. This enforces that dichotomy between app and app.src.
-consult_app_file(Filename) ->
-    case lists:suffix(".app.src", Filename) of
-        false ->
-            file:consult(Filename);
-        true ->
-            {ok, rebar_config:consult_file(Filename)}
-    end.
-
-get_value(Key, AppInfo, AppFile) ->
-    case proplists:get_value(Key, AppInfo) of
+-spec get_modules_list(file:filename_all(), proplists:proplist()) ->
+                              {ok, list()} |
+                              {warning, Reason::term()} |
+                              {error, Reason::term()}.
+get_modules_list(AppFile, AppDetail) ->
+    case proplists:get_value(modules, AppDetail) of
         undefined ->
-            ?ABORT("Failed to get app value '~p' from '~s'~n", [Key, AppFile]);
-        Value ->
-            Value
+            {warning, {invalid_app_file, AppFile}};
+        ModulesList ->
+            {ok, ModulesList}
     end.
+
+-spec has_all_beams(file:filename_all(), list()) -> true | providers:error().
+has_all_beams(EbinDir, [Module | ModuleList]) ->
+    BeamFile = filename:join([EbinDir,
+                              ec_cnv:to_list(Module) ++ ".beam"]),
+    case filelib:is_file(BeamFile) of
+        true ->
+            has_all_beams(EbinDir, ModuleList);
+        false ->
+            ?PRV_ERROR({missing_module, Module})
+    end;
+has_all_beams(_, []) ->
+    true.

src/rebar_config.erl

@@ -91,7 +91,7 @@ try_consult(File) ->
         {error, enoent} ->
             [];
         {error, Reason} ->
-            ?ABORT("Failed to read config file ~s: ~p", [File, Reason])
+            ?ABORT("Failed to read config file ~s:~n ~p", [File, Reason])
     end.
 
 bs(Vars) ->

src/rebar_erlc_compiler.erl

@@ -270,7 +270,7 @@ opts_changed(Opts, Target) ->
             code:purge(Mod),
             lists:sort(Opts) =/= lists:sort(proplists:get_value(options,
                                                                 Compile));
-        {error, nofile} -> true
+        {error, _} -> true
     end.
 
 check_erlcinfo(_Config, #erlcinfo{vsn=?ERLCINFO_VSN}) ->

src/rebar_otp_app.erl

@@ -27,8 +27,7 @@
 -module(rebar_otp_app).
 
 -export([compile/2,
-         format_error/1,
-         clean/2]).
+         format_error/1]).
 
 -include("rebar.hrl").
 -include_lib("providers/include/providers.hrl").
@@ -42,52 +41,33 @@ compile(State, App) ->
     %% written out as a ebin/*.app file. That resulting file will then
     %% be validated as usual.
     Dir = ec_cnv:to_list(rebar_app_info:dir(App)),
-    {State2, App1} = case rebar_app_info:app_file_src(App) of
-                          undefined ->
-                              {State, App};
-                          AppFileSrc ->
-                              {State1, File} = preprocess(State, Dir, AppFileSrc),
-                              {State1, rebar_app_info:app_file(App, File)}
-                      end,
+    App1 = case rebar_app_info:app_file_src(App) of
+               undefined ->
+                   App;
+               AppFileSrc ->
+                   File = preprocess(State, Dir, AppFileSrc),
+                   rebar_app_info:app_file(App, File)
+           end,
 
     %% Load the app file and validate it.
-    validate_app(State2, App1).
-
-
+    validate_app(State, App1).
 
 format_error({file_read, File, Reason}) ->
     io_lib:format("Failed to read ~s for processing: ~p", [File, Reason]);
 format_error({invalid_name, File, AppName}) ->
     io_lib:format("Invalid ~s: name of application (~p) must match filename.", [File, AppName]).
 
-clean(_State, File) ->
-    %% If the app file is a .app.src, delete the generated .app file
-    case rebar_app_utils:is_app_src(File) of
-        true ->
-            case file:delete(rebar_app_utils:app_src_to_app(File)) of
-                ok ->
-                    ok;
-                {error, enoent} ->
-                    %% The file not existing is OK, we can ignore the error.
-                    ok;
-                Other ->
-                    Other
-            end;
-        false ->
-            ok
-    end.
-
 %% ===================================================================
 %% Internal functions
 %% ===================================================================
 
 validate_app(State, App) ->
     AppFile = rebar_app_info:app_file(App),
-    case rebar_app_utils:load_app_file(State, AppFile) of
-        {ok, State1, AppName, AppData} ->
+    case consult_app_file(AppFile) of
+        {ok, [{application, AppName, AppData}]} ->
             case validate_name(AppName, AppFile) of
                 ok ->
-                    validate_app_modules(State1, App, AppData);
+                    validate_app_modules(State, App, AppData);
                 Error ->
                     Error
             end;
@@ -102,7 +82,7 @@ validate_app_modules(State, App, AppData) ->
     AppVsn = proplists:get_value(vsn, AppData),
     case rebar_state:get(State, validate_app_modules, true) of
         true ->
-            case rebar_app_discover:validate_application_info(App, AppData) of
+            case rebar_app_utils:validate_application_info(App, AppData) of
                 true ->
                     {ok, rebar_app_info:original_vsn(App, AppVsn)};
                 Error ->
@@ -113,16 +93,16 @@ validate_app_modules(State, App, AppData) ->
     end.
 
 preprocess(State, Dir, AppSrcFile) ->
-    case rebar_app_utils:load_app_file(State, AppSrcFile) of
-        {ok, State1, AppName, AppData} ->
+    case consult_app_file(AppSrcFile) of
+        {ok, [{application, AppName, AppData}]} ->
             %% Look for a configuration file with vars we want to
             %% substitute. Note that we include the list of modules available in
             %% ebin/ and update the app data accordingly.
-            AppVars = load_app_vars(State1) ++ [{modules, ebin_modules(Dir)}],
+            AppVars = load_app_vars(State) ++ [{modules, ebin_modules(Dir)}],
             A1 = apply_app_vars(AppVars, AppData),
 
             %% AppSrcFile may contain instructions for generating a vsn number
-            {State2, Vsn} = rebar_app_utils:app_vsn(State1, AppSrcFile),
+            Vsn = app_vsn(AppSrcFile),
             A2 = lists:keystore(vsn, 1, A1, {vsn, Vsn}),
 
             %% systools:make_relup/4 fails with {missing_param, registered}
@@ -140,7 +120,7 @@ preprocess(State, Dir, AppSrcFile) ->
             %% on the code path
             true = code:add_path(filename:absname(filename:dirname(AppFile))),
 
-            {State2, AppFile};
+            AppFile;
         {error, Reason} ->
             ?PRV_ERROR({file_read, AppSrcFile, Reason})
     end.
@@ -185,3 +165,33 @@ ensure_registered(AppData) ->
             %% We could further check whether the value is a list of atoms.
             AppData
     end.
+
+%% In the case of *.app.src we want to give the user the ability to
+%% dynamically script the application resource file (think dynamic version
+%% string, etc.), in a way similar to what can be done with the rebar
+%% config. However, in the case of *.app, rebar should not manipulate
+%% that file. This enforces that dichotomy between app and app.src.
+consult_app_file(Filename) ->
+    case lists:suffix(".app.src", Filename) of
+        false ->
+            file:consult(Filename);
+        true ->
+            {ok, rebar_config:consult_file(Filename)}
+    end.
+
+app_vsn(AppFile) ->
+    case consult_app_file(AppFile) of
+        {ok, [{application, _AppName, AppData}]} ->
+            AppDir = filename:dirname(filename:dirname(AppFile)),
+            rebar_utils:vcs_vsn(get_value(vsn, AppData, AppFile), AppDir);
+        {error, Reason} ->
+            ?ABORT("Failed to consult app file ~s: ~p\n", [AppFile, Reason])
+    end.
+
+get_value(Key, AppInfo, AppFile) ->
+    case proplists:get_value(Key, AppInfo) of
+        undefined ->
+            ?ABORT("Failed to get app value '~p' from '~s'~n", [Key, AppFile]);
+        Value ->
+            Value
+    end.

src/rebar_prv_install_deps.erl

@@ -370,10 +370,10 @@ maybe_fetch(AppInfo, Upgrade, Seen, State) ->
         true ->
             false;
         false ->
-            case not app_exists(AppDir) of
-                true ->
-                    fetch_app(AppInfo, AppDir, State);
+            case rebar_app_discover:find_app(AppDir, all) of
                 false ->
+                    fetch_app(AppInfo, AppDir, State);
+                {true, _} ->
                     case sets:is_element(rebar_app_info:name(AppInfo), Seen) of
                         true ->
                             false;
@@ -456,19 +456,6 @@ new_dep(DepsDir, Name, Vsn, Source, State) ->
                                rebar_state:overrides(S, ParentOverrides++Overrides)),
     rebar_app_info:source(Dep1, Source).
 
-app_exists(AppDir) ->
-    case rebar_app_utils:is_app_dir(filename:absname(AppDir)++"-*") of
-        {true, _} ->
-            true;
-        _ ->
-            case rebar_app_utils:is_app_dir(filename:absname(AppDir)) of
-                {true, _} ->
-                    true;
-                _ ->
-                    false
-            end
-    end.
-
 fetch_app(AppInfo, AppDir, State) ->
     ?INFO("Fetching ~s (~p)", [rebar_app_info:name(AppInfo), rebar_app_info:source(AppInfo)]),
     Source = rebar_app_info:source(AppInfo),

src/rebar_utils.erl

@@ -43,7 +43,7 @@
          beams/1,
          find_executable/1,
          expand_code_path/0,
-         vcs_vsn/3,
+         vcs_vsn/2,
          deprecated/3,
          deprecated/4,
          erl_opts/1,
@@ -179,19 +179,6 @@ expand_code_path() ->
                            end, [], code:get_path()),
     code:set_path(lists:reverse(CodePath)).
 
-vcs_vsn(Config, Vsn, Dir) ->
-    Key = {Vsn, Dir},
-    Cache = rebar_state:get(Config, vsn_cache, dict:new()),
-    case dict:find(Key, Cache) of
-        error ->
-            VsnString = vcs_vsn_1(Vsn, Dir),
-            Cache1 = dict:store(Key, VsnString, Cache),
-            Config1 = rebar_state:set(Config, vsn_cache, Cache1),
-            {Config1, VsnString};
-        {ok, VsnString} ->
-            {Config, VsnString}
-    end.
-
 deprecated(Old, New, Opts, When) when is_list(Opts) ->
     case lists:member(Old, Opts) of
         true ->
@@ -413,16 +400,16 @@ escript_foldl(Fun, Acc, File) ->
             Error
     end.
 
-vcs_vsn_1(Vcs, Dir) ->
+vcs_vsn(Vcs, Dir) ->
     case vcs_vsn_cmd(Vcs, Dir) of
         {plain, VsnString} ->
             VsnString;
         {cmd, CmdString} ->
             vcs_vsn_invoke(CmdString, Dir);
         unknown ->
-            ?ABORT("vcs_vsn: Unknown vsn format: ~p\n", [Vcs]);
+            ?ABORT("vcs_vsn: Unknown vsn format: ~p", [Vcs]);
         {error, Reason} ->
-            ?ABORT("vcs_vsn: ~s\n", [Reason])
+            ?ABORT("vcs_vsn: ~s", [Reason])
     end.
 
 %% Temp work around for repos like relx that use "semver"