Share consulting logic between compile.app and release

Author: josevalim

lib/elixir/lib/regex.ex

@@ -258,7 +258,7 @@ defmodule Regex do
   This checks the version stored in the regular expression
   and recompiles the regex in case of version mismatch.
   """
-  # Remove me on Elixir v1.22
+  # TODO: Deprecate on Elixir v1.22
   @doc deprecated: "It can be removed and it has no effect"
   @doc since: "1.4.0"
   def recompile(%Regex{} = regex) do
@@ -268,7 +268,7 @@ defmodule Regex do
   @doc """
   Recompiles the existing regular expression and raises `Regex.CompileError` in case of errors.
   """
-  # Remove me on Elixir v1.22
+  # TODO: Deprecate on Elixir v1.22
   @doc deprecated: "It can be removed and it has no effect"
   @doc since: "1.4.0"
   def recompile!(regex) do
@@ -278,7 +278,7 @@ defmodule Regex do
   @doc """
   Returns the version of the underlying Regex engine.
   """
-  # Remove me on Elixir v1.22
+  # TODO: Deprecate on Elixir v1.22
   @doc deprecated: "Use :re.version() instead"
   @doc since: "1.4.0"
   def version do

lib/mix/lib/mix/release.ex

@@ -468,51 +468,48 @@ defmodule Mix.Release do
     {sys_config, runtime_config?} =
       merge_provider_config(release, sys_config, config_provider_path)
 
-    path = Path.join(release.version_path, "sys.config")
-
-    args = [runtime_config?, sys_config]
-    format = "%% coding: utf-8~n%% RUNTIME_CONFIG=~s~n~tw.~n"
-    File.mkdir_p!(Path.dirname(path))
-    File.write!(path, IO.chardata_to_string(:io_lib.format(format, args)))
-
-    case :file.consult(path) do
-      {:ok, _} ->
+    case consultable_sys_config(sys_config) do
+      {:ok, contents} ->
+        path = Path.join(release.version_path, "sys.config")
+        args = [runtime_config?, contents]
+        format = "%% coding: utf-8~n%% RUNTIME_CONFIG=~s~n~ts.~n"
+        File.mkdir_p!(Path.dirname(path))
+        File.write!(path, IO.chardata_to_string(:io_lib.format(format, args)))
         :ok
 
-      {:error, reason} ->
-        invalid =
-          for {app, kv} <- sys_config,
-              {key, value} <- kv,
-              not valid_config?(value),
-              do: """
-
-              Application: #{inspect(app)}
-              Key: #{inspect(key)}
-              Value: #{inspect(value)}
-              """
-
-        message =
-          case invalid do
-            [] ->
-              "Could not read configuration file. Reason: #{inspect(reason)}"
-
-            _ ->
-              "Could not read configuration file. It has invalid configuration terms " <>
-                "such as functions, references, and pids. Please make sure your configuration " <>
-                "is made of numbers, atoms, strings, maps, tuples and lists. The following entries " <>
-                "are wrong:\n#{Enum.join(invalid)}"
-          end
-
+      {:error, message} ->
         {:error, message}
     end
   end
 
-  defp valid_config?(m) when is_map(m),
-    do: Enum.all?(Map.delete(m, :__struct__), &valid_config?/1)
+  defp consultable_sys_config(sys_config) do
+    contents =
+      Enum.map_intersperse(sys_config, ?,, fn {app, kv} ->
+        kv =
+          Enum.map_intersperse(kv, ?,, fn {key, value} ->
+            case Mix.Utils.consultable(value) do
+              {:ok, value} -> [?{, :io_lib.print(key), ?,, value, ?}]
+              {:error, term, reason} -> throw({:error, app, key, term, reason})
+            end
+          end)
 
-  defp valid_config?(l) when is_list(l), do: Enum.all?(l, &valid_config?/1)
-  defp valid_config?(t) when is_tuple(t), do: Enum.all?(Tuple.to_list(t), &valid_config?/1)
-  defp valid_config?(o), do: is_number(o) or is_atom(o) or is_binary(o)
+        [?{, :io_lib.print(app), ?,, ?[, kv, ?], ?}]
+      end)
+
+    {:ok, [?[, contents, ?]]}
+  catch
+    {:error, app, key, value, reason} ->
+      message = """
+      Could not write configuration file because it has invalid terms
+
+      Application: #{inspect(app)}
+      Key: #{inspect(key)}
+      Invalid value: #{inspect(value)}
+      Reason: #{reason}
+      """
+
+      {:error, message}
+  end
 
   defp merge_provider_config(%{config_providers: []}, sys_config, _), do: {sys_config, false}
 

lib/mix/lib/mix/tasks/compile.app.ex

@@ -187,7 +187,17 @@ defmodule Mix.Tasks.Compile.App do
         |> add_compile_env(current_properties)
         |> add_modules(modules, compile_path)
 
-      contents = to_erl_term({:application, app, properties})
+      contents =
+        case Mix.Utils.consultable({:application, app, properties}) do
+          {:ok, contents} ->
+            contents
+
+          {:error, term, reason} ->
+            Mix.raise(
+              "\"def application\" has a term which cannot be written to .app files: #{inspect(term)} (#{reason})"
+            )
+        end
+
       :application.unload(app)
       :application.load({:application, app, properties})
 
@@ -209,78 +219,6 @@ defmodule Mix.Tasks.Compile.App do
     end
   end
 
-  defp to_erl_term(tuple) when is_tuple(tuple) do
-    [?{, tuple |> Tuple.to_list() |> to_erl_head(), ?}]
-  end
-
-  defp to_erl_term(list) when is_list(list) do
-    if List.ascii_printable?(list) do
-      :io_lib.print(list)
-    else
-      [?[, to_erl_head(list), ?]]
-    end
-  end
-
-  defp to_erl_term(%Regex{re_pattern: {:re_pattern, _, _, _, ref}} = regex)
-       when is_reference(ref) do
-    Mix.raise("""
-    \"def application\" has a term which cannot be written to .app files: #{inspect(regex)}.
-    Use the E modifier to store regexes in application config.
-    """)
-  end
-
-  defp to_erl_term(map) when is_map(map) do
-    inner =
-      Enum.map_intersperse(
-        :maps.to_list(:maps.iterator(map, :reversed)),
-        ?,,
-        fn {key, value} -> [to_erl_term(key), "=>", to_erl_term(value)] end
-      )
-
-    [?#, ?{, inner, ?}]
-  end
-
-  defp to_erl_term(map) when is_map(map) do
-    inner =
-      Enum.map_intersperse(
-        :maps.to_list(:maps.iterator(map, :reversed)),
-        ?,,
-        fn {key, value} -> [to_erl_term(key), "=>", to_erl_term(value)] end
-      )
-
-    [?#, ?{, inner, ?}]
-  end
-
-  defp to_erl_term(function) when is_function(function) do
-    fun_info = Function.info(function)
-
-    if fun_info[:type] == :external and fun_info[:env] == [] do
-      :io_lib.print(function)
-    else
-      Mix.raise(
-        "\"def application\" has a function which cannot be written to .app files: #{inspect(function)}" <>
-          " (only functions in the form &Mod.fun/arity can be part of the application environment)"
-      )
-    end
-  end
-
-  defp to_erl_term(term) when is_reference(term) or is_pid(term) do
-    Mix.raise(
-      "\"def application\" has a term which cannot be written to .app files: #{inspect(term)}"
-    )
-  end
-
-  defp to_erl_term(term) do
-    :io_lib.print(term)
-  end
-
-  defp to_erl_head([]), do: []
-  defp to_erl_head([h | t]), do: [to_erl_term(h) | to_erl_tail(t)]
-
-  defp to_erl_tail([h | t]), do: [?,, to_erl_term(h) | to_erl_tail(t)]
-  defp to_erl_tail([]), do: []
-  defp to_erl_tail(other), do: [?|, to_erl_term(other)]
-
   defp current_app_properties(target) do
     case :file.consult(target) do
       {:ok, [{:application, _app, properties}]} -> properties

lib/mix/lib/mix/utils.ex

@@ -907,4 +907,77 @@ defmodule Mix.Utils do
         uid
     end
   end
+
+  @doc """
+  Convert the given terms to a consultable Erlang term.
+  """
+  def consultable(term) do
+    {:ok, to_erl_term(term)}
+  catch
+    {:error, term, reason} -> {:error, term, reason}
+  end
+
+  defp to_erl_term(tuple) when is_tuple(tuple) do
+    [?{, tuple |> Tuple.to_list() |> to_erl_head(), ?}]
+  end
+
+  defp to_erl_term(list) when is_list(list) do
+    if List.ascii_printable?(list) do
+      :io_lib.print(list)
+    else
+      [?[, to_erl_head(list), ?]]
+    end
+  end
+
+  defp to_erl_term(%Regex{re_pattern: {:re_pattern, _, _, _, ref}} = regex)
+       when is_reference(ref) do
+    throw({:error, regex, "you must use the /E modifier to store regexes"})
+  end
+
+  defp to_erl_term(map) when is_map(map) do
+    inner =
+      Enum.map_intersperse(
+        :maps.to_list(:maps.iterator(map, :reversed)),
+        ?,,
+        fn {key, value} -> [to_erl_term(key), "=>", to_erl_term(value)] end
+      )
+
+    [?#, ?{, inner, ?}]
+  end
+
+  defp to_erl_term(map) when is_map(map) do
+    inner =
+      Enum.map_intersperse(
+        :maps.to_list(:maps.iterator(map, :reversed)),
+        ?,,
+        fn {key, value} -> [to_erl_term(key), "=>", to_erl_term(value)] end
+      )
+
+    [?#, ?{, inner, ?}]
+  end
+
+  defp to_erl_term(function) when is_function(function) do
+    fun_info = Function.info(function)
+
+    if fun_info[:type] == :external and fun_info[:env] == [] do
+      :io_lib.print(function)
+    else
+      throw({:error, function, "only functions in the form &Mod.fun/arity are allowed"})
+    end
+  end
+
+  defp to_erl_term(term) when is_reference(term) or is_pid(term) do
+    throw({:error, term, "PIDs and References are not supported"})
+  end
+
+  defp to_erl_term(term) do
+    :io_lib.print(term)
+  end
+
+  defp to_erl_head([]), do: []
+  defp to_erl_head([h | t]), do: [to_erl_term(h) | to_erl_tail(t)]
+
+  defp to_erl_tail([h | t]), do: [?,, to_erl_term(h) | to_erl_tail(t)]
+  defp to_erl_tail([]), do: []
+  defp to_erl_tail(other), do: [?|, to_erl_term(other)]
 end

lib/mix/test/mix/release_test.exs

@@ -592,19 +592,20 @@ defmodule Mix.ReleaseTest do
       assert make_sys_config(release([]), [foo: [bar: :baz]], "unused/runtime/path") == :ok
       contents = File.read!(@sys_config)
       assert contents =~ "%% RUNTIME_CONFIG=false"
-      assert contents =~ "[{foo,[{bar,baz}]}]."
+      {:ok, contents} = :file.consult(@sys_config)
+      assert contents == [[foo: [bar: :baz]]]
     end
 
     test "writes sys_config with encoding" do
       assert make_sys_config(
                release([]),
-               [encoding: {:_μ, :"£", "£", ~c"£"}],
+               [encoding: [key: {:_μ, :"£", "£", ~c"£"}]],
                "unused/runtime/path"
              ) ==
                :ok
 
       {:ok, contents} = :file.consult(@sys_config)
-      assert contents == [[encoding: {:_μ, :"£", "£", ~c"£"}]]
+      assert contents == [[encoding: [key: {:_μ, :"£", "£", ~c"£"}]]]
     end
 
     test "writes the given sys_config with config providers" do
@@ -649,12 +650,12 @@ defmodule Mix.ReleaseTest do
     end
 
     test "errors on bad config" do
-      assert {:error, "Could not read configuration file." <> _} =
+      assert {:error, "Could not write configuration file " <> _} =
                make_sys_config(release([]), [foo: [bar: self()]], "unused/runtime/path")
 
       env = %{__ENV__ | lexical_tracker: self()}
 
-      assert {:error, "Could not read configuration file." <> _} =
+      assert {:error, "Could not write configuration file " <> _} =
                make_sys_config(release([]), [foo: [bar: env]], "unused/runtime/path")
     end
   end

lib/mix/test/mix/tasks/compile.app_test.exs

@@ -267,25 +267,17 @@ defmodule Mix.Tasks.Compile.AppTest do
   test "accepts only regexes without a reference" do
     in_fixture("no_mixfile", fn ->
       Mix.Project.push(CustomProject)
-
       Process.put(:application, env: [regex: ~r/foo/])
 
-      message = """
-      "def application" has a term which cannot be written to \.app files: ~r\/foo\/.
-      Use the E modifier to store regexes in application config.
-      """
-
-      assert_raise Mix.Error, message, fn ->
+      assert_raise Mix.Error, ~r/you must use the \/E modifier to store regexes/, fn ->
         Mix.Tasks.Compile.App.run([])
       end
 
       Process.put(:application, env: [exported: ~r/foo/E])
-
       Mix.Tasks.Compile.Elixir.run([])
       Mix.Tasks.Compile.App.run([])
 
       properties = parse_resource_file(:custom_project)
-
       assert properties[:env] == [exported: ~r/foo/E]
     end)
   end