From b94afdfa2d44b0cb1489234c7746cd72f85a167d Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Tue, 20 Jan 2015 01:39:43 +0000 Subject: WIP Test that all the correct locks are set for an upgrade run. Now to actually re-run the install deps and prove it works --- src/rebar_prv_install_deps.erl | 2 +- src/rebar_prv_lock.erl | 2 +- src/rebar_prv_upgrade.erl | 48 +++++++---- src/rebar_state.erl | 8 +- test/rebar_deps_SUITE.erl | 1 - test/rebar_test_utils.erl | 12 ++- test/rebar_upgrade_SUITE.erl | 185 +++++++++++++++++++++++++++++++++++++++++ 7 files changed, 238 insertions(+), 20 deletions(-) create mode 100644 test/rebar_upgrade_SUITE.erl diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index 025d32a..49aa65c 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -287,7 +287,7 @@ update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen, handle_update(AppInfo, UpdateName, UpdateLevel, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> Name = rebar_app_info:name(AppInfo), - {_, _, _, DepLevel} = lists:keyfind(Name, 1, Locks), + {_, _, DepLevel} = lists:keyfind(Name, 1, Locks), case UpdateLevel < DepLevel orelse Name =:= UpdateName of true -> diff --git a/src/rebar_prv_lock.erl b/src/rebar_prv_lock.erl index 2f26a7f..8a69434 100644 --- a/src/rebar_prv_lock.erl +++ b/src/rebar_prv_lock.erl @@ -41,7 +41,7 @@ do(State) -> ,rebar_app_info:dep_level(Dep)} end, AllDeps), Dir = rebar_state:dir(State), - file:write_file(filename:join(Dir, "rebar.lock"), io_lib:format("~p.~n", [Locks])), + file:write_file(filename:join(Dir, ?LOCK_FILE), io_lib:format("~p.~n", [Locks])), {ok, State}. -spec format_error(any()) -> iolist(). diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl index c0a56c3..4609f57 100644 --- a/src/rebar_prv_upgrade.erl +++ b/src/rebar_prv_upgrade.erl @@ -12,7 +12,10 @@ -include("rebar.hrl"). -define(PROVIDER, upgrade). --define(DEPS, []). +-define(DEPS, [lock]). +%% Also only upgrade top-level (0) deps. Transitive deps shouldn't be +%% upgradable -- if the user wants this, they should declare it at the +%% top level and then upgrade. %% =================================================================== %% Public API @@ -38,19 +41,36 @@ init(State) -> do(State) -> {Args, _} = rebar_state:command_parsed_args(State), Name = ec_cnv:to_binary(proplists:get_value(package, Args)), - Locks = rebar_state:get(State, locks, []), - case lists:keyfind(Name, 1, Locks) of - {_, _, _, Level} -> - Deps = rebar_state:get(State, deps), - case lists:keyfind(binary_to_atom(Name, utf8), 1, Deps) of - false -> - {error, io_lib:format("No such dependency ~s~n", [Name])}; - Dep -> - rebar_prv_install_deps:handle_deps(State, [Dep], {true, Name, Level}), - {ok, State} - end; - _ -> - {error, io_lib:format("No such dependency ~s~n", [Name])} + Locks = rebar_state:lock(State), + %% TODO: optimize by running the find + unlock in one sweep + case find_app(Name, Locks) of + {AppInfo, 0} -> + %% Unlock the app and all those with a lock level higher than + %% it has + NewLocks = unlock_higher_than(0, Locks -- [AppInfo]), + {ok, rebar_state:lock(State, NewLocks)}; + {_AppInfo, Level} when Level > 0 -> + {error, transitive_dependency}; + false -> + {error, unknown_dependency} + end. + +find_app(_Name, []) -> false; +find_app(Name, [App|Apps]) -> + case rebar_app_info:name(App) of + Name -> {App, rebar_app_info:dep_level(App)}; + _ -> find_app(Name, Apps) + end. + +%% Because we operate on a lock list, removing the app from the list +%% unlocks it. +unlock_higher_than(_, []) -> []; +unlock_higher_than(Level, [App | Apps]) -> + case rebar_app_info:dep_level(App) of + N when N > Level -> + unlock_higher_than(Level, Apps); + N when N =< Level -> + [App | unlock_higher_than(Level, Apps)] end. -spec format_error(any()) -> iolist(). diff --git a/src/rebar_state.erl b/src/rebar_state.erl index 29b7c3f..70aba51 100644 --- a/src/rebar_state.erl +++ b/src/rebar_state.erl @@ -89,8 +89,10 @@ new(ParentState, Config, Dir) -> Opts = ParentState#state_t.opts, LocalOpts = case rebar_config:consult_file(filename:join(Dir, ?LOCK_FILE)) of [D] -> - LockedDeps = [X || X <- D, element(3, X) =:= 0], - dict:from_list([{{locks, default}, LockedDeps}, {{deps, default}, D} | Config]); + %% We want the top level deps only from the lock file. + %% This ensures deterministic overrides for configs. + Deps = [X || X <- D, element(3, X) =:= 0], + dict:from_list([{{locks, default}, D}, {{deps, default}, Deps} | Config]); _ -> D = proplists:get_value(deps, Config, []), dict:from_list([{{deps, default}, D} | Config]) @@ -133,6 +135,8 @@ current_profiles(#state_t{current_profiles=Profiles}) -> lock(#state_t{lock=Lock}) -> Lock. +lock(State=#state_t{}, Apps) when is_list(Apps) -> + State#state_t{lock=Apps}; lock(State=#state_t{lock=Lock}, App) -> State#state_t{lock=[App | Lock]}. diff --git a/test/rebar_deps_SUITE.erl b/test/rebar_deps_SUITE.erl index d67efe4..8407b55 100644 --- a/test/rebar_deps_SUITE.erl +++ b/test/rebar_deps_SUITE.erl @@ -1,4 +1,3 @@ -%%% TODO: check that warnings are appearing -module(rebar_deps_SUITE). -compile(export_all). -include_lib("common_test/include/ct.hrl"). diff --git a/test/rebar_test_utils.erl b/test/rebar_test_utils.erl index 6095d6d..af40995 100644 --- a/test/rebar_test_utils.erl +++ b/test/rebar_test_utils.erl @@ -53,7 +53,17 @@ run_and_check(Config, RebarConfig, Command, Expect) -> ?assertEqual({error, Reason}, Res); {ok, Expected} -> {ok, _} = Res, - check_results(AppDir, Expected) + check_results(AppDir, Expected); + {unlocked, Expected} -> + ct:pal("Expected: ~p", [Expected]), + {ok, ResState} = Res, + Locks = rebar_state:lock(ResState), + ct:pal("Locks: ~p", [Locks]), + ?assertEqual(lists:sort(Expected), + lists:sort([rebar_app_info:name(Lock) + || Lock <- Locks])); + return -> + Res end. %% @doc Creates a dummy application including: diff --git a/test/rebar_upgrade_SUITE.erl b/test/rebar_upgrade_SUITE.erl new file mode 100644 index 0000000..5915d0d --- /dev/null +++ b/test/rebar_upgrade_SUITE.erl @@ -0,0 +1,185 @@ +-module(rebar_upgrade_SUITE). +-include_lib("common_test/include/ct.hrl"). +-include_lib("eunit/include/eunit.hrl"). +-compile(export_all). + +all() -> [{group, git}].%, {group, pkg}]. + +groups() -> + [{all, [], [top, pair, triplet, tree]}, + {git, [], [{group, all}]}, + {pkg, [], [{group, all}]}]. + +init_per_suite(Config) -> + application:start(meck), + Config. + +end_per_suite(_Config) -> + application:stop(meck). + +init_per_group(git, Config) -> + [{deps_type, git} | Config]; +init_per_group(pkg, Config) -> + [{deps_type, pkg} | Config]; +init_per_group(_, Config) -> + Config. + +end_per_group(_, Config) -> + Config. + +init_per_testcase(Case, Config) -> + DepsType = ?config(deps_type, Config), + {Deps, Unlocks} = upgrades(Case), + Expanded = expand_deps(DepsType, Deps), + [{unlocks, normalize_unlocks(Unlocks)}, + {mock, fun() -> mock_deps(DepsType, Expanded, []) end} + | setup_project(Case, Config, Expanded)]. + +end_per_testcase(_, Config) -> + meck:unload(), + Config. + +setup_project(Case, Config0, Deps) -> + DepsType = ?config(deps_type, Config0), + Config = rebar_test_utils:init_rebar_state( + Config0, + atom_to_list(Case)++"_"++atom_to_list(DepsType)++"_" + ), + AppDir = ?config(apps, Config), + rebar_test_utils:create_app(AppDir, "Root", "0.0.0", [kernel, stdlib]), + TopDeps = top_level_deps(Deps), + RebarConf = rebar_test_utils:create_config(AppDir, [{deps, TopDeps}]), + [{rebarconfig, RebarConf} | Config]. + + +upgrades(top) -> + {[{"A", [{"B", [{"D", "1", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% upgrade vs. locked after upgrade + [{"A", []}, + {"B", {error, transitive_dependency}}, + {"C", {error, transitive_dependency}}, + {"D", "1", {error, transitive_dependency}}, + {"D", "2", {error, unknown_dependency}}]}; +upgrades(pair) -> + {[{"A", [{"C", []}]}, + {"B", [{"D", []}]}], + [{"A", ["B"]}, + {"B", ["A"]}, + {"C", {error, transitive_dependency}}, + {"D", {error, transitive_dependency}}]}; +upgrades(triplet) -> + {[{"A", [{"D",[]}, + {"E",[]}]}, + {"B", [{"F",[]}, + {"G",[]}]}, + {"C", [{"H",[]}, + {"I",[]}]}], + [{"A", ["B","C"]}, + {"B", ["A","C"]}, + {"C", ["A","B"]}, + {"D", {error, transitive_dependency}}, + %% ... + {"I", {error, transitive_dependency}}]}; +upgrades(tree) -> + {[{"A", [{"D",[{"J",[]}]}, + {"E",[{"K",[]}]}]}, + {"B", [{"F",[]}, + {"G",[]}]}, + {"C", [{"H",[]}, + {"I",[]}]}], + [{"A", ["B","C"]}, + {"B", ["A","C"]}, + {"C", ["A","B"]}, + {"D", {error, transitive_dependency}}, + %% ... + {"K", {error, transitive_dependency}}]}. + +%% TODO: add a test that verifies that unlocking files and then +%% running the upgrade code is enough to properly upgrade things. + +top_level_deps([]) -> []; +top_level_deps([{{Name, Vsn, Ref}, _} | Deps]) -> + [{list_to_atom(Name), Vsn, Ref} | top_level_deps(Deps)]; +top_level_deps([{{pkg, Name, Vsn, _URL}, _} | Deps]) -> + [{list_to_atom(Name), Vsn} | top_level_deps(Deps)]. + +mock_deps(git, Deps, Upgrades) -> + mock_git_resource:mock([{deps, flat_deps(Deps)}, {upgrade, Upgrades}]); +mock_deps(pkg, Deps, Upgrades) -> + mock_pkg_resource:mock([{pkgdeps, flat_pkgdeps(Deps)}, {upgrade, Upgrades}]). + +flat_deps([]) -> []; +flat_deps([{{Name,_Vsn,Ref}, Deps} | Rest]) -> + [{{Name,vsn_from_ref(Ref)}, top_level_deps(Deps)}] + ++ + flat_deps(Deps) + ++ + flat_deps(Rest). + +vsn_from_ref({git, _, {_, Vsn}}) -> Vsn; +vsn_from_ref({git, _, Vsn}) -> Vsn. + +flat_pkgdeps([]) -> []; +flat_pkgdeps([{{pkg, Name, Vsn, _Url}, Deps} | Rest]) -> + [{{iolist_to_binary(Name),iolist_to_binary(Vsn)}, top_level_deps(Deps)}] + ++ + flat_pkgdeps(Deps) + ++ + flat_pkgdeps(Rest). + +expand_deps(_, []) -> []; +expand_deps(git, [{Name, Deps} | Rest]) -> + Dep = {Name, ".*", {git, "https://example.org/user/"++Name++".git", "master"}}, + [{Dep, expand_deps(git, Deps)} | expand_deps(git, Rest)]; +expand_deps(git, [{Name, Vsn, Deps} | Rest]) -> + Dep = {Name, Vsn, {git, "https://example.org/user/"++Name++".git", {tag, Vsn}}}, + [{Dep, expand_deps(git, Deps)} | expand_deps(git, Rest)]; +expand_deps(pkg, [{Name, Deps} | Rest]) -> + Dep = {pkg, Name, "0.0.0", "https://example.org/user/"++Name++".tar.gz"}, + [{Dep, expand_deps(pkg, Deps)} | expand_deps(pkg, Rest)]; +expand_deps(pkg, [{Name, Vsn, Deps} | Rest]) -> + Dep = {pkg, Name, Vsn, "https://example.org/user/"++Name++".tar.gz"}, + [{Dep, expand_deps(pkg, Deps)} | expand_deps(pkg, Rest)]. + +normalize_unlocks([]) -> []; +normalize_unlocks([{App, Locks} | Rest]) -> + [{iolist_to_binary(App), + normalize_unlocks_expect(Locks)} | normalize_unlocks(Rest)]; +normalize_unlocks([{App, Vsn, Locks} | Rest]) -> + [{iolist_to_binary(App), iolist_to_binary(Vsn), + normalize_unlocks_expect(Locks)} | normalize_unlocks(Rest)]. + +normalize_unlocks_expect({error, Reason}) -> + {error, Reason}; +normalize_unlocks_expect([]) -> + []; +normalize_unlocks_expect([{App,Vsn} | Rest]) -> + [{iolist_to_binary(App), iolist_to_binary(Vsn)} + | normalize_unlocks_expect(Rest)]; +normalize_unlocks_expect([App | Rest]) -> + [iolist_to_binary(App) | normalize_unlocks_expect(Rest)]. + +top(Config) -> run(Config). +pair(Config) -> run(Config). +triplet(Config) -> run(Config). +tree(Config) -> run(Config). + +run(Config) -> + apply(?config(mock, Config), []), + {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)), + %% Install dependencies before re-mocking for an upgrade + rebar_test_utils:run_and_check(Config, RebarConfig, ["lock"], {ok, []}), + Unlocks = ?config(unlocks, Config), + [begin + ct:pal("Unlocks: ~p -> ~p", [App, Unlocked]), + Expectation = case Unlocked of + Tuple when is_tuple(Tuple) -> Tuple; + _ -> {unlocked, Unlocked} + end, + rebar_test_utils:run_and_check( + Config, RebarConfig, ["upgrade", App], Expectation + ) + end || {App, Unlocked} <- Unlocks]. + -- cgit v1.1 From 9fb35fe6defb07dde99ba4442a66e3e6719c4d49 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Tue, 3 Feb 2015 15:03:23 +0000 Subject: Alternative attempt --- src/rebar_prv_install_deps.erl | 20 +++++++++-------- src/rebar_prv_upgrade.erl | 50 +++++++++++++++++++++--------------------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index 49aa65c..094fae0 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -254,10 +254,9 @@ update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen, {SeenAcc1, StateAcc1} = maybe_lock(Profile, AppInfo, SeenAcc, StateAcc, Level), {SrcDepsAcc1, PkgDepsAcc1, SrcAppsAcc1, StateAcc2, LocksAcc1} = case Update of - {true, UpdateName, UpdateLevel} -> + true -> + %{true, UpdateName, UpdateLevel} -> handle_update(AppInfo - ,UpdateName - ,UpdateLevel ,SrcDepsAcc ,PkgDepsAcc ,SrcAppsAcc @@ -285,14 +284,15 @@ update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen, update_src_deps(Profile, Level+1, NewSrcDeps, NewPkgDeps, NewSrcApps, State1, Update, Seen1, NewLocks) end. -handle_update(AppInfo, UpdateName, UpdateLevel, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> +handle_update(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> Name = rebar_app_info:name(AppInfo), - {_, _, DepLevel} = lists:keyfind(Name, 1, Locks), - case UpdateLevel < DepLevel - orelse Name =:= UpdateName of - true -> + ct:pal("update ~p", [Name]), + case lists:keyfind(Name, 1, Locks) of + false -> + ct:pal("in lock"), case maybe_fetch(AppInfo, true, []) of true -> + ct:pal("fetch!"), handle_dep(AppInfo ,SrcDeps ,PkgDeps @@ -302,9 +302,11 @@ handle_update(AppInfo, UpdateName, UpdateLevel, SrcDeps, PkgDeps, SrcApps, Level ,Locks); false -> + ct:pal("nofetch"), {SrcDeps, PkgDeps, SrcApps, State, Locks} end; - false -> + _StillLocked -> + ct:pal("stillocked"), {SrcDeps, PkgDeps, SrcApps, State, Locks} end. diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl index 4609f57..207d36a 100644 --- a/src/rebar_prv_upgrade.erl +++ b/src/rebar_prv_upgrade.erl @@ -12,7 +12,7 @@ -include("rebar.hrl"). -define(PROVIDER, upgrade). --define(DEPS, [lock]). +-define(DEPS, []). %% Also only upgrade top-level (0) deps. Transitive deps shouldn't be %% upgradable -- if the user wants this, they should declare it at the %% top level and then upgrade. @@ -41,38 +41,38 @@ init(State) -> do(State) -> {Args, _} = rebar_state:command_parsed_args(State), Name = ec_cnv:to_binary(proplists:get_value(package, Args)), - Locks = rebar_state:lock(State), - %% TODO: optimize by running the find + unlock in one sweep - case find_app(Name, Locks) of - {AppInfo, 0} -> - %% Unlock the app and all those with a lock level higher than - %% it has - NewLocks = unlock_higher_than(0, Locks -- [AppInfo]), - {ok, rebar_state:lock(State, NewLocks)}; - {_AppInfo, Level} when Level > 0 -> + Locks = rebar_state:get(State, {locks, default}, []), + case lists:keyfind(Name, 1, Locks) of + {_, _, 0} = Lock -> + Deps = rebar_state:get(State, deps), + case lists:keyfind(binary_to_atom(Name, utf8), 1, Deps) of + false -> + {error, unknown_dependency}; + Dep -> + Source = case Dep of + {_, Src} -> Src; + {_, _, Src} -> Src + end, + NewLocks = unlock_higher_than(0, Locks -- [Lock]), + State1 = rebar_state:set(State, {deps, default}, [{Name, Source, 0} | NewLocks]), + State2 = rebar_state:set(State1, {locks, default}, NewLocks), + rebar_prv_install_deps:do(State2) + end; + {_, _, Level} when Level > 0 -> {error, transitive_dependency}; false -> - {error, unknown_dependency} + ct:pal("deps: ~p", [{Name,Locks}]), + {error, unlocked_dependency} end. -find_app(_Name, []) -> false; -find_app(Name, [App|Apps]) -> - case rebar_app_info:name(App) of - Name -> {App, rebar_app_info:dep_level(App)}; - _ -> find_app(Name, Apps) - end. -%% Because we operate on a lock list, removing the app from the list -%% unlocks it. unlock_higher_than(_, []) -> []; -unlock_higher_than(Level, [App | Apps]) -> - case rebar_app_info:dep_level(App) of - N when N > Level -> - unlock_higher_than(Level, Apps); - N when N =< Level -> - [App | unlock_higher_than(Level, Apps)] +unlock_higher_than(Level, [App = {_,_,AppLevel} | Apps]) -> + if AppLevel > Level -> unlock_higher_than(Level, Apps); + AppLevel =< Level -> [App | unlock_higher_than(Level, Apps)] end. -spec format_error(any()) -> iolist(). format_error(Reason) -> io_lib:format("~p", [Reason]). + -- cgit v1.1 From 748a046133fa5b0b2570f4675c223326299d711a Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Wed, 4 Feb 2015 21:26:48 +0000 Subject: Partial work + Failing tests The problem with the current effort is handling of transitive dependency upgrades and possible values. --- src/rebar_prv_upgrade.erl | 2 +- test/mock_git_resource.erl | 5 +- test/rebar_test_utils.erl | 8 -- test/rebar_upgrade_SUITE.erl | 328 +++++++++++++++++++++++++++++++++---------- 4 files changed, 262 insertions(+), 81 deletions(-) diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl index 207d36a..0c1ed8a 100644 --- a/src/rebar_prv_upgrade.erl +++ b/src/rebar_prv_upgrade.erl @@ -62,7 +62,7 @@ do(State) -> {error, transitive_dependency}; false -> ct:pal("deps: ~p", [{Name,Locks}]), - {error, unlocked_dependency} + {error, unknown_dependency} end. diff --git a/test/mock_git_resource.erl b/test/mock_git_resource.erl index 00f0a03..5d8288e 100644 --- a/test/mock_git_resource.erl +++ b/test/mock_git_resource.erl @@ -54,11 +54,13 @@ mock_lock(_) -> %% @doc The config passed to the `mock/2' function can specify which apps %% should be updated on a per-name basis: `{update, ["App1", "App3"]}'. mock_update(Opts) -> - ToUpdate = proplists:get_value(update, Opts, []), + ToUpdate = proplists:get_value(upgrade, Opts, []), + ct:pal("TOUp: ~p", [ToUpdate]), meck:expect( ?MOD, needs_update, fun(_Dir, {git, Url, _Ref}) -> App = app(Url), + ct:pal("Needed update? ~p -> ~p", [App, lists:member(App, ToUpdate)]), lists:member(App, ToUpdate) end). @@ -106,6 +108,7 @@ mock_download(Opts) -> {git, Url, {_, Vsn}} = normalize_git(Git, Overrides, Default), App = app(Url), AppDeps = proplists:get_value({App,Vsn}, Deps, []), + ct:pal("creating app ~p", [{Dir, App, Vsn, AppDeps}]), rebar_test_utils:create_app( Dir, App, Vsn, [element(1,D) || D <- AppDeps] diff --git a/test/rebar_test_utils.erl b/test/rebar_test_utils.erl index af40995..827d134 100644 --- a/test/rebar_test_utils.erl +++ b/test/rebar_test_utils.erl @@ -54,14 +54,6 @@ run_and_check(Config, RebarConfig, Command, Expect) -> {ok, Expected} -> {ok, _} = Res, check_results(AppDir, Expected); - {unlocked, Expected} -> - ct:pal("Expected: ~p", [Expected]), - {ok, ResState} = Res, - Locks = rebar_state:lock(ResState), - ct:pal("Locks: ~p", [Locks]), - ?assertEqual(lists:sort(Expected), - lists:sort([rebar_app_info:name(Lock) - || Lock <- Locks])); return -> Res end. diff --git a/test/rebar_upgrade_SUITE.erl b/test/rebar_upgrade_SUITE.erl index 5915d0d..a7d4a06 100644 --- a/test/rebar_upgrade_SUITE.erl +++ b/test/rebar_upgrade_SUITE.erl @@ -6,7 +6,10 @@ all() -> [{group, git}].%, {group, pkg}]. groups() -> - [{all, [], [top, pair, triplet, tree]}, + [{all, [], [top_a, top_b, top_c, top_d1, top_d2, top_e, + pair_a, pair_b, pair_c, + triplet_a, triplet_b, triplet_c, + tree_a, tree_b, tree_c]}, {git, [], [{group, all}]}, {pkg, [], [{group, all}]}]. @@ -29,10 +32,12 @@ end_per_group(_, Config) -> init_per_testcase(Case, Config) -> DepsType = ?config(deps_type, Config), - {Deps, Unlocks} = upgrades(Case), + {Deps, UpDeps, ToUp, Expectations} = upgrades(Case), Expanded = expand_deps(DepsType, Deps), - [{unlocks, normalize_unlocks(Unlocks)}, - {mock, fun() -> mock_deps(DepsType, Expanded, []) end} + UpExpanded = expand_deps(DepsType, UpDeps), + [{expected, normalize_unlocks(Expectations)}, + {mock, fun() -> mock_deps(DepsType, Expanded, []) end}, + {mock_update, fun() -> mock_deps(DepsType, UpExpanded, ToUp) end} | setup_project(Case, Config, Expanded)]. end_per_testcase(_, Config) -> @@ -52,49 +57,216 @@ setup_project(Case, Config0, Deps) -> [{rebarconfig, RebarConf} | Config]. -upgrades(top) -> - {[{"A", [{"B", [{"D", "1", []}]}, - {"C", [{"D", "2", []}]}]} - ], - %% upgrade vs. locked after upgrade - [{"A", []}, - {"B", {error, transitive_dependency}}, - {"C", {error, transitive_dependency}}, - {"D", "1", {error, transitive_dependency}}, - {"D", "2", {error, unknown_dependency}}]}; -upgrades(pair) -> - {[{"A", [{"C", []}]}, - {"B", [{"D", []}]}], - [{"A", ["B"]}, - {"B", ["A"]}, - {"C", {error, transitive_dependency}}, - {"D", {error, transitive_dependency}}]}; -upgrades(triplet) -> - {[{"A", [{"D",[]}, - {"E",[]}]}, - {"B", [{"F",[]}, - {"G",[]}]}, - {"C", [{"H",[]}, - {"I",[]}]}], - [{"A", ["B","C"]}, - {"B", ["A","C"]}, - {"C", ["A","B"]}, - {"D", {error, transitive_dependency}}, - %% ... - {"I", {error, transitive_dependency}}]}; -upgrades(tree) -> - {[{"A", [{"D",[{"J",[]}]}, - {"E",[{"K",[]}]}]}, - {"B", [{"F",[]}, - {"G",[]}]}, - {"C", [{"H",[]}, - {"I",[]}]}], - [{"A", ["B","C"]}, - {"B", ["A","C"]}, - {"C", ["A","B"]}, - {"D", {error, transitive_dependency}}, - %% ... - {"K", {error, transitive_dependency}}]}. +upgrades(top_a) -> + %% Original tree + {[{"A", "1", [{"B", [{"D", "1", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Updated tree + [{"A", "1", [{"B", [{"D", "3", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Modified apps, gobally + ["A","B","D"], + %% upgrade vs. new tree + {"A", [{"A","1"}, "B", "C", {"D","3"}]}}; +upgrades(top_b) -> + %% Original tree + {[{"A", "1", [{"B", [{"D", "1", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Updated tree + [{"A", "1", [{"B", [{"D", "3", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Modified apps, gobally + ["A","B","D"], + %% upgrade vs. new tree + {"B", {error, transitive_dependency}}}; +upgrades(top_c) -> + %% Original tree + {[{"A", "1", [{"B", [{"D", "1", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Updated tree + [{"A", "1", [{"B", [{"D", "3", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Modified apps, gobally + ["A","B","D"], + %% upgrade vs. new tree + {"C", {error, transitive_dependency}}}; +upgrades(top_d1) -> + %% Original tree + {[{"A", "1", [{"B", [{"D", "1", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Updated tree + [{"A", "1", [{"B", [{"D", "3", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Modified apps, gobally + ["A","B","D"], + %% upgrade vs. new tree + {"D", {error, transitive_dependency}}}; +upgrades(top_d2) -> + %% Original tree + {[{"A", "1", [{"B", [{"D", "1", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Updated tree + [{"A", "1", [{"B", [{"D", "3", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Modified apps, gobally + ["A","B","D"], + %% upgrade vs. new tree + {"D", {error, transitive_dependency}}}; +upgrades(top_e) -> + %% Original tree + {[{"A", "1", [{"B", [{"D", "1", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Updated tree + [{"A", "1", [{"B", [{"D", "3", []}]}, + {"C", [{"D", "2", []}]}]} + ], + %% Modified apps, gobally + ["A","B","D"], + %% upgrade vs. new tree + {"E", {error, unknown_dependency}}}; +upgrades(pair_a) -> + {[{"A", "1", [{"C", "1", []}]}, + {"B", "1", [{"D", "1", []}]} + ], + [{"A", "2", [{"C", "2", []}]}, + {"B", "2", [{"D", "2", []}]} + ], + ["A","B","C","D"], + {"A", [{"A","2"},{"C","2"},{"B","1"},{"D","1"}]}}; +upgrades(pair_b) -> + {[{"A", "1", [{"C", "1", []}]}, + {"B", "1", [{"D", "1", []}]} + ], + [{"A", "2", [{"C", "2", []}]}, + {"B", "2", [{"D", "2", []}]} + ], + ["A","B","C","D"], + {"B", [{"A","1"},{"C","1"},{"B","2"},{"D","2"}]}}; +upgrades(pair_c) -> + {[{"A", "1", [{"C", "1", []}]}, + {"B", "1", [{"D", "1", []}]} + ], + [{"A", "2", [{"C", "2", []}]}, + {"B", "2", [{"D", "2", []}]} + ], + ["A","B","C","D"], + {"C", {error, transitive_dependency}}}; +upgrades(triplet_a) -> + {[{"A", "1", [{"D",[]}, + {"E","3",[]}]}, + {"B", "1", [{"F","1",[]}, + {"G",[]}]}, + {"C", "0", [{"H","3",[]}, + {"I",[]}]}], + [{"A", "1", [{"D",[]}, + {"E","2",[]}]}, + {"B", "1", [{"F","1",[]}, + {"G",[]}]}, + {"C", "1", [{"H","4",[]}, + {"I",[]}]}], + ["A","C","E","H"], + {"A", [{"A","1"}, "D", {"E","2"}, + {"B","1"}, {"F","1"}, "G", + {"C","0"}, {"H","3"}, "I"]}}; +upgrades(triplet_b) -> + {[{"A", "1", [{"D",[]}, + {"E","3",[]}]}, + {"B", "1", [{"F","1",[]}, + {"G",[]}]}, + {"C", "0", [{"H","3",[]}, + {"I",[]}]}], + [{"A", "1", [{"D",[]}, + {"E","2",[]}]}, + {"B", "1", [{"F","1",[]}, + {"G",[]}]}, + {"C", "1", [{"H","4",[]}, + {"I",[]}]}], + ["A","C","E","H"], + {"B", [{"A","1"}, "D", {"E","3"}, + {"B","1"}, {"F","1"}, "G", + {"C","0"}, {"H","3"}, "I"]}}; +upgrades(triplet_c) -> + {[{"A", "1", [{"D",[]}, + {"E","3",[]}]}, + {"B", "1", [{"F","1",[]}, + {"G",[]}]}, + {"C", "0", [{"H","3",[]}, + {"I",[]}]}], + [{"A", "1", [{"D",[]}, + {"E","2",[]}]}, + {"B", "1", [{"F","1",[]}, + {"G",[]}]}, + {"C", "1", [{"H","4",[]}, + {"I",[]}]}], + ["A","C","E","H"], + {"C", [{"A","1"}, "D", {"E","3"}, + {"B","1"}, {"F","1"}, "G", + {"C","1"}, {"H","4"}, "I"]}}; +upgrades(tree_a) -> + {[{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}, + {"I","2",[]}]} + ], + [{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}]} + ], + ["C"], + {"A", [{"A","1"}, "D", "J", "E", + {"B","1"}, "F", "G", + {"C","1"}, "H", {"I","2"}]}}; +upgrades(tree_b) -> + {[{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}, + {"I","2",[]}]} + ], + [{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}]} + ], + ["C"], + {"B", [{"A","1"}, "D", "J", "E", + {"B","1"}, "F", "G", + {"C","1"}, "H", {"I","2"}]}}; +upgrades(tree_c) -> + {[{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}, + {"I","2",[]}]} + ], + [{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}]} + ], + ["C"], + {"C", [{"A","1"}, "D", "J", "E", {"I","1"}, + {"B","1"}, "F", "G", + {"C","1"}, "H"]}}. %% TODO: add a test that verifies that unlocking files and then %% running the upgrade code is enough to properly upgrade things. @@ -106,8 +278,10 @@ top_level_deps([{{pkg, Name, Vsn, _URL}, _} | Deps]) -> [{list_to_atom(Name), Vsn} | top_level_deps(Deps)]. mock_deps(git, Deps, Upgrades) -> + catch mock_git_resource:unmock(), mock_git_resource:mock([{deps, flat_deps(Deps)}, {upgrade, Upgrades}]); mock_deps(pkg, Deps, Upgrades) -> + catch mock_pkg_resource:unmock(), mock_pkg_resource:mock([{pkgdeps, flat_pkgdeps(Deps)}, {upgrade, Upgrades}]). flat_deps([]) -> []; @@ -143,43 +317,55 @@ expand_deps(pkg, [{Name, Vsn, Deps} | Rest]) -> Dep = {pkg, Name, Vsn, "https://example.org/user/"++Name++".tar.gz"}, [{Dep, expand_deps(pkg, Deps)} | expand_deps(pkg, Rest)]. -normalize_unlocks([]) -> []; -normalize_unlocks([{App, Locks} | Rest]) -> - [{iolist_to_binary(App), - normalize_unlocks_expect(Locks)} | normalize_unlocks(Rest)]; -normalize_unlocks([{App, Vsn, Locks} | Rest]) -> - [{iolist_to_binary(App), iolist_to_binary(Vsn), - normalize_unlocks_expect(Locks)} | normalize_unlocks(Rest)]. +normalize_unlocks({App, Locks}) -> + {iolist_to_binary(App), + normalize_unlocks_expect(Locks)}; +normalize_unlocks({App, Vsn, Locks}) -> + {iolist_to_binary(App), iolist_to_binary(Vsn), + normalize_unlocks_expect(Locks)}. normalize_unlocks_expect({error, Reason}) -> {error, Reason}; normalize_unlocks_expect([]) -> []; normalize_unlocks_expect([{App,Vsn} | Rest]) -> - [{iolist_to_binary(App), iolist_to_binary(Vsn)} + [{dep, App, Vsn} | normalize_unlocks_expect(Rest)]; normalize_unlocks_expect([App | Rest]) -> - [iolist_to_binary(App) | normalize_unlocks_expect(Rest)]. + [{dep, App} | normalize_unlocks_expect(Rest)]. + +top_a(Config) -> run(Config). +top_b(Config) -> run(Config). +top_c(Config) -> run(Config). +top_d1(Config) -> run(Config). +top_d2(Config) -> run(Config). +top_e(Config) -> run(Config). + +pair_a(Config) -> run(Config). +pair_b(Config) -> run(Config). +pair_c(Config) -> run(Config). + +triplet_a(Config) -> run(Config). +triplet_b(Config) -> run(Config). +triplet_c(Config) -> run(Config). -top(Config) -> run(Config). -pair(Config) -> run(Config). -triplet(Config) -> run(Config). -tree(Config) -> run(Config). +tree_a(Config) -> run(Config). +tree_b(Config) -> run(Config). +tree_c(Config) -> run(Config). run(Config) -> apply(?config(mock, Config), []), {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)), %% Install dependencies before re-mocking for an upgrade rebar_test_utils:run_and_check(Config, RebarConfig, ["lock"], {ok, []}), - Unlocks = ?config(unlocks, Config), - [begin - ct:pal("Unlocks: ~p -> ~p", [App, Unlocked]), - Expectation = case Unlocked of - Tuple when is_tuple(Tuple) -> Tuple; - _ -> {unlocked, Unlocked} - end, - rebar_test_utils:run_and_check( - Config, RebarConfig, ["upgrade", App], Expectation - ) - end || {App, Unlocked} <- Unlocks]. + {App, Unlocks} = ?config(expected, Config), + ct:pal("Upgrades: ~p -> ~p", [App, Unlocks]), + Expectation = case Unlocks of + {error, Term} -> {error, Term}; + _ -> {ok, Unlocks} + end, + apply(?config(mock_update, Config), []), + rebar_test_utils:run_and_check( + Config, RebarConfig, ["upgrade", App], Expectation + ). -- cgit v1.1 From 92436d9960b9ce1ef8a8451c33e597248e1c9bfe Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 8 Feb 2015 23:23:15 +0000 Subject: More progress on upgrades Only the most complex case is failing, where cross-dependencies would need to be refetched as an update clears an app of its dependencies and a different subtree should override it. --- src/rebar_prv_install_deps.erl | 28 +++++++++++++++++----------- src/rebar_prv_upgrade.erl | 4 ++-- test/rebar_upgrade_SUITE.erl | 13 +++++++++---- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index 094fae0..125e889 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -44,6 +44,8 @@ -define(PROVIDER, install_deps). -define(DEPS, [app_discovery]). +-define(APP_NAME_INDEX, 2). + -type src_dep() :: {atom(), {atom(), string(), string()}} | {atom(), string(), {atom(), string(), string()}}. -type pkg_dep() :: {atom(), binary()} | atom(). @@ -135,7 +137,8 @@ handle_deps(Profile, State, Deps) -> handle_deps(Profile, State, Deps, Update) when is_boolean(Update) -> handle_deps(Profile, State, Deps, Update, []); handle_deps(Profile, State, Deps, Locks) when is_list(Locks) -> - handle_deps(Profile, State, Deps, false, Locks). + Update = rebar_state:get(State, upgrade, false), + handle_deps(Profile, State, Deps, Update, Locks). -spec handle_deps(atom(), rebar_state:t(), list(), boolean() | {true, binary(), integer()}, list()) -> {ok, [rebar_app_info:t()], rebar_state:t()} | {error, string()}. @@ -277,7 +280,7 @@ update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen, end end, {[], PkgDeps, SrcApps, State, Seen, Locks}, - lists:sort(SrcDeps)) of + sort_deps(SrcDeps)) of {[], NewPkgDeps, NewSrcApps, State1, Seen1, _NewLocks} -> {State1, NewSrcApps, NewPkgDeps, Seen1}; {NewSrcDeps, NewPkgDeps, NewSrcApps, State1, Seen1, NewLocks} -> @@ -286,13 +289,10 @@ update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen, handle_update(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> Name = rebar_app_info:name(AppInfo), - ct:pal("update ~p", [Name]), case lists:keyfind(Name, 1, Locks) of false -> - ct:pal("in lock"), - case maybe_fetch(AppInfo, true, []) of + case maybe_fetch(AppInfo, true, sets:new()) of true -> - ct:pal("fetch!"), handle_dep(AppInfo ,SrcDeps ,PkgDeps @@ -302,11 +302,9 @@ handle_update(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> ,Locks); false -> - ct:pal("nofetch"), {SrcDeps, PkgDeps, SrcApps, State, Locks} end; _StillLocked -> - ct:pal("stillocked"), {SrcDeps, PkgDeps, SrcApps, State, Locks} end. @@ -365,9 +363,9 @@ maybe_fetch(AppInfo, Update, Seen) -> end end, - case not Exists orelse Update of + case not Exists of% orelse Update of true -> - ?INFO("Fetching ~s", [rebar_app_info:name(AppInfo)]), + ?INFO("Fetching ~s (~p)", [rebar_app_info:name(AppInfo), rebar_app_info:source(AppInfo)]), Source = rebar_app_info:source(AppInfo), case rebar_fetch:download_source(AppDir, Source) of {error, Reason} -> @@ -381,7 +379,7 @@ maybe_fetch(AppInfo, Update, Seen) -> false; false -> Source = rebar_app_info:source(AppInfo), - case rebar_fetch:needs_update(AppDir, Source) of + case Update andalso rebar_fetch:needs_update(AppDir, Source) of true -> ?INFO("Updating ~s", [rebar_app_info:name(AppInfo)]), case rebar_fetch:download_source(AppDir, Source) of @@ -462,6 +460,14 @@ new_dep(DepsDir, Name, Vsn, Source, State) -> rebar_state:overrides(S, ParentOverrides++Overrides)), rebar_app_info:source(Dep1, Source). +sort_deps(Deps) -> + %% We need a sort stable, based on the name. So that for multiple deps on + %% the same level with the same name, we keep the order the parents had. + %% `lists:keysort/2' is documented as stable in the stdlib. + %% The list of deps is revered when we get it. For the proper stable + %% result, re-reverse it. + lists:keysort(?APP_NAME_INDEX, lists:reverse(Deps)). + -spec parse_goal(binary(), binary()) -> pkg_dep(). parse_goal(Name, Constraint) -> case re:run(Constraint, "([^\\d]*)(\\d.*)", [{capture, [1,2], binary}]) of diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl index 0c1ed8a..e43841c 100644 --- a/src/rebar_prv_upgrade.erl +++ b/src/rebar_prv_upgrade.erl @@ -56,12 +56,12 @@ do(State) -> NewLocks = unlock_higher_than(0, Locks -- [Lock]), State1 = rebar_state:set(State, {deps, default}, [{Name, Source, 0} | NewLocks]), State2 = rebar_state:set(State1, {locks, default}, NewLocks), - rebar_prv_install_deps:do(State2) + State3 = rebar_state:set(State2, upgrade, true), + rebar_prv_install_deps:do(State3) end; {_, _, Level} when Level > 0 -> {error, transitive_dependency}; false -> - ct:pal("deps: ~p", [{Name,Locks}]), {error, unknown_dependency} end. diff --git a/test/rebar_upgrade_SUITE.erl b/test/rebar_upgrade_SUITE.erl index a7d4a06..0727fff 100644 --- a/test/rebar_upgrade_SUITE.erl +++ b/test/rebar_upgrade_SUITE.erl @@ -38,13 +38,13 @@ init_per_testcase(Case, Config) -> [{expected, normalize_unlocks(Expectations)}, {mock, fun() -> mock_deps(DepsType, Expanded, []) end}, {mock_update, fun() -> mock_deps(DepsType, UpExpanded, ToUp) end} - | setup_project(Case, Config, Expanded)]. + | setup_project(Case, Config, Expanded, UpExpanded)]. end_per_testcase(_, Config) -> meck:unload(), Config. -setup_project(Case, Config0, Deps) -> +setup_project(Case, Config0, Deps, UpDeps) -> DepsType = ?config(deps_type, Config0), Config = rebar_test_utils:init_rebar_state( Config0, @@ -54,7 +54,8 @@ setup_project(Case, Config0, Deps) -> rebar_test_utils:create_app(AppDir, "Root", "0.0.0", [kernel, stdlib]), TopDeps = top_level_deps(Deps), RebarConf = rebar_test_utils:create_config(AppDir, [{deps, TopDeps}]), - [{rebarconfig, RebarConf} | Config]. + [{rebarconfig, RebarConf}, + {next_top_deps, top_level_deps(UpDeps)} | Config]. upgrades(top_a) -> @@ -279,6 +280,7 @@ top_level_deps([{{pkg, Name, Vsn, _URL}, _} | Deps]) -> mock_deps(git, Deps, Upgrades) -> catch mock_git_resource:unmock(), + ct:pal("mocked: ~p", [flat_deps(Deps)]), mock_git_resource:mock([{deps, flat_deps(Deps)}, {upgrade, Upgrades}]); mock_deps(pkg, Deps, Upgrades) -> catch mock_pkg_resource:unmock(), @@ -365,7 +367,10 @@ run(Config) -> _ -> {ok, Unlocks} end, apply(?config(mock_update, Config), []), + NewRebarConf = rebar_test_utils:create_config(?config(apps, Config), + [{deps, ?config(next_top_deps, Config)}]), + {ok, NewRebarConfig} = file:consult(NewRebarConf), rebar_test_utils:run_and_check( - Config, RebarConfig, ["upgrade", App], Expectation + Config, NewRebarConfig, ["upgrade", App], Expectation ). -- cgit v1.1 From f5acdfb9a5f600ae260a36a1fadd45576a1536fd Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Mon, 9 Feb 2015 16:05:08 +0000 Subject: Refactor install_deps to match 'upgrade' ideas - rename 'update' to 'upgrade' -- 'update' stands for package list updates, not dependencies upgrades - extracting some deeply nested code --- src/rebar_prv_install_deps.erl | 113 ++++++++++++++++++++++------------------- 1 file changed, 61 insertions(+), 52 deletions(-) diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index 125e889..0e54324 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -134,17 +134,17 @@ handle_deps(Profile, State, Deps) -> -spec handle_deps(atom(), rebar_state:t(), list(), list() | boolean()) -> {ok, [rebar_app_info:t()], rebar_state:t()} | {error, string()}. -handle_deps(Profile, State, Deps, Update) when is_boolean(Update) -> - handle_deps(Profile, State, Deps, Update, []); +handle_deps(Profile, State, Deps, Upgrade) when is_boolean(Upgrade) -> + handle_deps(Profile, State, Deps, Upgrade, []); handle_deps(Profile, State, Deps, Locks) when is_list(Locks) -> - Update = rebar_state:get(State, upgrade, false), - handle_deps(Profile, State, Deps, Update, Locks). + Upgrade = rebar_state:get(State, upgrade, false), + handle_deps(Profile, State, Deps, Upgrade, Locks). -spec handle_deps(atom(), rebar_state:t(), list(), boolean() | {true, binary(), integer()}, list()) -> {ok, [rebar_app_info:t()], rebar_state:t()} | {error, string()}. handle_deps(_Profile, State, [], _, _) -> {ok, [], State}; -handle_deps(Profile, State, Deps, Update, Locks) -> +handle_deps(Profile, State, Deps, Upgrade, Locks) -> %% Read in package index and dep graph {Packages, Graph} = rebar_packages:get_packages(State), %% Split source deps from pkg deps, needed to keep backwards compatibility @@ -153,7 +153,7 @@ handle_deps(Profile, State, Deps, Update, Locks) -> %% Fetch transitive src deps {State1, SrcApps, PkgDeps1, Seen} = - update_src_deps(Profile, 0, SrcDeps, PkgDeps, [], State, Update, sets:new(), Locks), + update_src_deps(Profile, 0, SrcDeps, PkgDeps, [], State, Upgrade, sets:new(), Locks), {Solved, State2} = case PkgDeps1 of [] -> %% No pkg deps @@ -169,7 +169,7 @@ handle_deps(Profile, State, Deps, Update, Locks) -> [warn_skip_pkg(Pkg) || Pkg <- Discarded], Solution end, - update_pkg_deps(Profile, S, Packages, Update, Seen, State1) + update_pkg_deps(Profile, S, Packages, Upgrade, Seen, State1) end, AllDeps = lists:ukeymerge(2 @@ -184,7 +184,7 @@ handle_deps(Profile, State, Deps, Update, Locks) -> %% Internal functions %% =================================================================== -update_pkg_deps(Profile, Pkgs, Packages, Update, Seen, State) -> +update_pkg_deps(Profile, Pkgs, Packages, Upgrade, Seen, State) -> %% Create app_info record for each pkg dep DepsDir = rebar_dir:deps_dir(State), {Solved, _, State1} @@ -193,7 +193,7 @@ update_pkg_deps(Profile, Pkgs, Packages, Update, Seen, State) -> ,Packages ,Pkg), {SeenAcc1, StateAcc1} = maybe_lock(Profile, AppInfo, SeenAcc, StateAcc, 0), - case maybe_fetch(AppInfo, Update, SeenAcc1) of + case maybe_fetch(AppInfo, Upgrade, SeenAcc1) of true -> {[AppInfo | Acc], SeenAcc1, StateAcc1}; false -> @@ -239,7 +239,7 @@ package_to_app(DepsDir, Packages, {Name, Vsn}) -> end. -spec update_src_deps(atom(), non_neg_integer(), list(), list(), list(), rebar_state:t(), boolean(), sets:set(binary()), list()) -> {rebar_state:t(), list(), list(), sets:set(binary())}. -update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen, Locks) -> +update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrade, Seen, Locks) -> case lists:foldl(fun(AppInfo, {SrcDepsAcc, PkgDepsAcc, SrcAppsAcc, StateAcc, SeenAcc, LocksAcc}) -> %% If not seen, add to list of locks to write out Name = rebar_app_info:name(AppInfo), @@ -256,10 +256,10 @@ update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen, false -> {SeenAcc1, StateAcc1} = maybe_lock(Profile, AppInfo, SeenAcc, StateAcc, Level), {SrcDepsAcc1, PkgDepsAcc1, SrcAppsAcc1, StateAcc2, LocksAcc1} = - case Update of + case Upgrade of true -> - %{true, UpdateName, UpdateLevel} -> - handle_update(AppInfo + %{true, UpgradeName, UpgradeLevel} -> + handle_upgrade(AppInfo ,SrcDepsAcc ,PkgDepsAcc ,SrcAppsAcc @@ -284,10 +284,10 @@ update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen, {[], NewPkgDeps, NewSrcApps, State1, Seen1, _NewLocks} -> {State1, NewSrcApps, NewPkgDeps, Seen1}; {NewSrcDeps, NewPkgDeps, NewSrcApps, State1, Seen1, NewLocks} -> - update_src_deps(Profile, Level+1, NewSrcDeps, NewPkgDeps, NewSrcApps, State1, Update, Seen1, NewLocks) + update_src_deps(Profile, Level+1, NewSrcDeps, NewPkgDeps, NewSrcApps, State1, Upgrade, Seen1, NewLocks) end. -handle_update(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> +handle_upgrade(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> Name = rebar_app_info:name(AppInfo), case lists:keyfind(Name, 1, Locks) of false -> @@ -334,7 +334,7 @@ handle_dep(State, DepsDir, AppInfo, Locks, Level) -> AppInfo1 = rebar_app_info:state(AppInfo, S3), Deps = rebar_state:get(S3, deps, []), - %% Update lock level to be the level the dep will have in this dep tree + %% Upgrade lock level to be the level the dep will have in this dep tree NewLocks = [{DepName, Source, LockLevel+Level} || {DepName, Source, LockLevel} <- rebar_state:get(S3, {locks, default}, [])], AppInfo2 = rebar_app_info:deps(AppInfo1, rebar_state:deps_names(Deps)), @@ -343,7 +343,7 @@ handle_dep(State, DepsDir, AppInfo, Locks, Level) -> -spec maybe_fetch(rebar_app_info:t(), boolean() | {true, binary(), integer()}, sets:set(binary())) -> boolean(). -maybe_fetch(AppInfo, Update, Seen) -> +maybe_fetch(AppInfo, Upgrade, Seen) -> AppDir = ec_cnv:to_list(rebar_app_info:dir(AppInfo)), Apps = rebar_app_discover:find_apps(["_checkouts"], all), case rebar_app_utils:find(rebar_app_info:name(AppInfo), Apps) of @@ -351,46 +351,15 @@ maybe_fetch(AppInfo, Update, Seen) -> %% Don't fetch dep if it exists in the _checkouts dir false; error -> - Exists = 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, - - case not Exists of% orelse Update of + case not app_exists(AppDir) of true -> - ?INFO("Fetching ~s (~p)", [rebar_app_info:name(AppInfo), rebar_app_info:source(AppInfo)]), - Source = rebar_app_info:source(AppInfo), - case rebar_fetch:download_source(AppDir, Source) of - {error, Reason} -> - throw(Reason); - Result -> - Result - end; - _ -> + fetch_app(AppInfo, AppDir); + false -> case sets:is_element(rebar_app_info:name(AppInfo), Seen) of true -> false; false -> - Source = rebar_app_info:source(AppInfo), - case Update andalso rebar_fetch:needs_update(AppDir, Source) of - true -> - ?INFO("Updating ~s", [rebar_app_info:name(AppInfo)]), - case rebar_fetch:download_source(AppDir, Source) of - {error, Reason} -> - throw(Reason); - Result -> - Result - end; - false -> - false - end + maybe_upgrade(AppInfo, AppDir, Upgrade) end end end. @@ -460,6 +429,46 @@ 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) -> + ?INFO("Fetching ~s (~p)", [rebar_app_info:name(AppInfo), rebar_app_info:source(AppInfo)]), + Source = rebar_app_info:source(AppInfo), + case rebar_fetch:download_source(AppDir, Source) of + {error, Reason} -> + throw(Reason); + Result -> + Result + end. + +maybe_upgrade(_AppInfo, _AppDir, false) -> + false; +maybe_upgrade(AppInfo, AppDir, true) -> + Source = rebar_app_info:source(AppInfo), + case rebar_fetch:needs_update(AppDir, Source) of + true -> + ?INFO("Updating ~s", [rebar_app_info:name(AppInfo)]), + case rebar_fetch:download_source(AppDir, Source) of + {error, Reason} -> + throw(Reason); + Result -> + Result + end; + false -> + false + end. + sort_deps(Deps) -> %% We need a sort stable, based on the name. So that for multiple deps on %% the same level with the same name, we keep the order the parents had. -- cgit v1.1 From 62dc8fa9c745cca60f45ad301e05c8b70517626c Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Wed, 11 Feb 2015 14:00:43 +0000 Subject: Fix testcases, add multi-app upgrade support todo: - relock stuff - default to all apps needing upgrade - more tests? - pkgs? --- src/rebar_prv_install_deps.erl | 26 +++++++++++++--- src/rebar_prv_upgrade.erl | 71 ++++++++++++++++++++++++++++++++---------- test/mock_git_resource.erl | 3 +- test/rebar_upgrade_SUITE.erl | 69 ++++++++++++++++++++++++++++++++++------ 4 files changed, 135 insertions(+), 34 deletions(-) diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index 0e54324..dab4c24 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -252,7 +252,22 @@ update_src_deps(Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrade, Seen, true -> ok end, - {SrcDepsAcc, PkgDepsAcc, SrcAppsAcc, StateAcc, SeenAcc, LocksAcc}; + %% scan for app children here if upgrading + case Upgrade of + false -> + {SrcDepsAcc, PkgDepsAcc, SrcAppsAcc, StateAcc, SeenAcc, LocksAcc}; + true -> + {SrcDepsAcc1, PkgDepsAcc1, SrcAppsAcc1, StateAcc2, LocksAcc1} = + handle_dep(AppInfo + ,SrcDepsAcc + ,PkgDepsAcc + ,SrcAppsAcc + ,Level + ,StateAcc + ,LocksAcc), + + {SrcDepsAcc1, PkgDepsAcc1, SrcAppsAcc1, StateAcc2, SeenAcc, LocksAcc1} + end; false -> {SeenAcc1, StateAcc1} = maybe_lock(Profile, AppInfo, SeenAcc, StateAcc, Level), {SrcDepsAcc1, PkgDepsAcc1, SrcAppsAcc1, StateAcc2, LocksAcc1} = @@ -302,10 +317,10 @@ handle_upgrade(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> ,Locks); false -> - {SrcDeps, PkgDeps, SrcApps, State, Locks} + {[AppInfo|SrcDeps], PkgDeps, SrcApps, State, Locks} end; _StillLocked -> - {SrcDeps, PkgDeps, SrcApps, State, Locks} + {[AppInfo|SrcDeps], PkgDeps, SrcApps, State, Locks} end. handle_dep(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) -> @@ -452,8 +467,9 @@ fetch_app(AppInfo, AppDir) -> Result end. -maybe_upgrade(_AppInfo, _AppDir, false) -> - false; +maybe_upgrade(AppInfo, AppDir, false) -> + Source = rebar_app_info:source(AppInfo), + rebar_fetch:needs_update(AppDir, Source); maybe_upgrade(AppInfo, AppDir, true) -> Source = rebar_app_info:source(AppInfo), case rebar_fetch:needs_update(AppDir, Source) of diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl index e43841c..b9c6495 100644 --- a/src/rebar_prv_upgrade.erl +++ b/src/rebar_prv_upgrade.erl @@ -29,47 +29,84 @@ init(State) -> {module, ?MODULE}, {bare, false}, {deps, ?DEPS}, - {example, "rebar upgrade cowboy"}, + {example, "rebar upgrade cowboy[,ranch]"}, {short_desc, "Upgrade dependency."}, {desc, ""}, {opts, [ - {package, undefined, undefined, string, "Package to upgrade."} + {package, undefined, undefined, string, "Packages to upgrade."} ]}])), {ok, State1}. -spec do(rebar_state:t()) -> {ok, rebar_state:t()} | {error, string()}. do(State) -> {Args, _} = rebar_state:command_parsed_args(State), - Name = ec_cnv:to_binary(proplists:get_value(package, Args)), + Names = parse_names(ec_cnv:to_binary(proplists:get_value(package, Args))), + %% TODO: support many names. Only a subtree can be updated per app + %% mentioned. When no app is named, unlock *everything* Locks = rebar_state:get(State, {locks, default}, []), + Deps = rebar_state:get(State, deps), + case prepare_locks(Names, Deps, Locks, []) of + {error, Reason} -> + {error, Reason}; + {Locks0, Unlocks0} -> + Deps0 = top_level_deps(Deps, Locks), + State1 = rebar_state:set(State, {deps, default}, Deps0), + State2 = rebar_state:set(State1, {locks, default}, Locks0), + State3 = rebar_state:set(State2, upgrade, true), + Res = rebar_prv_install_deps:do(State3), + case Res of + {ok, S} -> + ct:pal("original locks ~p", [Locks]), + ct:pal("new locks ~p", [Locks0]), + ct:pal("old deps: ~p", [Deps]), + ct:pal("new deps: ~p", [Deps0]), + ct:pal("Unlocks: ~p", [Unlocks0]), + %% TODO: replace new locks onto the old locks list + rebar_prv_lock:do(S); + _ -> Res + end + end. + +parse_names(Bin) -> + lists:usort(re:split(Bin, <<" *, *">>, [trim])). + +prepare_locks([], _, Locks, Unlocks) -> + {Locks, Unlocks}; +prepare_locks([Name|Names], Deps, Locks, Unlocks) -> case lists:keyfind(Name, 1, Locks) of {_, _, 0} = Lock -> - Deps = rebar_state:get(State, deps), - case lists:keyfind(binary_to_atom(Name, utf8), 1, Deps) of + AtomName = binary_to_atom(Name, utf8), + case lists:keyfind(AtomName, 1, Deps) of false -> - {error, unknown_dependency}; + {error, {unknown_dependency, Name}}; Dep -> Source = case Dep of {_, Src} -> Src; {_, _, Src} -> Src end, - NewLocks = unlock_higher_than(0, Locks -- [Lock]), - State1 = rebar_state:set(State, {deps, default}, [{Name, Source, 0} | NewLocks]), - State2 = rebar_state:set(State1, {locks, default}, NewLocks), - State3 = rebar_state:set(State2, upgrade, true), - rebar_prv_install_deps:do(State3) + {NewLocks, NewUnlocks} = unlock_higher_than(0, Locks -- [Lock]), + prepare_locks(Names, + %deps_like_locks(Deps, [{Name,Source,0} | NewLocks]), + Deps, + NewLocks, + [{Name, Source, 0} | NewUnlocks ++ Unlocks]) end; {_, _, Level} when Level > 0 -> - {error, transitive_dependency}; + {error, {transitive_dependency,Name}}; false -> - {error, unknown_dependency} + {error, {unknown_dependency,Name}} end. +top_level_deps(Deps, Locks) -> + [Dep || Dep <- Deps, lists:keymember(0, 3, Locks)]. + +unlock_higher_than(Level, Locks) -> unlock_higher_than(Level, Locks, [], []). -unlock_higher_than(_, []) -> []; -unlock_higher_than(Level, [App = {_,_,AppLevel} | Apps]) -> - if AppLevel > Level -> unlock_higher_than(Level, Apps); - AppLevel =< Level -> [App | unlock_higher_than(Level, Apps)] +unlock_higher_than(_, [], Locks, Unlocks) -> + {Locks, Unlocks}; +unlock_higher_than(Level, [App = {_,_,AppLevel} | Apps], Locks, Unlocks) -> + if AppLevel > Level -> unlock_higher_than(Level, Apps, Locks, [App | Unlocks]); + AppLevel =< Level -> unlock_higher_than(Level, Apps, [App | Locks], Unlocks) end. -spec format_error(any()) -> iolist(). diff --git a/test/mock_git_resource.erl b/test/mock_git_resource.erl index 5d8288e..2f7a72d 100644 --- a/test/mock_git_resource.erl +++ b/test/mock_git_resource.erl @@ -60,7 +60,7 @@ mock_update(Opts) -> ?MOD, needs_update, fun(_Dir, {git, Url, _Ref}) -> App = app(Url), - ct:pal("Needed update? ~p -> ~p", [App, lists:member(App, ToUpdate)]), + ct:pal("Needed update? ~p (~p) -> ~p", [App, {Url,_Ref}, lists:member(App, ToUpdate)]), lists:member(App, ToUpdate) end). @@ -108,7 +108,6 @@ mock_download(Opts) -> {git, Url, {_, Vsn}} = normalize_git(Git, Overrides, Default), App = app(Url), AppDeps = proplists:get_value({App,Vsn}, Deps, []), - ct:pal("creating app ~p", [{Dir, App, Vsn, AppDeps}]), rebar_test_utils:create_app( Dir, App, Vsn, [element(1,D) || D <- AppDeps] diff --git a/test/rebar_upgrade_SUITE.erl b/test/rebar_upgrade_SUITE.erl index 0727fff..6491527 100644 --- a/test/rebar_upgrade_SUITE.erl +++ b/test/rebar_upgrade_SUITE.erl @@ -7,9 +7,9 @@ all() -> [{group, git}].%, {group, pkg}]. groups() -> [{all, [], [top_a, top_b, top_c, top_d1, top_d2, top_e, - pair_a, pair_b, pair_c, + pair_a, pair_b, pair_ab, pair_c, triplet_a, triplet_b, triplet_c, - tree_a, tree_b, tree_c]}, + tree_a, tree_b, tree_c, tree_c2, tree_ac]}, {git, [], [{group, all}]}, {pkg, [], [{group, all}]}]. @@ -83,7 +83,7 @@ upgrades(top_b) -> %% Modified apps, gobally ["A","B","D"], %% upgrade vs. new tree - {"B", {error, transitive_dependency}}}; + {"B", {error, {transitive_dependency, <<"B">>}}}}; upgrades(top_c) -> %% Original tree {[{"A", "1", [{"B", [{"D", "1", []}]}, @@ -96,7 +96,7 @@ upgrades(top_c) -> %% Modified apps, gobally ["A","B","D"], %% upgrade vs. new tree - {"C", {error, transitive_dependency}}}; + {"C", {error, {transitive_dependency, <<"C">>}}}}; upgrades(top_d1) -> %% Original tree {[{"A", "1", [{"B", [{"D", "1", []}]}, @@ -109,7 +109,7 @@ upgrades(top_d1) -> %% Modified apps, gobally ["A","B","D"], %% upgrade vs. new tree - {"D", {error, transitive_dependency}}}; + {"D", {error, {transitive_dependency, <<"D">>}}}}; upgrades(top_d2) -> %% Original tree {[{"A", "1", [{"B", [{"D", "1", []}]}, @@ -122,7 +122,7 @@ upgrades(top_d2) -> %% Modified apps, gobally ["A","B","D"], %% upgrade vs. new tree - {"D", {error, transitive_dependency}}}; + {"D", {error, {transitive_dependency, <<"D">>}}}}; upgrades(top_e) -> %% Original tree {[{"A", "1", [{"B", [{"D", "1", []}]}, @@ -135,7 +135,7 @@ upgrades(top_e) -> %% Modified apps, gobally ["A","B","D"], %% upgrade vs. new tree - {"E", {error, unknown_dependency}}}; + {"E", {error, {unknown_dependency, <<"E">>}}}}; upgrades(pair_a) -> {[{"A", "1", [{"C", "1", []}]}, {"B", "1", [{"D", "1", []}]} @@ -154,6 +154,15 @@ upgrades(pair_b) -> ], ["A","B","C","D"], {"B", [{"A","1"},{"C","1"},{"B","2"},{"D","2"}]}}; +upgrades(pair_ab) -> + {[{"A", "1", [{"C", "1", []}]}, + {"B", "1", [{"D", "1", []}]} + ], + [{"A", "2", [{"C", "2", []}]}, + {"B", "2", [{"D", "2", []}]} + ], + ["A","B","C","D"], + {"A,B", [{"A","2"},{"C","2"},{"B","2"},{"D","2"}]}}; upgrades(pair_c) -> {[{"A", "1", [{"C", "1", []}]}, {"B", "1", [{"D", "1", []}]} @@ -162,7 +171,7 @@ upgrades(pair_c) -> {"B", "2", [{"D", "2", []}]} ], ["A","B","C","D"], - {"C", {error, transitive_dependency}}}; + {"C", {error, {transitive_dependency, <<"C">>}}}}; upgrades(triplet_a) -> {[{"A", "1", [{"D",[]}, {"E","3",[]}]}, @@ -264,10 +273,47 @@ upgrades(tree_c) -> {"G",[]}]}, {"C", "1", [{"H",[]}]} ], - ["C"], + ["C","I"], {"C", [{"A","1"}, "D", "J", "E", {"I","1"}, {"B","1"}, "F", "G", - {"C","1"}, "H"]}}. + {"C","1"}, "H"]}}; +upgrades(tree_c2) -> + {[{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}, + {"I","2",[]}]} + ], + [{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[{"K",[]}]}, + {"I","2",[]}]} + ], + ["C", "H"], + {"C", [{"A","1"}, "D", "J", "E", + {"B","1"}, "F", "G", + {"C","1"}, "H", {"I", "2"}, "K"]}}; +upgrades(tree_ac) -> + {[{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}, + {"I","2",[]}]} + ], + [{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}]} + ], + ["C","I"], + {"C, A", [{"A","1"}, "D", "J", "E", {"I","1"}, + {"B","1"}, "F", "G", + {"C","1"}, "H"]}}. %% TODO: add a test that verifies that unlocking files and then %% running the upgrade code is enough to properly upgrade things. @@ -345,6 +391,7 @@ top_e(Config) -> run(Config). pair_a(Config) -> run(Config). pair_b(Config) -> run(Config). +pair_ab(Config) -> run(Config). pair_c(Config) -> run(Config). triplet_a(Config) -> run(Config). @@ -354,6 +401,8 @@ triplet_c(Config) -> run(Config). tree_a(Config) -> run(Config). tree_b(Config) -> run(Config). tree_c(Config) -> run(Config). +tree_c2(Config) -> run(Config). +tree_ac(Config) -> run(Config). run(Config) -> apply(?config(mock, Config), []), -- cgit v1.1 From 6d8567a7edbf206a0f595c22ecd17af0c46acb87 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Wed, 11 Feb 2015 20:06:49 +0000 Subject: Support multiple app upgrade & lock tests - Many apps is supported through and through - Not mentioning any app upgrades all apps - Locks are refreshed on disk and tested as such after an upgrade --- src/rebar_prv_upgrade.erl | 38 +++++++++++++++++++------------------- test/rebar_test_utils.erl | 14 ++++++++++++++ test/rebar_upgrade_SUITE.erl | 41 ++++++++++++++++++++++++++++++++++++----- 3 files changed, 69 insertions(+), 24 deletions(-) diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl index b9c6495..5ee2be6 100644 --- a/src/rebar_prv_upgrade.erl +++ b/src/rebar_prv_upgrade.erl @@ -40,35 +40,39 @@ init(State) -> -spec do(rebar_state:t()) -> {ok, rebar_state:t()} | {error, string()}. do(State) -> {Args, _} = rebar_state:command_parsed_args(State), - Names = parse_names(ec_cnv:to_binary(proplists:get_value(package, Args))), - %% TODO: support many names. Only a subtree can be updated per app - %% mentioned. When no app is named, unlock *everything* Locks = rebar_state:get(State, {locks, default}, []), Deps = rebar_state:get(State, deps), + Names = parse_names(ec_cnv:to_binary(proplists:get_value(package, Args)), Locks), case prepare_locks(Names, Deps, Locks, []) of {error, Reason} -> {error, Reason}; - {Locks0, Unlocks0} -> + {Locks0, _Unlocks0} -> % unlocks may be useful for deletions Deps0 = top_level_deps(Deps, Locks), State1 = rebar_state:set(State, {deps, default}, Deps0), State2 = rebar_state:set(State1, {locks, default}, Locks0), State3 = rebar_state:set(State2, upgrade, true), Res = rebar_prv_install_deps:do(State3), case Res of - {ok, S} -> - ct:pal("original locks ~p", [Locks]), - ct:pal("new locks ~p", [Locks0]), - ct:pal("old deps: ~p", [Deps]), - ct:pal("new deps: ~p", [Deps0]), - ct:pal("Unlocks: ~p", [Unlocks0]), - %% TODO: replace new locks onto the old locks list - rebar_prv_lock:do(S); - _ -> Res + {ok, State4} -> + rebar_prv_lock:do(State4); + _ -> + Res end end. -parse_names(Bin) -> - lists:usort(re:split(Bin, <<" *, *">>, [trim])). +-spec format_error(any()) -> iolist(). +format_error(Reason) -> + io_lib:format("~p", [Reason]). + + +parse_names(Bin, Locks) -> + case lists:usort(re:split(Bin, <<" *, *">>, [trim])) of + %% Nothing submitted, use *all* apps + [<<"">>] -> [Name || {Name, _, 0} <- Locks]; + [] -> [Name || {Name, _, 0} <- Locks]; + %% Regular options + Other -> Other + end. prepare_locks([], _, Locks, Unlocks) -> {Locks, Unlocks}; @@ -109,7 +113,3 @@ unlock_higher_than(Level, [App = {_,_,AppLevel} | Apps], Locks, Unlocks) -> AppLevel =< Level -> unlock_higher_than(Level, Apps, [App | Locks], Unlocks) end. --spec format_error(any()) -> iolist(). -format_error(Reason) -> - io_lib:format("~p", [Reason]). - diff --git a/test/rebar_test_utils.erl b/test/rebar_test_utils.erl index 827d134..96200a6 100644 --- a/test/rebar_test_utils.erl +++ b/test/rebar_test_utils.erl @@ -103,6 +103,8 @@ create_random_vsn() -> check_results(AppDir, Expected) -> BuildDir = filename:join([AppDir, "_build", "lib"]), CheckoutsDir = filename:join([AppDir, "_checkouts"]), + LockFile = filename:join([AppDir, "rebar.lock"]), + Locks = lists:flatten(rebar_config:consult_file(LockFile)), Apps = rebar_app_discover:find_apps([AppDir]), InvalidApps = rebar_app_discover:find_apps([AppDir], invalid), ValidApps = rebar_app_discover:find_apps([AppDir], valid), @@ -153,6 +155,18 @@ check_results(AppDir, Expected) -> ?assertEqual(iolist_to_binary(Vsn), iolist_to_binary(rebar_app_info:original_vsn(App))) end + ; ({lock, Name}) -> + ct:pal("Name: ~p", [Name]), + ?assertNotEqual(false, lists:keyfind(iolist_to_binary(Name), 1, Locks)) + ; ({lock, Name, Vsn}) -> + ct:pal("Name: ~p, Vsn: ~p", [Name, Vsn]), + case lists:keyfind(iolist_to_binary(Name), 1, Locks) of + false -> + error({lock_not_found, Name}); + {_LockName, {_, _, {ref, LockVsn}}, _} -> + ?assertEqual(iolist_to_binary(Vsn), + iolist_to_binary(LockVsn)) + end end, Expected). write_src_file(Dir, Name) -> diff --git a/test/rebar_upgrade_SUITE.erl b/test/rebar_upgrade_SUITE.erl index 6491527..e0bb011 100644 --- a/test/rebar_upgrade_SUITE.erl +++ b/test/rebar_upgrade_SUITE.erl @@ -7,9 +7,9 @@ all() -> [{group, git}].%, {group, pkg}]. groups() -> [{all, [], [top_a, top_b, top_c, top_d1, top_d2, top_e, - pair_a, pair_b, pair_ab, pair_c, + pair_a, pair_b, pair_ab, pair_c, pair_all, triplet_a, triplet_b, triplet_c, - tree_a, tree_b, tree_c, tree_c2, tree_ac]}, + tree_a, tree_b, tree_c, tree_c2, tree_ac, tree_all]}, {git, [], [{group, all}]}, {pkg, [], [{group, all}]}]. @@ -172,6 +172,15 @@ upgrades(pair_c) -> ], ["A","B","C","D"], {"C", {error, {transitive_dependency, <<"C">>}}}}; +upgrades(pair_all) -> + {[{"A", "1", [{"C", "1", []}]}, + {"B", "1", [{"D", "1", []}]} + ], + [{"A", "2", [{"C", "2", []}]}, + {"B", "2", [{"D", "2", []}]} + ], + ["A","B","C","D"], + {"", [{"A","2"},{"C","2"},{"B","2"},{"D","2"}]}}; upgrades(triplet_a) -> {[{"A", "1", [{"D",[]}, {"E","3",[]}]}, @@ -313,7 +322,25 @@ upgrades(tree_ac) -> ["C","I"], {"C, A", [{"A","1"}, "D", "J", "E", {"I","1"}, {"B","1"}, "F", "G", - {"C","1"}, "H"]}}. + {"C","1"}, "H"]}}; +upgrades(tree_all) -> + {[{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}, + {"I","2",[]}]} + ], + [{"A", "1", [{"D",[{"J",[]}]}, + {"E",[{"I","1",[]}]}]}, + {"B", "1", [{"F",[]}, + {"G",[]}]}, + {"C", "1", [{"H",[]}]} + ], + ["C","I"], + {"", [{"A","1"}, "D", "J", "E", {"I","1"}, + {"B","1"}, "F", "G", + {"C","1"}, "H"]}}. %% TODO: add a test that verifies that unlocking files and then %% running the upgrade code is enough to properly upgrade things. @@ -377,10 +404,12 @@ normalize_unlocks_expect({error, Reason}) -> normalize_unlocks_expect([]) -> []; normalize_unlocks_expect([{App,Vsn} | Rest]) -> - [{dep, App, Vsn} + [{dep, App, Vsn}, + {lock, App, Vsn} | normalize_unlocks_expect(Rest)]; normalize_unlocks_expect([App | Rest]) -> - [{dep, App} | normalize_unlocks_expect(Rest)]. + [{dep, App}, + {lock, App} | normalize_unlocks_expect(Rest)]. top_a(Config) -> run(Config). top_b(Config) -> run(Config). @@ -393,6 +422,7 @@ pair_a(Config) -> run(Config). pair_b(Config) -> run(Config). pair_ab(Config) -> run(Config). pair_c(Config) -> run(Config). +pair_all(Config) -> run(Config). triplet_a(Config) -> run(Config). triplet_b(Config) -> run(Config). @@ -403,6 +433,7 @@ tree_b(Config) -> run(Config). tree_c(Config) -> run(Config). tree_c2(Config) -> run(Config). tree_ac(Config) -> run(Config). +tree_all(Config) -> run(Config). run(Config) -> apply(?config(mock, Config), []), -- cgit v1.1 From 2e34e91dfb54218597f72faac3a39aedd57bcf97 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Wed, 11 Feb 2015 21:00:37 +0000 Subject: Warnings for Deletions and friendly errors Apps that are no longer used are not automatically deleted, but we tell users it can be done. This is safer while we're not sure of the correctness of these messages. Error messages are added for transient dependencies and dependencies not found. --- src/rebar_prv_upgrade.erl | 38 ++++++++++++++++++++++++++++---------- test/mock_git_resource.erl | 2 +- test/rebar_upgrade_SUITE.erl | 37 ++++++++++++++++++++++++++++--------- 3 files changed, 57 insertions(+), 20 deletions(-) diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl index 5ee2be6..1cae5aa 100644 --- a/src/rebar_prv_upgrade.erl +++ b/src/rebar_prv_upgrade.erl @@ -29,11 +29,14 @@ init(State) -> {module, ?MODULE}, {bare, false}, {deps, ?DEPS}, - {example, "rebar upgrade cowboy[,ranch]"}, - {short_desc, "Upgrade dependency."}, - {desc, ""}, + {example, "rebar3 upgrade [cowboy[,ranch]]"}, + {short_desc, "Upgrade dependencies."}, + {desc, "Upgrade project dependecies. Mentioning no application " + "will upgrade all dependencies. To upgrade specific dependencies, " + "their names can be listed in the command."}, {opts, [ - {package, undefined, undefined, string, "Packages to upgrade."} + {package, undefined, undefined, string, + "List of packages to upgrade. If not specified, all dependencies are upgraded."} ]}])), {ok, State1}. @@ -42,11 +45,11 @@ do(State) -> {Args, _} = rebar_state:command_parsed_args(State), Locks = rebar_state:get(State, {locks, default}, []), Deps = rebar_state:get(State, deps), - Names = parse_names(ec_cnv:to_binary(proplists:get_value(package, Args)), Locks), + Names = parse_names(ec_cnv:to_binary(proplists:get_value(package, Args, <<"">>)), Locks), case prepare_locks(Names, Deps, Locks, []) of {error, Reason} -> {error, Reason}; - {Locks0, _Unlocks0} -> % unlocks may be useful for deletions + {Locks0, _Unlocks0} -> Deps0 = top_level_deps(Deps, Locks), State1 = rebar_state:set(State, {deps, default}, Deps0), State2 = rebar_state:set(State1, {locks, default}, Locks0), @@ -54,6 +57,10 @@ do(State) -> Res = rebar_prv_install_deps:do(State3), case Res of {ok, State4} -> + info_useless( + [element(1,Lock) || Lock <- Locks], + [rebar_app_info:name(App) || App <- rebar_state:lock(State4)] + ), rebar_prv_lock:do(State4); _ -> Res @@ -61,6 +68,12 @@ do(State) -> end. -spec format_error(any()) -> iolist(). +format_error({unknown_dependency, Name}) -> + io_lib:format("Dependency ~ts not found", [Name]); +format_error({transitive_dependency, Name}) -> + io_lib:format("Dependency ~ts is transient and cannot be safely upgraded. " + "Promote it to your top-level rebar.config file to upgrade it.", + [Name]); format_error(Reason) -> io_lib:format("~p", [Reason]). @@ -82,7 +95,7 @@ prepare_locks([Name|Names], Deps, Locks, Unlocks) -> AtomName = binary_to_atom(Name, utf8), case lists:keyfind(AtomName, 1, Deps) of false -> - {error, {unknown_dependency, Name}}; + {error, {?MODULE, {unknown_dependency, Name}}}; Dep -> Source = case Dep of {_, Src} -> Src; @@ -90,15 +103,14 @@ prepare_locks([Name|Names], Deps, Locks, Unlocks) -> end, {NewLocks, NewUnlocks} = unlock_higher_than(0, Locks -- [Lock]), prepare_locks(Names, - %deps_like_locks(Deps, [{Name,Source,0} | NewLocks]), Deps, NewLocks, [{Name, Source, 0} | NewUnlocks ++ Unlocks]) end; {_, _, Level} when Level > 0 -> - {error, {transitive_dependency,Name}}; + {error, {?MODULE, {transitive_dependency,Name}}}; false -> - {error, {unknown_dependency,Name}} + {error, {?MODULE, {unknown_dependency,Name}}} end. top_level_deps(Deps, Locks) -> @@ -113,3 +125,9 @@ unlock_higher_than(Level, [App = {_,_,AppLevel} | Apps], Locks, Unlocks) -> AppLevel =< Level -> unlock_higher_than(Level, Apps, [App | Locks], Unlocks) end. +info_useless(Old, New) -> + [?INFO("App ~ts is no longer needed and can be deleted.", [Name]) + || Name <- Old, + not lists:member(Name, New)], + ok. + diff --git a/test/mock_git_resource.erl b/test/mock_git_resource.erl index 2f7a72d..d2f0207 100644 --- a/test/mock_git_resource.erl +++ b/test/mock_git_resource.erl @@ -60,7 +60,7 @@ mock_update(Opts) -> ?MOD, needs_update, fun(_Dir, {git, Url, _Ref}) -> App = app(Url), - ct:pal("Needed update? ~p (~p) -> ~p", [App, {Url,_Ref}, lists:member(App, ToUpdate)]), +% ct:pal("Needed update? ~p (~p) -> ~p", [App, {Url,_Ref}, lists:member(App, ToUpdate)]), lists:member(App, ToUpdate) end). diff --git a/test/rebar_upgrade_SUITE.erl b/test/rebar_upgrade_SUITE.erl index e0bb011..39b9687 100644 --- a/test/rebar_upgrade_SUITE.erl +++ b/test/rebar_upgrade_SUITE.erl @@ -9,7 +9,8 @@ groups() -> [{all, [], [top_a, top_b, top_c, top_d1, top_d2, top_e, pair_a, pair_b, pair_ab, pair_c, pair_all, triplet_a, triplet_b, triplet_c, - tree_a, tree_b, tree_c, tree_c2, tree_ac, tree_all]}, + tree_a, tree_b, tree_c, tree_c2, tree_ac, tree_all, + delete_d]}, {git, [], [{group, all}]}, {pkg, [], [{group, all}]}]. @@ -83,7 +84,7 @@ upgrades(top_b) -> %% Modified apps, gobally ["A","B","D"], %% upgrade vs. new tree - {"B", {error, {transitive_dependency, <<"B">>}}}}; + {"B", {error, {rebar_prv_upgrade, {transitive_dependency, <<"B">>}}}}}; upgrades(top_c) -> %% Original tree {[{"A", "1", [{"B", [{"D", "1", []}]}, @@ -96,7 +97,7 @@ upgrades(top_c) -> %% Modified apps, gobally ["A","B","D"], %% upgrade vs. new tree - {"C", {error, {transitive_dependency, <<"C">>}}}}; + {"C", {error, {rebar_prv_upgrade, {transitive_dependency, <<"C">>}}}}}; upgrades(top_d1) -> %% Original tree {[{"A", "1", [{"B", [{"D", "1", []}]}, @@ -109,7 +110,7 @@ upgrades(top_d1) -> %% Modified apps, gobally ["A","B","D"], %% upgrade vs. new tree - {"D", {error, {transitive_dependency, <<"D">>}}}}; + {"D", {error, {rebar_prv_upgrade, {transitive_dependency, <<"D">>}}}}}; upgrades(top_d2) -> %% Original tree {[{"A", "1", [{"B", [{"D", "1", []}]}, @@ -122,7 +123,7 @@ upgrades(top_d2) -> %% Modified apps, gobally ["A","B","D"], %% upgrade vs. new tree - {"D", {error, {transitive_dependency, <<"D">>}}}}; + {"D", {error, {rebar_prv_upgrade, {transitive_dependency, <<"D">>}}}}}; upgrades(top_e) -> %% Original tree {[{"A", "1", [{"B", [{"D", "1", []}]}, @@ -135,7 +136,7 @@ upgrades(top_e) -> %% Modified apps, gobally ["A","B","D"], %% upgrade vs. new tree - {"E", {error, {unknown_dependency, <<"E">>}}}}; + {"E", {error, {rebar_prv_upgrade, {unknown_dependency, <<"E">>}}}}}; upgrades(pair_a) -> {[{"A", "1", [{"C", "1", []}]}, {"B", "1", [{"D", "1", []}]} @@ -171,7 +172,7 @@ upgrades(pair_c) -> {"B", "2", [{"D", "2", []}]} ], ["A","B","C","D"], - {"C", {error, {transitive_dependency, <<"C">>}}}}; + {"C", {error, {rebar_prv_upgrade, {transitive_dependency, <<"C">>}}}}}; upgrades(pair_all) -> {[{"A", "1", [{"C", "1", []}]}, {"B", "1", [{"D", "1", []}]} @@ -340,7 +341,17 @@ upgrades(tree_all) -> ["C","I"], {"", [{"A","1"}, "D", "J", "E", {"I","1"}, {"B","1"}, "F", "G", - {"C","1"}, "H"]}}. + {"C","1"}, "H"]}}; +upgrades(delete_d) -> + {[{"A", "1", [{"B", [{"D", "1", []}]}, + {"C", [{"D", "2", []}]}]} + ], + [{"A", "2", [{"B", []}, + {"C", []}]} + ], + ["A","B", "C"], + %% upgrade vs. new tree + {"", [{"A","2"}, "B", "C"]}}. %% TODO: add a test that verifies that unlocking files and then %% running the upgrade code is enough to properly upgrade things. @@ -353,7 +364,6 @@ top_level_deps([{{pkg, Name, Vsn, _URL}, _} | Deps]) -> mock_deps(git, Deps, Upgrades) -> catch mock_git_resource:unmock(), - ct:pal("mocked: ~p", [flat_deps(Deps)]), mock_git_resource:mock([{deps, flat_deps(Deps)}, {upgrade, Upgrades}]); mock_deps(pkg, Deps, Upgrades) -> catch mock_pkg_resource:unmock(), @@ -435,6 +445,15 @@ tree_c2(Config) -> run(Config). tree_ac(Config) -> run(Config). tree_all(Config) -> run(Config). +delete_d(Config) -> + meck:new(rebar_log, [no_link, passthrough]), + run(Config), + Infos = [{Str, Args} + || {_, {rebar_log, log, [info, Str, Args]}, _} <- meck:history(rebar_log)], + meck:unload(rebar_log), + ?assertNotEqual([], + [1 || {"App ~ts is no longer needed and can be deleted.", + [<<"D">>]} <- Infos]). run(Config) -> apply(?config(mock, Config), []), {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)), -- cgit v1.1