remove unused code and improve resource_v2 specs

bootstrap

@@ -36,14 +36,6 @@ main(_) ->
 
     setup_env(),
     os:putenv("REBAR_PROFILE", "bootstrap"),
-    RegistryFile = default_registry_file(),
-    case filelib:is_file(RegistryFile) of
-        true ->
-            ok;
-        false ->
-            rebar3:run(["update"])
-    end,
-
     {ok, State} = rebar3:run(["compile"]),
     reset_env(),
     os:putenv("REBAR_PROFILE", ""),
@@ -57,11 +49,6 @@ main(_) ->
     %% Done with compile, can turn back on error logger
     error_logger:tty(true).
 
-default_registry_file() ->
-    {ok, [[Home]]} = init:get_argument(home),
-    CacheDir = filename:join([Home, ".cache", "rebar3"]),
-    filename:join([CacheDir, "hex", "default", "registry"]).
-
 fetch_and_compile({Name, ErlFirstFiles}, Deps) ->
     case lists:keyfind(Name, 1, Deps) of
         {Name, Vsn} ->

src/rebar_app_info.erl

@@ -44,8 +44,6 @@
          get/2,
          get/3,
          set/3,
-         resource_type/1,
-         resource_type/2,
          source/1,
          source/2,
          is_lock/1,
@@ -86,7 +84,6 @@
                      dep_level=0        :: integer(),
                      dir                :: file:name(),
                      out_dir            :: file:name(),
-                     resource_type      :: pkg | src | undefined,
                      source             :: string() | tuple() | checkout | undefined,
                      is_lock=false      :: boolean(),
                      is_checkout=false  :: boolean(),
@@ -154,8 +151,8 @@ new(Parent, AppName, Vsn, Dir, Deps) ->
 %% file for the app
 -spec update_opts(t(), rebar_dict(), [any()]) -> t().
 update_opts(AppInfo, Opts, Config) ->
-    LockDeps = case resource_type(AppInfo) of
-                   pkg ->
+    LockDeps = case source(AppInfo) of
+                   Tuple when is_tuple(Tuple) andalso element(1, Tuple) =:= pkg ->
                        %% Deps are set separate for packages
                        %% instead of making it seem we have no deps
                        %% don't set anything here.
