Leave it to the application to start Logger after compilation

Closes #2649

Author: José Valim

lib/logger/lib/logger/app.ex

@@ -3,6 +3,7 @@ defmodule Logger.App do
 
   use Application
 
+  @doc false
   def start(_type, _args) do
     import Supervisor.Spec
 
@@ -31,6 +32,7 @@ defmodule Logger.App do
     end
   end
 
+  @doc false
   def stop(_) do
     Application.get_env(:logger, :deleted_handlers)
     |> Enum.each(&:error_logger.add_report_handler/1)
@@ -42,6 +44,16 @@ defmodule Logger.App do
     :ok
   end
 
+  @doc """
+  Stops the application without sending messages to error logger.
+  """
+  def stop() do
+    set = Application.get_env(:logger, :deleted_handlers)
+    Application.put_env(:logger, :deleted_handlers, HashSet.new)
+    Application.stop(:logger)
+    Enum.each(set, &:error_logger.add_report_handler/1)
+  end
+
   defp store_deleted_handlers(list) do
     Application.put_env(:logger, :deleted_handlers, Enum.into(list, HashSet.new))
   end

lib/logger/lib/logger/config.ex

@@ -51,14 +51,6 @@ defmodule Logger.Config do
     Application.delete_env(:logger, @data)
   end
 
-  def restart do
-    set = Application.get_env(:logger, :deleted_handlers)
-    Application.put_env(:logger, :deleted_handlers, HashSet.new)
-    Application.stop(:logger)
-    Enum.each(set, &:error_logger.add_report_handler/1)
-    Application.start(:logger)
-  end
-
   ## Callbacks
 
   def init(_) do

lib/logger/test/logger/translator_test.exs

@@ -34,13 +34,18 @@ defmodule Logger.TranslatorTest do
   setup_all do
     sasl_reports? = Application.get_env(:logger, :handle_sasl_reports, false)
     Application.put_env(:logger, :handle_sasl_reports, true)
+
+    # Restart the app but change the level before to avoid warnings
     level = Logger.level()
-    Logger.configure([level: :error])
-    Logger.Config.restart()
-    Logger.configure([level: level])
+    Logger.configure(level: :error)
+    Logger.App.stop()
+    Application.start(:logger)
+    Logger.configure(level: level)
+
     on_exit(fn() ->
       Application.put_env(:logger, :handle_sasl_reports, sasl_reports?)
-      Logger.Config.restart()
+      Logger.App.stop()
+      Application.start(:logger)
     end)
   end
 

lib/logger/test/logger_test.exs

@@ -170,16 +170,10 @@ defmodule LoggerTest do
     end
   end
 
-  test "Logger.Config survives Logger exit" do
-    Process.whereis(Logger) |> Process.exit(:kill)
-    wait_for_logger()
-    wait_for_handler(Logger, Logger.Config)
-    wait_for_handler(:error_logger, Logger.ErrorHandler)
-  end
-
-  test "Logger.Config can restart the application" do
+  test "stop the application silently" do
     Application.put_env(:logger, :backends, [])
