From 8fbc8d7a1abde3efa352c8c29f1cb91fef368620 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Wed, 12 Sep 2018 11:22:21 -0600 Subject: [PATCH] remove unused code and improve resource_v2 specs --- bootstrap | 13 ------------- src/rebar_app_info.erl | 17 ++--------------- src/rebar_app_utils.erl | 14 +++++--------- src/rebar_fetch.erl | 11 +++++------ src/rebar_git_resource.erl | 7 ++++++- src/rebar_hg_resource.erl | 7 ++++++- src/rebar_pkg_resource.erl | 31 ++++++++++++++++--------------- src/rebar_resource.erl | 7 ++++++- src/rebar_resource_v2.erl | 2 +- test/mock_git_resource.erl | 2 +- test/mock_pkg_resource.erl | 2 +- test/rebar_pkg_SUITE.erl | 8 ++++---- 12 files changed, 53 insertions(+), 68 deletions(-) diff --git a/bootstrap b/bootstrap index fc6da254..3a7839b0 100755 --- a/bootstrap +++ b/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} -> diff --git a/src/rebar_app_info.erl b/src/rebar_app_info.erl index 162d8e78..56ae4c0d 100644 --- a/src/rebar_app_info.erl +++ b/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}) -> diff --git a/src/rebar_app_utils.erl b/src/rebar_app_utils.erl index 4851de42..35e908ca 100644 --- a/src/rebar_app_utils.erl +++ b/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}} -> diff --git a/src/rebar_fetch.erl b/src/rebar_fetch.erl index f04a6215..4a60864f 100644 --- a/src/rebar_fetch.erl +++ b/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. diff --git a/src/rebar_git_resource.erl b/src/rebar_git_resource.erl index b378ee8e..b835b923 100644 --- a/src/rebar_git_resource.erl +++ b/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.", []), diff --git a/src/rebar_hg_resource.erl b/src/rebar_hg_resource.erl index c61e7d3f..eb35ce88 100644 --- a/src/rebar_hg_resource.erl +++ b/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.", []), diff --git a/src/rebar_pkg_resource.erl b/src/rebar_pkg_resource.erl index 5d7f4856..0956639d 100644 --- a/src/rebar_pkg_resource.erl +++ b/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(<>), @@ -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), diff --git a/src/rebar_resource.erl b/src/rebar_resource.erl index 0954df76..a3a8edb0 100644 --- a/src/rebar_resource.erl +++ b/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)). diff --git a/src/rebar_resource_v2.erl b/src/rebar_resource_v2.erl index ef897872..0563cce5 100644 --- a/src/rebar_resource_v2.erl +++ b/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()}. diff --git a/test/mock_git_resource.erl b/test/mock_git_resource.erl index fd6db573..56733492 100644 --- a/test/mock_git_resource.erl +++ b/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). %%%%%%%%%%%%%%% diff --git a/test/mock_pkg_resource.erl b/test/mock_pkg_resource.erl index 7ec7441b..4b23497c 100644 --- a/test/mock_pkg_resource.erl +++ b/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 diff --git a/test/rebar_pkg_SUITE.erl b/test/rebar_pkg_SUITE.erl index 650117b7..1085f5fc 100644 --- a/test/rebar_pkg_SUITE.erl +++ b/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, <>))). @@ -130,7 +130,7 @@ good_cached(Config) -> CachedFile = filename:join(Cache, <>), ?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, <>), ?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).