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

Always adding :debug_info on erlc_options #9348

Merged
merged 6 commits into from
Sep 22, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion lib/elixir/lib/code.ex
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,7 @@ defmodule Code do

* `:debug_info` - when `true`, retain debug information in the compiled
module. This allows a developer to reconstruct the original source
code. Defaults to `false`.
code. Defaults to `true`.

* `:ignore_module_conflict` - when `true`, override modules that were
already defined without raising errors. Defaults to `false`.
Expand Down
3 changes: 2 additions & 1 deletion lib/mix/lib/mix/tasks/compile.erlang.ex
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ defmodule Mix.Tasks.Compile.Erlang do
Mix.raise(":erlc_options should be a list of options, got: #{inspect(erlc_options)}")
end

erlc_options = erlc_options ++ [:return, :report, outdir: compile_path, i: include_path]
erlc_options =
erlc_options ++ [:debug_info, :return, :report, outdir: compile_path, i: include_path]

erlc_options =
Enum.map(erlc_options, fn
Expand Down
14 changes: 14 additions & 0 deletions lib/mix/test/mix/tasks/compile.erlang_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -174,4 +174,18 @@ defmodule Mix.Tasks.Compile.ErlangTest do
assert output == ""
end)
end

@tag erlc_options: [{:warnings_as_errors, true}]
test "adds :debug_info to erlc_options by default" do
in_fixture("compile_erlang", fn ->
Mix.Tasks.Compile.Erlang.run([])

binary = File.read!("_build/dev/lib/sample/ebin/b.beam")

{:ok, {module, [debug_info: {:debug_info_v1, backend, data}]}} =
:beam_lib.chunks(binary, [:debug_info])

assert backend.debug_info(:elixir_v1, module, data, []) != {:error, :missing}
Copy link
Member

Choose a reason for hiding this comment

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

I think this should assert with :erlang_v1 - after all it's compiling erlang modules, so it's kind of expected it won't be able to return elixir debug info.

Suggested change
assert backend.debug_info(:elixir_v1, module, data, []) != {:error, :missing}
assert backend.debug_info(:erlang_v1, module, data, []) != {:error, :missing}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alrighty!

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't even assert this. Asserting it returns a debug info chunk is probably good enough!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean something like this?

{:ok, {_, [debug_info: {:debug_info_v1, _, {info, _}}]}} = :beam_lib.chunks(binary, [:debug_info])
assert info != :none

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've checked and the only difference is that it returns :none for that part when debug info is disabled, instead of the list of attributes.

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 have started with that actually, just changed to use backend.debug_info because I was afraid that maybe the format could change in the future and then the test could fail. Very unlikely, but who knows 🤷‍♂

end)
end
end