Merge pull request #2787 from elixir-lang/emj-hex-file-error

Report correct location if local.hex failed

Signed-off-by: José Valim <jose.valim@plataformatec.com.br>

Author: josevalim

lib/mix/lib/mix/generator.ex

@@ -17,7 +17,7 @@ defmodule Mix.Generator do
   def create_file(path, contents, opts \\ []) when is_binary(path) do
     Mix.shell.info [:green, "* creating ", :reset, Path.relative_to_cwd(path)]
 
-    if opts[:force] || overwriting?(path) do
+    if opts[:force] || Mix.Utils.overwriting?(path) do
       File.mkdir_p!(Path.dirname(path))
       File.write!(path, contents)
     end
@@ -31,15 +31,6 @@ defmodule Mix.Generator do
     File.mkdir_p! path
   end
 
-  defp overwriting?(path) do
-    if File.exists?(path) do
-      full = Path.expand(path)
-      Mix.shell.yes?(Path.relative_to_cwd(full) <> " already exists, overwrite?")
-    else
-      true
-    end
-  end
-
   @doc false
   defmacro from_file(path) do
     quote do

lib/mix/lib/mix/tasks/archive.install.ex

@@ -59,8 +59,7 @@ defmodule Mix.Tasks.Archive.Install do
       check_file_exists(archive)
 
       File.mkdir_p!(dest)
-      File.write!(archive, Mix.Utils.read_path!(src, opts))
-      Mix.shell.info [:green, "* creating ", :reset, Path.relative_to_cwd(archive)]
+      Mix.Utils.copy_path!(src, archive, opts)
 
       true = Code.append_path(Mix.Archive.ebin(archive))
     else
@@ -80,7 +79,7 @@ defmodule Mix.Tasks.Archive.Install do
   defp should_install?(_src, previous_files) do
     files = Enum.map_join(previous_files, ", ", &Path.basename/1)
 
