Move recompile-free extraction to its own gettext.extract_from_attributes task

Following Jose's review on PR #437, split the attribute-based extraction out
of the --from-attributes flag into a dedicated mix gettext.extract_from_attributes
task. The two extraction modes are now separate code paths that share only the
POT merge/fuse and output steps via Mix.Tasks.Gettext.Extract.process/3.

Also drop the incremental_compile/0 reenable dance. The new task does a plain
mix compile (a no-op when the project is already compiled), since by the time it
reads the persisted attributes the project is already compiled and up to date.

Author: oliver-kriska

CHANGELOG.md

@@ -4,9 +4,9 @@
 
   * Parallelize per-locale merging in `mix gettext.merge`.
 
-  * Add experimental `--from-attributes` flag to `mix gettext.extract`: messages
-    are persisted as module attributes during normal compilation (for backends
-    with `automatic_extraction: true` in the application environment, for example
+  * Add experimental `mix gettext.extract_from_attributes` task: messages are
+    persisted as module attributes during normal compilation (for backends with
+    `automatic_extraction: true` in the application environment, for example
     `config :gettext, MyApp.Gettext, automatic_extraction: true` in
     `config/dev.exs`) and read back from the compiled BEAM files, so extraction
     no longer needs to force-recompile the project.

lib/mix/tasks/gettext.extract.ex

@@ -57,43 +57,30 @@ defmodule Mix.Tasks.Gettext.Extract do
 
   ## Extraction Without Recompiling (Experimental)
 
-  By default, this task extracts messages by **force-recompiling** the whole
-  project, because extraction happens during the expansion of the Gettext
-  macros. As an experimental alternative, the `--from-attributes` flag reads
-  messages back from the compiled BEAM files instead:
-
-  ```bash
-  mix gettext.extract --from-attributes
-  ```
-
-  For this to work, messages must have been persisted as module attributes
-  during normal compilation. This happens automatically when the backend has
-  automatic extraction enabled in the application environment, which you
-  typically set in `config/dev.exs` so it stays off in `:prod`:
-
-      # config/dev.exs
-      config :gettext, MyApp.Gettext, automatic_extraction: true
-
-  With this flag, the task only runs a normal **incremental** compilation
-  (changed files are recompiled and get fresh attributes; unchanged BEAM
-  files already carry theirs), then scans the compiled BEAM files. Since the
-  attributes are only persisted when `automatic_extraction` is enabled (so not
-  in `:prod`), release artifacts are unaffected.
-
-  `--from-attributes` can be combined with `--merge` and `--check-up-to-date`.
+  This task always **force-recompiles** the whole project, because extraction
+  happens during the expansion of the Gettext macros. If you would rather
+  extract without a force-recompile, see `mix gettext.extract_from_attributes`,
+  which reads the messages back from the compiled BEAM files instead.
 
   """
 
-  @switches [merge: :boolean, check_up_to_date: :boolean, from_attributes: :boolean]
+  @switches [merge: :boolean, check_up_to_date: :boolean]
 
   @impl true
   def run(args) do
     Application.ensure_all_started(:gettext)
     _ = Mix.Project.get!()
     mix_config = Mix.Project.config()
     {opts, _} = OptionParser.parse!(args, switches: @switches)
-    pot_files = extract(mix_config[:app], mix_config[:gettext] || [], opts)
+    pot_files = extract_via_recompilation(mix_config[:app], mix_config[:gettext] || [])
+    process(pot_files, opts, args)
+  end
 
+  # Shared by `mix gettext.extract` and `mix gettext.extract_from_attributes`:
+  # both compute `pot_files` (their only difference) and then write, check, or
+  # merge them in exactly the same way.
+  @doc false
+  def process(pot_files, opts, args) do
     if opts[:check_up_to_date] do
       run_up_to_date_check(pot_files)
     else
@@ -130,14 +117,6 @@ defmodule Mix.Tasks.Gettext.Extract do
     end
   end
 
-  defp extract(app, gettext_config, opts) do
-    if opts[:from_attributes] do
-      extract_from_attributes(app, gettext_config)
-    else
-      extract_via_recompilation(app, gettext_config)
-    end
-  end
-
   defp extract_via_recompilation(app, gettext_config) do
     Gettext.Extractor.enable()
     force_compile()
@@ -146,48 +125,6 @@ defmodule Mix.Tasks.Gettext.Extract do
     Gettext.Extractor.disable()
   end
 
-  defp extract_from_attributes(app, gettext_config) do
-    incremental_compile()
-
-    {backends, messages} =
-      Gettext.Extractor.fill_from_compiled_beams(Mix.Project.compile_path())
-
-    if backends == 0 and messages == 0 do
-      Mix.raise("""
-      mix gettext.extract --from-attributes found no persisted Gettext messages \
-      or backends in #{Path.relative_to_cwd(Mix.Project.compile_path())}.
-
-      Messages are persisted to module attributes during normal compilation only \
-      when the backend has automatic extraction enabled in the application \
-      environment, for example in config/dev.exs:
-
-          config :gettext, MyApp.Gettext, automatic_extraction: true
-
-      If you just enabled this or updated Gettext, force a recompile so that \
-      up-to-date modules get their attributes written:
-
-          mix compile --force
-      """)
-    end
-
-    Gettext.Extractor.pot_files(app, gettext_config)
-  end
-
-  defp incremental_compile do
-    # A plain incremental compile, with one wrinkle: "compile" and the
-    # compilers it runs may already have been invoked in this VM (for
-    # example, through a task alias), in which case running them again would
-    # be a no-op unless they are reenabled first. The trailing explicit
-    # "compile.elixir" run is a no-op when "compile" just ran it, and covers
-    # the case where a custom "compile" alias does not.
-    Mix.Task.reenable("compile")
-    Mix.Task.reenable("compile.all")
-    Mix.Task.reenable("compile.elixir")
-    Mix.Task.reenable("compile.app")
-    Mix.Task.run("compile", [])
-    Mix.Task.run("compile.elixir", [])
-  end
-
   defp force_compile do
     # For old Elixir versions, we have to clean the manifest,
     # otherwise we are forced to compile all dependencies.

lib/mix/tasks/gettext.extract_from_attributes.ex

@@ -0,0 +1,83 @@
+defmodule Mix.Tasks.Gettext.ExtractFromAttributes do
+  use Mix.Task
+  @recursive true
+
+  @shortdoc "Extracts messages from compiled BEAM files without recompiling"
+
+  @moduledoc """
+  Extracts messages without force-recompiling the project (experimental).
+
+  ```bash
+  mix gettext.extract_from_attributes [OPTIONS]
+  ```
+
+  Unlike `mix gettext.extract`, which force-recompiles the whole project so
+  that the Gettext macros run again, this task reads the messages back from
+  the compiled BEAM files. It compiles the project normally (a no-op when it
+  is already compiled) and then scans the persisted module attributes, so it
+  avoids the cost of a force-recompile.
+
+  For this to work, messages must have been persisted as module attributes
+  during normal compilation. This happens automatically when the backend has
+  automatic extraction enabled in the application environment, which you
+  typically set in `config/dev.exs` so it stays off in `:prod`:
+
+      # config/dev.exs
+      config :gettext, MyApp.Gettext, automatic_extraction: true
+
+  Since the attributes are only persisted when `automatic_extraction` is
+  enabled (so not in `:prod`), release artifacts are unaffected.
+
+  This task accepts the same `--merge` and `--check-up-to-date` options as
+  `mix gettext.extract`, and forwards any other options to
+  `Mix.Tasks.Gettext.Merge`:
+
+  ```bash
+  mix gettext.extract_from_attributes --merge --no-fuzzy
+  mix gettext.extract_from_attributes --check-up-to-date
+  ```
+
+  """
+
+  @switches [merge: :boolean, check_up_to_date: :boolean]
+
+  @impl true
+  def run(args) do
+    Application.ensure_all_started(:gettext)
+    _ = Mix.Project.get!()
+    mix_config = Mix.Project.config()
+    {opts, _} = OptionParser.parse!(args, switches: @switches)
+    pot_files = extract(mix_config[:app], mix_config[:gettext] || [])
+    Mix.Tasks.Gettext.Extract.process(pot_files, opts, args)
+  end
+
+  defp extract(app, gettext_config) do
+    # The project is compiled (and its attributes persisted) by the normal
+    # compilation; here we just make sure that has happened. This is a no-op
+    # when the project is already compiled.
+    Mix.Task.run("compile", [])
+
+    {backends, messages} =
+      Gettext.Extractor.fill_from_compiled_beams(Mix.Project.compile_path())
+
+    if backends == 0 and messages == 0 do
+      Mix.raise("""
+      mix gettext.extract_from_attributes found no persisted Gettext messages \
+      or backends in #{Path.relative_to_cwd(Mix.Project.compile_path())}.
+
+      Messages are persisted to module attributes during normal compilation only \
+      when the backend has automatic extraction enabled in the application \
+      environment, for example in config/dev.exs:
+
+          config :gettext, MyApp.Gettext, automatic_extraction: true
+
+      If you just enabled this or updated Gettext, force a recompile so that \
+      up-to-date modules get their attributes written:
+
+          mix compile --force
+      """)
+    end
+
+    Gettext.Extractor.pot_files(app, gettext_config)
+  end
+end

test/mix/tasks/gettext.extract_from_attributes_test.exs

@@ -124,7 +124,7 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
     # POT files written by the recompilation path unchanged.
     output =
       capture_io(fn ->
-        in_project(test, tmp_dir, fn _module -> run(["--from-attributes"]) end)
+        in_project(test, tmp_dir, fn _module -> run_from_attributes() end)
       end)
 
     refute output =~ "Extracted"
@@ -137,7 +137,7 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
 
     output =
       capture_io(fn ->
-        in_project(test, tmp_dir, fn _module -> run(["--from-attributes"]) end)
+        in_project(test, tmp_dir, fn _module -> run_from_attributes() end)
       end)
 
     assert output =~ "Extracted priv/gettext/default.pot"
@@ -184,7 +184,7 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
     """)
 
     capture_io(fn ->
-      in_project(test, tmp_dir, fn _module -> run(["--from-attributes"]) end)
+      in_project(test, tmp_dir, fn _module -> run_from_attributes() end)
     end)
 
     pot = read_file(context, "priv/gettext/default.pot")