-    Logger.Config.restart()
+    Logger.App.stop()
+    Application.start(:logger)
 
     assert capture_log(fn ->
       assert Logger.debug("hello", []) == :ok
@@ -190,6 +184,14 @@ defmodule LoggerTest do
            {:error, {:already_started, pid}}
   after
     Application.put_env(:logger, :backends, [:console])
-    Logger.Config.restart()
+    Logger.App.stop()
+    Application.start(:logger)
+  end
+
+  test "restarts Logger.Config on Logger exits" do
+    Process.whereis(Logger) |> Process.exit(:kill)
+    wait_for_logger()
+    wait_for_handler(Logger, Logger.Config)
+    wait_for_handler(:error_logger, Logger.ErrorHandler)
   end
 end

lib/mix/lib/mix/tasks/app.start.ex

@@ -25,6 +25,11 @@ defmodule Mix.Tasks.App.Start do
       Mix.Task.run "compile", ["--no-readd"|args]
     end
 
+    # Stop the Logger after we have used it for compilation.
+    # It is up to the application to decide if it should be
+    # restarted or not.
+    Logger.App.stop()
+
     unless "--no-start" in args do
       start(Mix.Project.config[:app])
     end

lib/mix/lib/mix/tasks/loadconfig.ex

@@ -27,15 +27,7 @@ defmodule Mix.Tasks.Loadconfig do
   end
 
   defp load(file) do
-    opts = Mix.Config.read!(file)
-    Mix.Config.persist(opts)
-
-    # In case the Logger was changed, we reload the whole
-    # Logger. This is required because many of Logger config
-    # reflects on its supervision tree. No other application
-    # that is bundled with Elixir requires such reloading.
-    if opts[:logger], do: Logger.Config.restart
-
+    Mix.Config.persist Mix.Config.read!(file)
     :ok
   end
 end

lib/mix/test/mix/tasks/app.start_test.exs

@@ -7,6 +7,10 @@ defmodule Mix.Tasks.App.StartTest do
     def project do
       [app: :app_start_sample, version: "0.1.0"]
     end
+
+    def application do
+      [applications: [:logger]]
+    end
   end
 
   defmodule WrongElixirProject do
@@ -22,39 +26,23 @@ defmodule Mix.Tasks.App.StartTest do
   end
 
   setup config do
+    on_exit fn ->
+      Application.start(:logger)
+    end
+
     if app = config[:app] do
       Logger.remove_backend(:console)
 
       on_exit fn ->
-        :application.stop(app)
-        :application.unload(app)
+        Application.stop(app)
+        Application.unload(app)
         Logger.add_backend(:console, flush: true)
       end
     end
 
     :ok
   end
 
-  test "recompiles project if elixir version changed" do
-    Mix.Project.push MixTest.Case.Sample
-
-    in_fixture "no_mixfile", fn ->
-      Mix.Tasks.Compile.run []
-      purge [A, B, C]
-
-      assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
-      assert System.version == Mix.Dep.Lock.elixir_vsn
-
-      Mix.Task.clear
-      File.write!("_build/dev/lib/sample/.compile.lock", "the_past")
-      File.touch!("_build/dev/lib/sample/.compile.lock", {{2010, 1, 1}, {0, 0, 0}})
-
-      Mix.Tasks.App.Start.run ["--no-start"]
-      assert System.version == Mix.Dep.Lock.elixir_vsn
-      assert File.stat!("_build/dev/lib/sample/.compile.lock").mtime > {{2010, 1, 1}, {0, 0, 0}}
-    end
-  end
-
   test "compiles and starts the project" do
     Mix.Project.push AppStartSample
 
@@ -66,10 +54,13 @@ defmodule Mix.Tasks.App.StartTest do
       Mix.Tasks.App.Start.run ["--no-start"]
       assert File.regular?("_build/dev/lib/app_start_sample/ebin/Elixir.A.beam")
       assert File.regular?("_build/dev/lib/app_start_sample/ebin/app_start_sample.app")
-      refute List.keyfind(:application.loaded_applications, :app_start_sample, 0)
+
+      refute List.keyfind(:application.which_applications, :app_start_sample, 0)
+      refute List.keyfind(:application.which_applications, :logger, 0)
 
       Mix.Tasks.App.Start.run []
-      assert List.keyfind(:application.loaded_applications, :app_start_sample, 0)
+      assert List.keyfind(:application.which_applications, :app_start_sample, 0)
+      assert List.keyfind(:application.which_applications, :logger, 0)
     end
   end
 

lib/mix/test/mix/tasks/compile_test.exs

@@ -56,4 +56,22 @@ defmodule Mix.Tasks.CompileTest do
       refute File.regular?("ebin/Elixir.C.beam")
     end
   end
+
+  test "recompiles project if elixir version changed" do
+    in_fixture "no_mixfile", fn ->
+      Mix.Tasks.Compile.run []
+      purge [A, B, C]
+
+      assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
+      assert System.version == Mix.Dep.Lock.elixir_vsn
+
+      Mix.Task.clear
+      File.write!("_build/dev/lib/sample/.compile.lock", "the_past")
+      File.touch!("_build/dev/lib/sample/.compile.lock", {{2010, 1, 1}, {0, 0, 0}})
+
+      Mix.Tasks.Compile.run []
+      assert System.version == Mix.Dep.Lock.elixir_vsn
+      assert File.stat!("_build/dev/lib/sample/.compile.lock").mtime > {{2010, 1, 1}, {0, 0, 0}}
+    end
+  end
 end

lib/mix/test/mix/tasks/loadconfig_test.exs

@@ -21,8 +21,7 @@ defmodule Mix.Tasks.LoadconfigTest do
 
     in_fixture "no_mixfile", fn ->
       write_config """
-      [my_app: [key: :project],
-       logger: [level: :error]]
+      [my_app: [key: :project]]
       """
 
       # Original Logger level
@@ -36,9 +35,6 @@ defmodule Mix.Tasks.LoadconfigTest do
       :ok = :application.load({:application, :my_app, [vsn: '1.0.0', env: [key: :app]]})
       assert Application.fetch_env(:my_app, :key) == {:ok, :project}
 
-      # Logger is reloaded
-      assert Logger.level == :error
-
       # laodconfig can be called multiple times
       # Later values should have higher precedence
       Mix.Task.run "loadconfig", [fixture_path("configs/good_config.exs")]