@@ -454,16 +451,6 @@ ebin_dir(#app_info_t{out_dir=OutDir}) ->
 priv_dir(#app_info_t{out_dir=OutDir}) ->
     rebar_utils:to_list(filename:join(OutDir, "priv")).
 
-%% @doc returns whether the app is source app or a package app.
--spec resource_type(t()) -> pkg | src.
-resource_type(#app_info_t{resource_type=ResourceType}) ->
-    ResourceType.
-
-%% @doc sets whether the app is source app or a package app.
--spec resource_type(t(), pkg | src) -> t().
-resource_type(AppInfo=#app_info_t{}, Type) ->
-    AppInfo#app_info_t{resource_type=Type}.
-
 %% @doc finds the source specification for the app
 -spec source(t()) -> string() | tuple().
 source(#app_info_t{source=Source}) ->

src/rebar_app_utils.erl

@@ -252,16 +252,12 @@ update_source(AppInfo, {pkg, PkgName, PkgVsn, Hash}, State) ->
     case rebar_packages:resolve_version(PkgName, PkgVsn, Hash,
                                         ?PACKAGE_TABLE, State) of
         {ok, Package, RepoConfig} ->
-            #package{key = {_, PkgVsn1, _},
-                     checksum = Hash1,
-                     dependencies = Deps} = Package,
+            #package{key={_, PkgVsn1, _},
+                     checksum=Hash1,
+                     dependencies=Deps} = Package,
             AppInfo1 = rebar_app_info:source(AppInfo, {pkg, PkgName, PkgVsn1, Hash1, RepoConfig}),
-
-            %% TODO: Remove?
-            AppInfo2 = rebar_app_info:resource_type(AppInfo1, pkg),
-
-            AppInfo3 = rebar_app_info:update_opts_deps(AppInfo2, Deps),
-            rebar_app_info:original_vsn(AppInfo3, PkgVsn1);
+            AppInfo2 = rebar_app_info:update_opts_deps(AppInfo1, Deps),
+            rebar_app_info:original_vsn(AppInfo2, PkgVsn1);
         not_found ->
             throw(?PRV_ERROR({missing_package, PkgName, PkgVsn}));
         {error, {invalid_vsn, InvalidVsn}} ->

src/rebar_fetch.erl

@@ -26,7 +26,7 @@ lock_source(AppInfo, State)      ->
 download_source(AppInfo, State)  ->
     AppDir = rebar_app_info:dir(AppInfo),
     try download_source_(AppInfo, State) of
-        true ->
+        ok ->
             %% freshly downloaded, update the app info opts to reflect the new config
             Config = rebar_config:consult(AppDir),
             AppInfo1 = rebar_app_info:update_opts(AppInfo, rebar_app_info:opts(AppInfo), Config),
@@ -36,8 +36,8 @@ download_source(AppInfo, State)  ->
                 false ->
                     throw(?PRV_ERROR({dep_app_not_found, rebar_app_info:name(AppInfo1)}))
             end;
-        Error ->
-            throw(?PRV_ERROR(Error))
+        {error, Reason} ->
+            throw(?PRV_ERROR(Reason))
     catch
         throw:{no_resource, Type, Location} ->
             throw(?PRV_ERROR({no_resource, Location, Type}));
@@ -51,13 +51,12 @@ download_source_(AppInfo, State) ->
     TmpDir = ec_file:insecure_mkdtemp(),
     AppDir1 = rebar_utils:to_list(AppDir),
     case rebar_resource_v2:download(TmpDir, AppInfo, State) of
-        {ok, _} ->
+        ok ->
             ec_file:mkdir_p(AppDir1),
             code:del_path(filename:absname(filename:join(AppDir1, "ebin"))),
             ok = rebar_file_utils:rm_rf(filename:absname(AppDir1)),
             ?DEBUG("Moving checkout ~p to ~p", [TmpDir, filename:absname(AppDir1)]),
-            ok = rebar_file_utils:mv(TmpDir, filename:absname(AppDir1)),
-            true;
+            rebar_file_utils:mv(TmpDir, filename:absname(AppDir1));
         Error ->
             Error
     end.

src/rebar_git_resource.erl

@@ -111,7 +111,12 @@ parse_git_url(not_scp, Url) ->
     end.
 
 download(TmpDir, AppInfo, State, _) ->
-    download_(TmpDir, rebar_app_info:source(AppInfo), State).
+    case download_(TmpDir, rebar_app_info:source(AppInfo), State) of
+        {ok, _} ->
+            ok;
+        Error ->
+            {error, Error}
+    end.
 
 download_(Dir, {git, Url}, State) ->
     ?WARN("WARNING: It is recommended to use {branch, Name}, {tag, Tag} or {ref, Ref}, otherwise updating the dep may not work as expected.", []),

src/rebar_hg_resource.erl

@@ -61,7 +61,12 @@ needs_update_(Dir, {hg, Url, Ref}) ->
     not ((LocalRef =:= TargetRef) andalso compare_url(Dir, Url)).
 
 download(TmpDir, AppInfo, State, _) ->
-    download_(TmpDir, rebar_app_info:source(AppInfo), State).
+    case download_(TmpDir, rebar_app_info:source(AppInfo), State) of
+        {ok, _} ->
+            ok;
+        Error ->
+            {error, Error}
+    end.
 
 download_(Dir, {hg, Url}, State) ->
     ?WARN("WARNING: It is recommended to use {branch, Name}, {tag, Tag} or {ref, Ref}, otherwise updating the dep may not work as expected.", []),

src/rebar_pkg_resource.erl

@@ -18,14 +18,11 @@
 
 -include("rebar.hrl").
 
--type cached_result()   :: {'bad_checksum',string()} |
-                           {'bad_registry_checksum',string()} |
-                           {'failed_extract',string()} |
-                           {'ok','true'} |
-                           {'unexpected_hash',string(),_,binary()}.
-
--type download_result() :: {bad_download, binary() | string()} |
-                           {fetch_fail, _, _} | cached_result().
+-type cached_result()   :: ok |
+                           {bad_checksum,string()} |
+                           {bad_registry_checksum,string()} |
+                           {failed_extract,string()} |
+                           {unexpected_hash,string(),_,binary()}.
 
 %%==============================================================================
 %% Public API
@@ -82,9 +79,14 @@ needs_update(AppInfo, _) ->
       AppInfo :: rebar_app_info:t(),
       ResourceState :: rebar_resource_v2:resource_state(),
       State :: rebar_state:t(),
-      Res :: {'error',_} | {'ok',_} | {'tarball',binary() | string()}.
+      Res :: ok | {error,_}.
 download(TmpDir, AppInfo, State, ResourceState) ->
-    download(TmpDir, rebar_app_info:source(AppInfo), State, ResourceState, true).
+    case download(TmpDir, rebar_app_info:source(AppInfo), State, ResourceState, true) of
+        ok ->
+            ok;
+        Error ->
+            {error, Error}
+    end.
 
 %%------------------------------------------------------------------------------
 %% @doc
@@ -99,7 +101,7 @@ download(TmpDir, AppInfo, State, ResourceState) ->
       State :: rebar_state:t(),
       ResourceState:: rebar_resource_v2:resource_state(),
       UpdateETag :: boolean(),
-      Res :: download_result().
+      Res :: ok | {error,_}.
 download(TmpDir, Pkg={pkg, Name, Vsn, _Hash, Repo}, State, _ResourceState, UpdateETag) ->
     {ok, PackageDir} = rebar_packages:package_dir(Repo, State),
     Package = binary_to_list(<<Name/binary, "-", Vsn/binary, ".tar">>),
@@ -193,7 +195,7 @@ store_etag_in_cache(Path, ETag) ->
       State :: rebar_state:t(),
       ETagPath :: file:name(),
       UpdateETag :: boolean(),
-      Res :: download_result().
+      Res :: cached_result() | {fetch_fail, binary(), binary()} | {bad_download, file:name()}.
 cached_download(TmpDir, CachePath, Pkg={pkg, Name, Vsn, _Hash, RepoConfig}, ETag,
                 State, ETagPath, UpdateETag) ->
     case request(RepoConfig, Name, Vsn, ETag) of
@@ -223,8 +225,7 @@ serve_from_cache(TmpDir, CachePath, Pkg, State) ->
     {Files, Contents, Version, Meta} = extract(TmpDir, CachePath),
     case checksums(Pkg, Files, Contents, Version, Meta, State) of
         {Chk, Chk, Chk} ->
-            ok = erl_tar:extract({binary, Contents}, [{cwd, TmpDir}, compressed]),
-            {ok, true};
+            erl_tar:extract({binary, Contents}, [{cwd, TmpDir}, compressed]);
         {_Hash, Chk, Chk} ->
             ?DEBUG("Expected hash ~p does not match checksums ~p", [_Hash, Chk]),
             {unexpected_hash, CachePath, _Hash, Chk};
@@ -246,7 +247,7 @@ serve_from_cache(TmpDir, CachePath, Pkg, State) ->
       Binary :: binary(),
       State :: rebar_state:t(),
       ETagPath :: file:name(),
-      Res :: download_result().
+      Res :: ok | {error,_} | {bad_download, file:name()}.
 serve_from_download(TmpDir, CachePath, Package, ETag, Binary, State, ETagPath) ->
     ?DEBUG("Writing ~p to cache at ~ts", [Package, CachePath]),
     file:write_file(CachePath, Binary),

src/rebar_resource.erl

@@ -40,7 +40,12 @@ lock(Module, AppInfo) ->
     Module:lock(rebar_app_info:dir(AppInfo), rebar_app_info:source(AppInfo)).
 
 download(Module, TmpDir, AppInfo, State) ->
-    Module:download(TmpDir, rebar_app_info:source(AppInfo), State).
+    case Module:download(TmpDir, rebar_app_info:source(AppInfo), State) of
+        {ok, _} ->
+            ok;
+        Error ->
+            Error
+    end.
 
 needs_update(Module, AppInfo) ->
     Module:needs_update(rebar_app_info:dir(AppInfo), rebar_app_info:source(AppInfo)).

src/rebar_resource_v2.erl

@@ -31,7 +31,7 @@
 -callback init(type(), rebar_state:t()) -> {ok, resource()}.
 -callback lock(rebar_app_info:t(), resource_state()) -> source().
 -callback download(file:filename_all(), rebar_app_info:t(), resource_state(), rebar_state:t()) ->
-    {tarball, file:filename_all()} | {ok, any()} | {error, any()}.
+    {ok, rebar_app_info:t()} | {error, any()}.
 -callback needs_update(rebar_app_info:t(), resource_state()) -> boolean().
 -callback make_vsn(rebar_app_info:t(), resource_state()) ->
     {plain, string()} | {error, string()}.

test/mock_git_resource.erl

@@ -121,7 +121,7 @@ mock_download(Opts, CreateType) ->
                 [kernel, stdlib] ++ [element(1,D) || D  <- AppDeps]
             ),
             rebar_test_utils:create_config(Dir, [{deps, AppDeps}]++Config),
-            {ok, 'WHATEVER'}
+            ok
         end).
 
 %%%%%%%%%%%%%%%

test/mock_pkg_resource.erl

@@ -107,7 +107,7 @@ mock_download(Opts) ->
             Cached = filename:join([Cache, TarApp]),
             filelib:ensure_dir(Cached),
             rebar_file_utils:mv(Tarball, Cached),
-            {ok, true}
+            ok
         end).
 
 %% @doc On top of the pkg resource mocking, we need to mock the package

