Skip to content

Commit

Permalink
Ensure duplicate modules are recompiled, closes #13040
Browse files Browse the repository at this point in the history
  • Loading branch information
@josevalim
josevalim committed Oct 27, 2023
1 parent 0e1a5d3 commit 8923da0
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 26 deletions.
63 changes: 38 additions & 25 deletions lib/mix/lib/mix/compilers/elixir.ex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ defmodule Mix.Compilers.Elixir do
compiler_loop(stale, stale_modules, dest, timestamp, opts, state)
else
{:ok, info, state} ->
{modules, _exports, sources, pending_modules, _pending_exports} = state
{modules, _exports, sources, pending_modules, _stale_exports} = state

previous_warnings =
if Keyword.get(opts, :all_warnings, true),
Expand Down Expand Up @@ -531,7 +531,7 @@ defmodule Mix.Compilers.Elixir do

defp remove_stale_entry(entry, acc, sources, stale_exports, compile_path) do
module(module: module, sources: source_files, export: export) = entry
{rest, exports, changed, stale} = acc
{rest, exports, changed, stale_modules} = acc

{compile_references, export_references, runtime_references} =
Enum.reduce(source_files, {[], [], []}, fn file, {compile_acc, export_acc, runtime_acc} ->
Expand All @@ -548,20 +548,20 @@ defmodule Mix.Compilers.Elixir do
# If I changed in disk or have a compile time reference to
# something stale or have a reference to an old export,
# I need to be recompiled.
has_any_key?(changed, source_files) or has_any_key?(stale, compile_references) or
has_any_key?(changed, source_files) or has_any_key?(stale_modules, compile_references) or
has_any_key?(stale_exports, export_references) ->
remove_and_purge(beam_path(compile_path, module), module)
changed = Enum.reduce(source_files, changed, &Map.put(&2, &1, true))
{rest, Map.put(exports, module, export), changed, Map.put(stale, module, true)}
{rest, Map.put(exports, module, export), changed, Map.put(stale_modules, module, true)}

# If I have a runtime references to something stale,
# I am stale too.
has_any_key?(stale, runtime_references) ->
{[entry | rest], exports, changed, Map.put(stale, module, true)}
has_any_key?(stale_modules, runtime_references) ->
{[entry | rest], exports, changed, Map.put(stale_modules, module, true)}

# Otherwise, we don't store it anywhere
true ->
{[entry | rest], exports, changed, stale}
{[entry | rest], exports, changed, stale_modules}
end
end

Expand Down Expand Up @@ -1032,11 +1032,29 @@ defmodule Mix.Compilers.Elixir do
end
end

defp each_cycle(stale_modules, compile_path, timestamp, state) do
{modules, _exports, sources, pending_modules, pending_exports} = state
defp each_cycle(runtime_modules, compile_path, timestamp, state) do
{modules, _exports, sources, pending_modules, stale_exports} = state

stale_modules =
modules
|> Enum.map(&module(&1, :module))
|> Map.from_keys(true)

# Because a module may be accidentally overridden in another file,
# we need to mark any pending module that has been defined as changed.
{pending_modules, changed} =
Enum.reduce(pending_modules, {[], []}, fn module, {pending_acc, changed_acc} ->
if is_map_key(stale_modules, module(module, :module)) do
{pending_acc, module(module, :sources) ++ changed_acc}
else
{[module | pending_acc], changed_acc}
end
end)

# We don't need to pass stale_modules here because
# runtime dependencies have already been marked as stale.
{pending_modules, exports, changed} =
update_stale_entries(pending_modules, sources, [], %{}, pending_exports, compile_path)
update_stale_entries(pending_modules, sources, changed, %{}, stale_exports, compile_path)

# For each changed file, mark it as changed.
# If compilation fails mid-cycle, they will be picked next time around.
Expand All @@ -1045,18 +1063,13 @@ defmodule Mix.Compilers.Elixir do
end

if changed == [] do
stale_modules =
modules
|> Enum.map(&module(&1, :module))
|> Map.from_keys(true)
|> Map.merge(stale_modules)

{_, runtime_modules, sources} = fixpoint_runtime_modules(sources, stale_modules)
{_, runtime_modules, sources} =
fixpoint_runtime_modules(sources, Map.merge(stale_modules, runtime_modules))

runtime_paths =
Enum.map(runtime_modules, &{&1, Path.join(compile_path, Atom.to_string(&1) <> ".beam")})

state = {modules, exports, sources, pending_modules, pending_exports}
state = {modules, exports, sources, pending_modules, stale_exports}
{{:runtime, runtime_paths, []}, state}
else
Mix.Utils.compiling_n(length(changed), :ex)
Expand Down Expand Up @@ -1085,7 +1098,7 @@ defmodule Mix.Compilers.Elixir do

defp each_file(file, references, verbose, state, cwd) do
{compile_references, export_references, runtime_references, compile_env} = references
{modules, exports, sources, pending_modules, pending_exports} = state
{modules, exports, sources, pending_modules, stale_exports} = state

file = Path.relative_to(file, cwd)

Expand Down Expand Up @@ -1114,22 +1127,22 @@ defmodule Mix.Compilers.Elixir do
compile_env: compile_env
)

