Merge pull request #204 from tsloughter/git_hg_cmd

use rebar_utils:sh for git and hg commands to have better errors and log

3 files changed, 66 insertions, 23 deletions

diff --git a/src/rebar_git_resource.erl b/src/rebar_git_resource.erl
index 785c362..7626772 100644
--- a/src/rebar_git_resource.erl
+++ b/src/rebar_git_resource.erl
@@ -14,9 +14,11 @@
 lock(AppDir, {git, Url, _}) ->
     lock(AppDir, {git, Url});
 lock(AppDir, {git, Url}) ->
-    Ref = string:strip(
-            os:cmd("git --git-dir='" ++ AppDir ++ "/.git' rev-parse --verify HEAD")
-            ,both, $\n),
+    AbortMsg = io_lib:format("Locking of git dependency failed in ~s", [AppDir]),
+    {ok, VsnString} =
+        rebar_utils:sh("git --git-dir='" ++ AppDir ++ "/.git' rev-parse --verify HEAD",
+                       [{use_stdout, false}, {debug_abort_on_error, AbortMsg}]),
+    Ref = string:strip(VsnString, both, $\n),
     {git, Url, {ref, Ref}}.
 
 %% Return true if either the git url or tag/branch/ref is not the same as the currently
@@ -118,13 +120,21 @@ make_vsn(Dir) ->
 collect_default_refcount() ->
     %% Get the tag timestamp and minimal ref from the system. The
     %% timestamp is really important from an ordering perspective.
-    RawRef = os:cmd("git log -n 1 --pretty=format:'%h\n' "),
+    AbortMsg1 = "Gtting log of git dependency failed in " ++ rebar_dir:get_cwd(),
+    {ok, String} =
+        rebar_utils:sh("git log -n 1 --pretty=format:'%h\n' ",
+                       [{use_stdout, false},
+                        {debug_abort_on_error, AbortMsg1}]),
+    RawRef = string:strip(String, both, $\n),
 
     {Tag, TagVsn} = parse_tags(),
-    RawCount =
+    {ok, RawCount} =
         case Tag of
             undefined ->
-                os:cmd("git rev-list HEAD | wc -l");
+                AbortMsg2 = "Getting rev-list of git depedency failed in " ++ rebar_dir:get_cwd(),
+                rebar_utils:sh("git rev-list HEAD | wc -l",
+                               [{use_stdout, false},
+                                {debug_abort_on_error, AbortMsg2}]);
             _ ->
                 get_patch_count(Tag)
         end,
@@ -146,18 +156,25 @@ build_vsn_string(Vsn, RawRef, RawCount) ->
     end.
 
 get_patch_count(RawRef) ->
+    AbortMsg = "Getting rev-list of git dep failed in " ++ rebar_dir:get_cwd(),
     Ref = re:replace(RawRef, "\\s", "", [global]),
     Cmd = io_lib:format("git rev-list ~s..HEAD | wc -l",
                          [Ref]),
-    os:cmd(Cmd).
+    rebar_utils:sh(Cmd,
+                   [{use_stdout, false},
+                    {debug_abort_on_error, AbortMsg}]).
 
 parse_tags() ->
-    first_valid_tag(os:cmd("git log --oneline --decorate  | fgrep \"tag: \" -1000")).
-
-first_valid_tag(Line) ->
-    case re:run(Line, "(\\(|\\s)tag:\\s(v([^,\\)]+))", [{capture, [2, 3], list}]) of
-        {match,[Tag, Vsn]} ->
-            {Tag, Vsn};
-        nomatch ->
-            {undefined, "0.0.0"}
+    %% Don't abort on error, we want the bad return to be turned into 0.0.0
+    case rebar_utils:sh("git log --oneline --decorate  | fgrep \"tag: \" -1000",
+                        [{use_stdout, false}, return_on_error]) of
+        {error, _} ->
+            {undefined, "0.0.0"};
+        {ok, Line} ->
+            case re:run(Line, "(\\(|\\s)tag:\\s(v([^,\\)]+))", [{capture, [2, 3], list}]) of
+                {match,[Tag, Vsn]} ->
+                    {Tag, Vsn};
+                nomatch ->
+                    {undefined, "0.0.0"}
+            end
     end.