test/rebar_pkg_SUITE.erl

@@ -117,7 +117,7 @@ good_uncached(Config) ->
     Tmp = ?config(tmp_dir, Config),
     {Pkg,Vsn} = ?config(pkg, Config),
     State = ?config(state, Config),
-    ?assertEqual({ok, true},
+    ?assertEqual(ok,
                  rebar_pkg_resource:download(Tmp, {pkg, Pkg, Vsn, ?good_checksum, #{}}, State, #{}, true)),
     Cache = ?config(cache_dir, Config),
     ?assert(filelib:is_regular(filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".tar">>))).
@@ -130,7 +130,7 @@ good_cached(Config) ->
     CachedFile = filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".tar">>),
     ?assert(filelib:is_regular(CachedFile)),
     {ok, Content} = file:read_file(CachedFile),
-    ?assertEqual({ok, true},
+    ?assertEqual(ok,
                  rebar_pkg_resource:download(Tmp, {pkg, Pkg, Vsn, ?good_checksum, #{}}, State, #{}, true)),
     {ok, Content} = file:read_file(CachedFile).
 
@@ -156,7 +156,7 @@ bad_to_good(Config) ->
     CachedFile = filename:join(Cache, <<Pkg/binary, "-", Vsn/binary, ".tar">>),
     ?assert(filelib:is_regular(CachedFile)),
     {ok, Contents} = file:read_file(CachedFile),
-    ?assertEqual({ok, true},
+    ?assertEqual(ok,
                  rebar_pkg_resource:download(Tmp, {pkg, Pkg, Vsn, ?good_checksum, #{}}, State, #{}, true)),
     %% Cache has refreshed
     ?assert({ok, Contents} =/= file:read_file(CachedFile)).
@@ -171,7 +171,7 @@ good_disconnect(Config) ->
     ?assert(filelib:is_regular(CachedFile)),
     {ok, Content} = file:read_file(CachedFile),
     rebar_pkg_resource:store_etag_in_cache(ETagFile, ?BADPKG_ETAG),
-    ?assertEqual({ok, true},
+    ?assertEqual(ok,
                  rebar_pkg_resource:download(Tmp, {pkg, Pkg, Vsn, ?good_checksum, #{}}, State, #{}, true)),
     {ok, Content} = file:read_file(CachedFile).