{modules, exports, [source | sources], pending_modules, pending_exports}
{modules, exports, [source | sources], pending_modules, stale_exports}
end

defp each_module(file, module, kind, external, new_export, state, timestamp, cwd) do
{modules, exports, sources, pending_modules, pending_exports} = state
{modules, exports, sources, pending_modules, stale_exports} = state

file = Path.relative_to(file, cwd)
external = process_external_resources(external, cwd)

old_export = Map.get(exports, module)

pending_exports =
stale_exports =
if old_export && old_export != new_export do
pending_exports
stale_exports
else
Map.delete(pending_exports, module)
Map.delete(stale_exports, module)
end

{module_sources, existing_module?} =
Expand Down Expand Up @@ -1163,7 +1176,7 @@ defmodule Mix.Compilers.Elixir do
)

modules = prepend_or_merge(modules, module, module(:module), module, existing_module?)
{modules, exports, [source | sources], pending_modules, pending_exports}
{modules, exports, [source | sources], pending_modules, stale_exports}
end

defp prepend_or_merge(collection, key, pos, value, true) do
Expand Down
30 changes: 29 additions & 1 deletion lib/mix/test/mix/tasks/compile.elixir_test.exs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -931,10 +931,11 @@ defmodule Mix.Tasks.Compile.ElixirTest do
assert File.regular?("_build/dev/lib/sample/ebin/Elixir.A.beam")
assert File.regular?("_build/dev/lib/sample/ebin/Elixir.B.beam")

# Compile directly so it does not point to a .beam file
Code.put_compiler_option(:ignore_module_conflict, true)
Code.compile_file("lib/b.ex")
force_recompilation("lib/a.ex")

force_recompilation("lib/a.ex")
Mix.Tasks.Compile.Elixir.run(["--verbose"])
assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
end)
Expand Down Expand Up @@ -963,6 +964,33 @@ defmodule Mix.Tasks.Compile.ElixirTest do
end)
end

test "compiles dependent changed on conflict" 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"]}
purge([A, B])

capture_io(:stderr, fn ->
File.write!("lib/a.ex", "defmodule B, do: :not_ok")
assert {:ok, [_, _]} = Mix.Tasks.Compile.Elixir.run(["--verbose"])
assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
assert_received {:mix_shell, :info, ["Compiled lib/b.ex"]}
purge([A, B])
end)

capture_io(:stderr, fn ->
File.write!("lib/a.ex", "defmodule A, do: :ok")
assert {:ok, []} = Mix.Tasks.Compile.Elixir.run(["--verbose"])
assert_received {:mix_shell, :info, ["Compiled lib/a.ex"]}
assert_received {:mix_shell, :info, ["Compiled lib/b.ex"]}
purge([A, B])
end)
end)
end

test "compiles dependent changed external resources" do
in_fixture("no_mixfile", fn ->
Mix.Project.push(MixTest.Case.Sample)
Expand Down

0 comments on commit 8923da0

Please sign in to comment.