-    Mix.shell.yes?("Found existing archives: #{files}.\n" <>
+    Mix.shell.yes?("Found existing archive(s): #{files}.\n" <>
                    "Are you sure you want to replace them?")
   end
 

lib/mix/lib/mix/tasks/local.rebar.ex

@@ -1,8 +1,6 @@
 defmodule Mix.Tasks.Local.Rebar do
   use Mix.Task
 
-  import Mix.Generator, only: [create_file: 3]
-
   @rebar_url "http://s3.hex.pm/rebar"
   @shortdoc  "Install rebar locally"
 
@@ -31,10 +29,10 @@ defmodule Mix.Tasks.Local.Rebar do
   end
 
   defp do_install(path, opts) do
-    rebar = Mix.Utils.read_path!(path)
     local_rebar_path = Mix.Rebar.local_rebar_path
     File.mkdir_p! Path.dirname(local_rebar_path)
-    create_file local_rebar_path, rebar, opts
+
+    Mix.Utils.copy_path!(path, local_rebar_path, opts)
     :ok = :file.change_mode local_rebar_path, 0o755
     true
   end

lib/mix/lib/mix/utils.ex

@@ -354,10 +354,8 @@ defmodule Mix.Utils do
   end
 
   @doc """
-  Opens and reads content from either a URL or a local filesystem path.
-
-  Used by tasks like `archive.install` and `local.rebar` that support
-  installation either from a URL or a local file.
+  Opens and reads content from either a URL or a local filesystem path
+  and returns the contents as a binary.
 
   Raises if the given path is not a URL, nor a file or if the
   file or URL are invalid.
@@ -370,21 +368,74 @@ defmodule Mix.Utils do
   def read_path!(path, opts \\ []) do
     cond do
       url?(path) && opts[:shell] ->
-        read_shell(path)
+        read_shell(path, [])
       url?(path) ->
-        read_httpc(path)
+        read_httpc(path, [])
       file?(path) ->
         read_file(path)
       true ->
         Mix.raise "Expected #{path} to be a url or a local file path"
     end
   end
 
+  @doc """
+  Copies content from either a URL or a local filesystem path to
+  target path.
+
+  Used by tasks like `archive.install` and `local.rebar` that support
+  installation either from a URL or a local file.
+
+  Raises if the given path is not a URL, nor a file or if the
+  file or URL are invalid.
+
+  ## Options
+
+    * `:shell` - Forces the use of `wget` or `curl` to fetch the file if the
+      given path is a URL.
+
+    * `:force` - Forces overwriting target file without a shell prompt.
+  """
+  def copy_path!(source, target, opts \\ []) do
+    if opts[:force] || overwriting?(target) do
+      cond do
+        url?(source) && opts[:shell] ->
+          read_shell(source, file: target)
+        url?(source) ->
+          read_httpc(source, file: target)
+        file?(source) ->
+          copy_file(source, target)
+        true ->
+          Mix.raise "Expected #{source} to be a url or a local file path"
+      end
+
+      put_creating_file(target)
+    end
+
+    :ok
+  end
+
+  @doc """
+  Prompts the user to overwrite the file if it exists. Returns
+  the user input.
+  """
+  def overwriting?(path) do
+    if File.exists?(path) do
+      full = Path.expand(path)
+      Mix.shell.yes?(Path.relative_to_cwd(full) <> " already exists, overwrite?")
+    else
+      true
+    end
+  end
+
   defp read_file(path) do
     File.read!(path)
   end
 
-  defp read_httpc(path) do
+  defp copy_file(source, target) do
+    File.cp!(source, target)
+  end
+
+  defp read_httpc(path, opts) do
     {:ok, _} = Application.ensure_all_started(:ssl)
     {:ok, _} = Application.ensure_all_started(:inets)
 
@@ -401,11 +452,20 @@ defmodule Mix.Utils do
     if http_proxy,  do: proxy(http_proxy)
     if https_proxy, do: proxy(https_proxy)
 
-    # We are using relaxed: true because some clients is returning a Location
+    if out_path = opts[:file] do
+      File.rm(out_path)
+      req_opts = [stream: String.to_char_list(out_path)]
+    else
+      req_opts = [body_format: :binary]
+    end
+
+    # We are using relaxed: true because some servers is returning a Location
     # header with relative paths, which does not follow the spec. This would
     # cause the request to fail with {:error, :no_scheme} unless :relaxed
     # is given.
-    case :httpc.request(:get, request, [relaxed: true], [body_format: :binary], :mix) do
+    case :httpc.request(:get, request, [relaxed: true], req_opts, :mix) do
+      {:ok, :saved_to_file} ->
+        :ok
       {:ok, {{_, status, _}, _, body}} when status in 200..299 ->
         body
       {:ok, {{_, status, _}, _, _}} ->
@@ -430,9 +490,10 @@ defmodule Mix.Utils do
     end
   end
 
-  defp read_shell(path) do
+  defp read_shell(path, opts) do
     filename = URI.parse(path).path |> Path.basename
-    out_path = Path.join(System.tmp_dir!, filename)
+    out_path = opts[:file] || Path.join(System.tmp_dir!, filename)
+
     File.rm(out_path)
 
     status = cond do
@@ -450,14 +511,20 @@ defmodule Mix.Utils do
         1
     end
 
-    check_command!(status, path, out_path)
+    check_command!(status, path, opts[:file])
 
-    data = File.read!(out_path)
-    File.rm!(out_path)
-    data
+    unless opts[:file] do
+      data = File.read!(out_path)
+      File.rm!(out_path)
+      data
+    end
   end
 
   defp check_command!(0, _path, _out_path), do: :ok
+  defp check_command!(_status, path, nil) do
+    Mix.raise "Could not fetch data, please download manually from " <>
+              "#{inspect path}"
+  end
   defp check_command!(_status, path, out_path) do
     Mix.raise "Could not fetch data, please download manually from " <>
               "#{inspect path} and copy it to #{inspect out_path}"
@@ -467,6 +534,10 @@ defmodule Mix.Utils do
     match?({:win32, _}, :os.type)
   end
 
+  defp put_creating_file(path) do
+    Mix.shell.info [:green, "* creating ", :reset, Path.relative_to_cwd(path)]
+  end
+
   defp file?(path) do
     File.regular?(path)
   end