Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Mix.Tasks.Compile.reenable #13771

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions lib/mix/lib/mix/task.compiler.ex
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,33 @@ defmodule Mix.Task.Compiler do
{:noop, []}
end
end

@doc """
Reenables given compilers so they can be executed again down the stack.

If an umbrella project reenables compilers, they are re-enabled for all
child projects.

Default is `[]`, for none compilers to be reenabled.
This task always re-enables `"compiler.all"`.
"""
@spec reenable([{:compilers, compilers}]) :: :ok when compilers: :all | [Mix.Task.task_name()]
def reenable(opts \\ []) do
if not Keyword.keyword?(opts) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to check for the arguments.

IO.warn(
"Mix.Task.Compiler.reenable/1 expects a keyword list " <>
"as an argument with compilers: [compiler()], " <>
"reenabling no compiler"
)
else
compilers =
case Keyword.get(opts, :compilers, []) do
:all -> Mix.Tasks.Compile.compilers()
list when is_list(list) -> list
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with all by default, because it is easier to customize.

Suggested change
case Keyword.get(opts, :compilers, []) do
:all -> Mix.Tasks.Compile.compilers()
list when is_list(list) -> list
end
Keyword.get_lazy(opts, :compilers, &Mix.Tasks.Compile.compilers/0)

I am also thinking we should move this function to Mix.Tasks.Compile, since compilers is already defined there.


Enum.each(["loadpaths", "compile", "compile.all"], &Mix.Task.reenable(&1))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure if we should enable loadpaths. Thoughts? Why not deps.loadpathsas well? That's one of the slippery slopes that worry me. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure if we should enable loadpaths.

We should not. It is a leftover from the code when I thought the enabling should be mirrored by a roll-back, but it should not in the latest approach. I cannot think of any change resulting in a need to reload paths from within same external task execution, even if it calls reenable/1.

Enum.each(compilers, &Mix.Task.reenable("compile.#{&1}"))
end
end
end
7 changes: 5 additions & 2 deletions lib/mix/lib/mix/task.ex
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,11 @@ defmodule Mix.Task do
child projects.
"""
@spec reenable(task_name) :: :ok
def reenable(task) when is_binary(task) or is_atom(task) do
task = to_string(task)
def reenable(task) when is_atom(task) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the changes to this function. :)

task |> to_string() |> reenable()
end

def reenable(task) when is_binary(task) do
proj = Mix.Project.get()
recursive = (module = get(task)) && recursive(module)

Expand Down
27 changes: 27 additions & 0 deletions lib/mix/test/mix/tasks/compile.elixir_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,33 @@ defmodule Mix.Tasks.Compile.ElixirTest do
end)
end

test "recompiles newly created files" do
in_fixture("no_mixfile", fn ->
Mix.Project.push(MixTest.Case.Sample)

assert Mix.Tasks.Compile.Elixir.run(["--verbose"]) == {:ok, []}
assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
assert_received {:mix_shell, :info, ["Compiled lib/b.ex"]}

Mix.shell().flush()

File.write!("lib/z.ex", """
defmodule Z do
def ok, do: :ok
end
""")

Mix.Task.reenable("compile.elixir")
assert {:ok, []} = Mix.Tasks.Compile.Elixir.run(["--verbose"])

assert_received {:mix_shell, :info, ["Compiled lib/z.ex"]}
refute_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
refute_received {:mix_shell, :info, ["Compiled lib/b.ex"]}

assert File.regular?("_build/dev/lib/sample/ebin/Elixir.Z.beam")
end)
end

josevalim marked this conversation as resolved.
Show resolved Hide resolved
test "recompiles dependent changed modules" do
in_fixture("no_mixfile", fn ->
Mix.Project.push(MixTest.Case.Sample)
Expand Down
29 changes: 29 additions & 0 deletions lib/mix/test/mix/tasks/compile_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,35 @@ defmodule Mix.Tasks.CompileTest do
end)
end

test "recompiles newly created files" do
in_fixture("no_mixfile", fn ->
assert Mix.Tasks.Compile.run(["--verbose"]) == {:ok, []}
Mix.shell().flush()

File.write!("lib/z.ex", """
defmodule Z do
def ok, do: :ok
end
""")

assert :ok = Mix.Task.reenable("compile")
assert {:noop, []} = Mix.Task.run("compile")
refute File.regular?("_build/dev/lib/sample/ebin/Elixir.Z.beam")

assert :ok = Mix.Task.Compiler.reenable()
assert {:noop, []} = Mix.Task.run("compile")
refute File.regular?("_build/dev/lib/sample/ebin/Elixir.Z.beam")

assert :ok = Mix.Task.Compiler.reenable(compilers: ["erlang"])
assert {:noop, []} = Mix.Task.run("compile")
refute File.regular?("_build/dev/lib/sample/ebin/Elixir.Z.beam")

assert :ok = Mix.Task.Compiler.reenable(compilers: ["elixir", "erlang"])
assert {:ok, []} = Mix.Task.run("compile")
assert File.regular?("_build/dev/lib/sample/ebin/Elixir.Z.beam")
end)
end

test "recompiles app cache if manifest changes" do
in_fixture("no_mixfile", fn ->
Mix.Tasks.Compile.run(["--force"])
Expand Down