@@ -215,7 +215,7 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
     """)
 
     capture_io(fn ->
-      in_project(test, tmp_dir, fn _module -> run(["--from-attributes"]) end)
+      in_project(test, tmp_dir, fn _module -> run_from_attributes() end)
     end)
 
     default_pot = read_file(context, "priv/gettext/default.pot")
@@ -248,7 +248,7 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
     """)
 
     capture_io(fn ->
-      in_project(test, tmp_dir, fn _module -> run(["--from-attributes"]) end)
+      in_project(test, tmp_dir, fn _module -> run_from_attributes() end)
     end)
 
     pot = read_file(context, "priv/gettext/default.pot")
@@ -267,7 +267,7 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
     write_file(context, "priv/gettext/it/LC_MESSAGES/default.po", "")
 
     capture_io(fn ->
-      in_project(test, tmp_dir, fn _module -> run(["--from-attributes", "--merge"]) end)
+      in_project(test, tmp_dir, fn _module -> run_from_attributes(["--merge"]) end)
     end)
 
     po = read_file(context, "priv/gettext/it/LC_MESSAGES/default.po")
@@ -291,7 +291,7 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
     """)
 
     capture_io(fn ->
-      in_project(test, tmp_dir, fn _module -> run(["--from-attributes"]) end)
+      in_project(test, tmp_dir, fn _module -> run_from_attributes() end)
     end)
 
     assert read_file(context, "priv/custom_gettext/default.pot") =~
@@ -307,11 +307,11 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
 
     capture_io(fn ->
       in_project(test, tmp_dir, fn _module ->
-        run(["--from-attributes"])
+        run_from_attributes()
       end)
 
       in_project(test, tmp_dir, fn _module ->
-        run(["--from-attributes", "--check-up-to-date"])
+        run_from_attributes(["--check-up-to-date"])
       end)
     end)
 
@@ -325,7 +325,7 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
     capture_io(fn ->
       assert_raise Mix.Error, expected_error, fn ->
         in_project(test, tmp_dir, fn _module ->
-          run(["--from-attributes", "--check-up-to-date"])
+          run_from_attributes(["--check-up-to-date"])
         end)
       end
     end)
@@ -344,7 +344,7 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
     capture_io(fn ->
       assert_raise Mix.Error, ~r/found no persisted Gettext messages/, fn ->
         in_project(test, tmp_dir, fn _module ->
-          run(["--from-attributes"])
+          run_from_attributes()
         end)
       end
     end)
@@ -415,7 +415,7 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
       in_project(test, tmp_dir, fn _module ->
         # Mix.Task.run (not a direct module call) so that @recursive true
         # recurses into each umbrella app.
-        Mix.Task.run("gettext.extract", ["--from-attributes"])
+        Mix.Task.run("gettext.extract_from_attributes", [])
       end)
     end)
 
@@ -440,4 +440,15 @@ defmodule Mix.Tasks.Gettext.ExtractFromAttributesTest do
   defp run(args) do
     Mix.Tasks.Gettext.Extract.run(args)
   end
+
+  defp run_from_attributes(args \\ []) do
+    # Reenable compile so each invocation picks up source changes, mirroring a
+    # fresh `mix` invocation (the task itself does a plain compile, not a
+    # force-recompile).
+    for task <- ~w(compile compile.all compile.elixir compile.app) do
+      Mix.Task.reenable(task)
+    end
+
+    Mix.Tasks.Gettext.ExtractFromAttributes.run(args)
+  end
 end