From c9fa7339bf1deddadc59453d7cddc0a63864830e Mon Sep 17 00:00:00 2001
From: Pablo Costas <pablo.costas.sanchez@gmail.com>
Date: Fri, 28 Aug 2020 18:34:36 +0200
Subject: [PATCH] Improve warnings about checkout dependencies when locking and
 upgrading

---
 src/rebar_prv_lock.erl    | 11 ++++++++++-
 src/rebar_prv_upgrade.erl | 29 +++++++++++++++++++----------
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/src/rebar_prv_lock.erl b/src/rebar_prv_lock.erl
index 03cf36ca..917480f0 100644
--- a/src/rebar_prv_lock.erl
+++ b/src/rebar_prv_lock.erl
@@ -38,9 +38,14 @@ do(State) ->
             rebar_config:maybe_write_lock_file(filename:join(Dir, ?LOCK_FILE), Locks, OldLocks),
             State1 = rebar_state:set(State, {locks, default}, Locks),
 
-            OldLockNames = [element(1,L) || L <- OldLocks],
+            Checkouts = [rebar_app_info:name(Dep)
+                            || Dep <- rebar_state:all_deps(State), rebar_app_info:is_checkout(Dep)],
+            %% Remove the checkout dependencies from the old lock info
+            %% so that they do not appear in the rebar_utils:info_useless/1 warning.
+            OldLockNames = [element(1,L) || L <- OldLocks] -- Checkouts,
             NewLockNames = [element(1,L) || L <- Locks],
             rebar_utils:info_useless(OldLockNames, NewLockNames),
+            info_checkout_deps(Checkouts),
 
             {ok, State1};
         _ ->
@@ -60,3 +65,7 @@ build_locks(State) ->
          rebar_fetch:lock_source(Dep, State),
          rebar_app_info:dep_level(Dep)}
      end || Dep <- AllDeps, not(rebar_app_info:is_checkout(Dep))].
+
+info_checkout_deps(Checkouts) ->
+    [?INFO("App ~ts is a checkout dependency and cannot be locked.", [CheckoutDep])
+        || CheckoutDep <- Checkouts].
\ No newline at end of file
diff --git a/src/rebar_prv_upgrade.erl b/src/rebar_prv_upgrade.erl
index 565f342c..c03a78c2 100644
--- a/src/rebar_prv_upgrade.erl
+++ b/src/rebar_prv_upgrade.erl
@@ -86,7 +86,9 @@ do_(State) ->
     DepsDict = deps_dict(rebar_state:all_deps(State)),
     AltDeps = find_non_default_deps(Deps, State),
     FilteredNames = cull_default_names_if_profiles(Names, Deps, State),
-    case prepare_locks(FilteredNames, Deps, Locks, [], DepsDict, AltDeps) of
+    Checkouts = [rebar_app_info:name(Dep)
+                            || Dep <- rebar_state:all_deps(State), rebar_app_info:is_checkout(Dep)],
+    case prepare_locks(FilteredNames, Deps, Locks, [], DepsDict, AltDeps, Checkouts) of
         {error, Reason} ->
             {error, Reason};
         {Locks0, Unlocks0} ->
@@ -123,6 +125,8 @@ format_error({transitive_dependency, Name}) ->
     io_lib:format("Dependency ~ts is transitive and cannot be safely upgraded. "
                  "Promote it to your top-level rebar.config file to upgrade it.",
                  [Name]);
+format_error({checkout_dependency, Name}) ->
+    io_lib:format("Dependency ~ts is a checkout dependency and cannot be upgraded.", [Name]);
 format_error(Reason) ->
     io_lib:format("~p", [Reason]).
 
@@ -190,20 +194,20 @@ cull_default_names_if_profiles(Names, Deps, State) ->
             end, Names)
     end.
 
-prepare_locks([], _, Locks, Unlocks, _Dict, _AltDeps) ->
+prepare_locks([], _, Locks, Unlocks, _Dict, _AltDeps, _Checkouts) ->
     {Locks, Unlocks};
-prepare_locks([Name|Names], Deps, Locks, Unlocks, Dict, AltDeps) ->
+prepare_locks([Name|Names], Deps, Locks, Unlocks, Dict, AltDeps, Checkouts) ->
     AtomName = binary_to_atom(Name, utf8),
     case lists:keyfind(Name, 1, Locks) of
         {_, _, 0} = Lock ->
             case rebar_utils:tup_find(AtomName, Deps) of
                 false ->
                     ?WARN("Dependency ~ts has been removed and will not be upgraded", [Name]),
-                    prepare_locks(Names, Deps, Locks, Unlocks, Dict, AltDeps);
+                    prepare_locks(Names, Deps, Locks, Unlocks, Dict, AltDeps, Checkouts);
                 Dep ->
                     {Source, NewLocks, NewUnlocks} = prepare_lock(Dep, Lock, Locks, Dict),
                     prepare_locks(Names, Deps, NewLocks,
-                                  [{Name, Source, 0} | NewUnlocks ++ Unlocks], Dict, AltDeps)
+                                  [{Name, Source, 0} | NewUnlocks ++ Unlocks], Dict, AltDeps, Checkouts)
             end;
         {_, _, Level} = Lock when Level > 0 ->
             case rebar_utils:tup_find(AtomName, Deps) of
@@ -212,14 +216,19 @@ prepare_locks([Name|Names], Deps, Locks, Unlocks, Dict, AltDeps) ->
                 Dep -> % Dep has been promoted
                     {Source, NewLocks, NewUnlocks} = prepare_lock(Dep, Lock, Locks, Dict),
                     prepare_locks(Names, Deps, NewLocks,
-                                  [{Name, Source, 0} | NewUnlocks ++ Unlocks], Dict, AltDeps)
+                                  [{Name, Source, 0} | NewUnlocks ++ Unlocks], Dict, AltDeps, Checkouts)
             end;
         false ->
-            case rebar_utils:tup_find(AtomName, AltDeps) of
+            case lists:member(atom_to_binary(AtomName, utf8), Checkouts) of
+                true ->
+                    ?PRV_ERROR({checkout_dependency, Name});
                 false ->
-                    ?PRV_ERROR({unknown_dependency, Name});
-                _ -> % non-default profile dependency found, pass through
-                    prepare_locks(Names, Deps, Locks, Unlocks, Dict, AltDeps)
+                    case rebar_utils:tup_find(AtomName, AltDeps) of
+                        false ->
+                            ?PRV_ERROR({unknown_dependency, Name});
+                        _ -> % non-default profile dependency found, pass through
+                            prepare_locks(Names, Deps, Locks, Unlocks, Dict, AltDeps, Checkouts)
+                    end
             end
     end.