diff --git a/src/rebar_hg_resource.erl b/src/rebar_hg_resource.erl
index a0ebb6d..1bd992e 100644
--- a/src/rebar_hg_resource.erl
+++ b/src/rebar_hg_resource.erl
@@ -80,7 +80,12 @@ make_vsn(Dir) ->
     Ref = get_ref(Dir),
     Cmd = BaseHg ++ "log --template \"{latesttag}+build.{latesttagdistance}.rev.{node|short}\""
           " --rev " ++ Ref,
-    RawVsn = string:strip(os:cmd(Cmd), both, $\n),
+    AbortMsg = io_lib:format("Version resolution of hg dependency failed in ~s", [Dir]),
+    {ok, VsnString} =
+        rebar_utils:sh(Cmd,
+                       [{use_stdout, false}, {debug_abort_on_error, AbortMsg}]),
+    RawVsn = string:strip(VsnString, both, $\n),
+
     Vsn = case RawVsn of
         "null+" ++ Rest -> "0.0.0+" ++ Rest;
         _ -> RawVsn
@@ -95,20 +100,30 @@ compare_url(Dir, Url) ->
     parse_hg_url(CurrentUrl1) =:= parse_hg_url(Url).
 
 get_ref(Dir) ->
-    string:strip(os:cmd("hg -R '" ++ Dir ++ "' --debug id -i"), both, $\n).
+    AbortMsg = io_lib:format("Get ref of hg dependency failed in ~s", [Dir]),
+    {ok, RefString} =
+        rebar_utils:sh("hg -R '" ++ Dir ++ "' --debug id -i",
+                       [{use_stdout, false}, {debug_abort_on_error, AbortMsg}]),
+    string:strip(RefString, both, $\n).
 
 get_tag_distance(Dir, Ref) ->
-    Log = string:strip(os:cmd("hg -R '" ++ Dir ++ "' "
-                              "log --template \"{latesttag}-{latesttagdistance}\n\" "
-                              "--rev " ++ Ref),
+    AbortMsg = io_lib:format("Get tag distance of hg dependency failed in ~s", [Dir]),
+    {ok, LogString} =
+        rebar_utils:sh("hg -R '" ++ Dir ++ "' "
+                      "log --template \"{latesttag}-{latesttagdistance}\n\" "
+                      "--rev " ++ Ref,
+                      [{use_stdout, false}, {debug_abort_on_error, AbortMsg}]),
+    Log = string:strip(LogString,
                        both, $\n),
     [Tag, Distance] = re:split(Log, "-([0-9]+)$", [{parts,0}]),
     {Tag, Distance}.
 
 get_branch_ref(Dir, Branch) ->
-    string:strip(
-        os:cmd("hg -R '" ++ Dir ++ "' log --template \"{node}\n\" --rev " ++ Branch),
-        both, $\n).
+    AbortMsg = io_lib:format("Get branch ref of hg dependency failed in ~s", [Dir]),
+    {ok, BranchRefString} =
+        rebar_utils:sh("hg -R '" ++ Dir ++ "' log --template \"{node}\n\" --rev " ++ Branch,
+                       [{use_stdout, false}, {debug_abort_on_error, AbortMsg}]),
+    string:strip(BranchRefString, both, $\n).
 
 parse_hg_url("ssh://" ++ HostPath) ->
     [Host | Path] = string:tokens(HostPath, "/"),
diff --git a/src/rebar_utils.erl b/src/rebar_utils.erl
index 1a7bf98..5bb0e63 100644
--- a/src/rebar_utils.erl
+++ b/src/rebar_utils.erl
@@ -308,6 +308,9 @@ expand_sh_flag(abort_on_error) ->
 expand_sh_flag({abort_on_error, Message}) ->
     {error_handler,
      log_msg_and_abort(Message)};
+expand_sh_flag({debug_abort_on_error, Message}) ->
+    {error_handler,
+     debug_log_msg_and_abort(Message)};
 expand_sh_flag(debug_and_abort_on_error) ->
     {error_handler,
      fun debug_and_abort/2};
@@ -335,6 +338,14 @@ log_msg_and_abort(Message) ->
             ?ABORT(Message, [])
     end.
 
+debug_log_msg_and_abort(Message) ->
+    fun(Command, {Rc, Output}) ->
+            ?DEBUG("sh(~s)~n"
+                  "failed with return code ~w and the following output:~n"
+                  "~s", [Command, Rc, Output]),
+            ?ABORT(Message, [])
+    end.
+
 -spec log_and_abort(string(), {integer(), string()}) -> no_return().
 log_and_abort(Command, {Rc, Output}) ->
     ?ABORT("sh(~s)~n"