Support a configurable module lookup from a path

[Gazler]

Erlang has a constraint that ensures there is a mapping from filename to module name:

foo.erl => -module(foo)
bar.erl => -module(bar)

This makes looking up a module based on a path name deterministic. This is not the case in other BEAM languages such as Elixir. In Elixir, there is no mapping between the file name and the module name. It is also possible for an Elixir file to contain multiple modules.

In order to allow edb to be used in Elixir, we need a way to configure the mapping from path to module. However, instead of baking in support just for Elixir, it would be ideal to have this be generic.

This commit introduces a configurable function which can be provided on the target and called with edb:eval. This is configured using the file_to_module_cb application config. This is expected to return a list of modules, which will then be used for setting breakpoints on the module.

This does introduce another issue though. When removing a breakpoint, if the module is changed in any way, or it is not possible to call the same lookup function, it may not be possible to unset the breakpoint. For this reason, the lookup is cached, and if the breakpoints are set to any empty list, then the cached module lookup is used and removed from the cache.

[meta-cla]

Hi @Gazler!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[meta-cla]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

[jcpetruzza]

Thanks for looking into this @Gazler!

Configuring language specific overrides via an application env makes sense, but by passing simply a function callback, it seems to me like we fail to keep it general enough. For example, to make things work for Elixir we'd currently need to add Elixir-specific logic to edb, like extending the DAP state with the source cache or assuming that the callback needs to be evaluated on the debuggee.

So I'd propose we instead pass in configuration a module implementing a new behaviour, say edb_dap_language? The behaviour would have something like an init() function, returning a cb state, and, for now, a source_to_modules() function as you are introducing here,, but it also gives us an extension point for other places requiring language-specific customization (e.g. eval).

We'd need to add here a default implementation for Erlang, which will serve as reference for other languages; and the Elixir implementation would then have the source_modules mapping in its own cb state, and use edb:eval() to implement source_to_modules().

What do you think?

[Gazler]

For example, to make things work for Elixir we'd currently need to add Elixir-specific logic to edb, like extending the DAP state with the source cache or assuming that the callback needs to be evaluated on the debuggee.

Maybe I'm misunderstanding here, I'm not sure this is necessarily Elixir specific, but this is definitely the requirement today since the function needs to be called in the context of Elixir (or Mix) to list the sources.

Perhaps you mean Elixir specific in the sense that neither of these things are required for the Erlang implementation, in which case I totally agree.

So I'd propose we instead pass in configuration a module implementing a new behaviour, say edb_dap_language? The behaviour would have something like an init() function, returning a cb state, and, for now, a source_to_modules() function as you are introducing here,, but it also gives us an extension point for other places requiring language-specific customization (e.g. eval).

I like this idea. It defines a concrete API for the expectation of the module. For example, in the code I proposed, there is an implicit expectation that the provided callback function maps from a %{binary() -> [atom]} - a behaviour would solidify this contract.

What I'd like to know, is how do we provide this module to edb?

What I did for this implementation, is have a script that is called via the edb integration in the IDE (in this case a neovim plugin, but the same logic applies for say, VSCode)

Where we provide a script that does something like:

defmodule Edb.MixSourceResolver do
  def install do
    Application.put_env(:edb, :file_to_module_cb, {:eval, &modules_for_source/1})
  end

  def modules_for_source(path), do: ...
end

Edb.MixSourceResolver.install()

Do you envision something similar, but instead of file_to_module_cb we define the implementation module at this point?

I think if we do it that way, we need to define it before reverse attach is called. However, this would be simpler as we can do something like:

ERL_AFLAGS="-eval application:put_env(:edb, :edb_implementation, 'Elixir.Edb'). $EDB_DAP_DEBUGGEE_INIT" mix run ...

I think this will depend on where the module lives though:

Elixir stdlib module is available for "-eval":

ERL_AFLAGS="-eval erlang:display(code:ensure_loaded(\\'Elixir.Kernel\\'))" elixir -e "IO.inspect(:start)"
{module,'Elixir.Kernel'}
:start

User application level module is not available for "-eval":

ERL_AFLAGS="-eval erlang:display(code:ensure_loaded(\\'Elixir.MyApp\\'))" mix run -e "IO.inspect(:start)"
{error,nofile}
:start

[Gazler]

Probably worth noting that the other thing added here is merging lines for when there are multiple modules with breakpoints. This likely wouldn't be covered if we just had a source_to_modules() function, and again, is not something necessary for an Erlang implementation.

[jcpetruzza]

Perhaps you mean Elixir specific in the sense that neither of these things are required for the Erlang implementation, in which case I totally agree.

Yes, I meant this in that sense,

What I'd like to know, is how do we provide this module to edb?

Good question. Thinking out loud a bit more, an alternative to an app env would be to extend the CLI with an optional arg, say, --language=path/to/LANG_MODULE.beam? When provided, on start up, we add the given path to the code path, load the module and put it in the DAP state.

That way one could then re-package edb for other languages by providing the beams for edb + behaviour, plus with a wrapper script, say, edb-elixir that execs the edb cli with the right language arg. Then IDE extensions just need to call edb-elixir dap and things work transparently.

ERL_AFLAGS="-eval application:put_env(:edb, :edb_implementation, 'Elixir.Edb'). $EDB_DAP_DEBUGGEE_INIT" mix run ...

Btw, I'm not sure I understood this part. These $EDB_DAP_DEBUGGEE_INIT part isintended to be applied on the debuggee node to be launched, but the application env is needed in the debugger node, where the DAP runs.

However this makes me think that if, say, the Elixir behaviour implemention needs to run code on the debuggee (e.g. via edb:eval()), it may want to add some modules to the list of debugging modules uploaded to the debuggee during bootstrapping, which could be done exposing another callback on the behaviour.

[Gazler]

Good question. Thinking out loud a bit more, an alternative to an app env would be to extend the CLI with an optional arg, say, --language=path/to/LANG_MODULE.beam? When provided, on start up, we add the given path to the code path, load the module and put it in the DAP state.

This seems like it could be feasible.

Btw, I'm not sure I understood this part. These $EDB_DAP_DEBUGGEE_INIT part isintended to be applied on the debuggee node to be launched, but the application env is needed in the debugger node, where the DAP runs.

Right, I was talking about the debuggee here.

For example, for the erlang config we have something like:

      dap.configurations.erlang = {
        edb_config("EDB: rebar3 test shell", "launch", {
          runInTerminal = {
            kind = "integrated",
            title = "rebar3 shell",
            cwd = "${workspaceFolder}",
            args = {
              "sh",
              "-c",
              'exec "$0" "$@" --eval="$EDB_DAP_DEBUGGEE_INIT"', # THIS LINE HERE
              "rebar3",
              "as",
              "test",
              "shell",
            },
          },
          config = {
            nameDomain = "shortnames",
            nodeInitCodeInEnvVar = "EDB_DAP_DEBUGGEE_INIT",
            timeout = 300,
          },
        }),
      }

We'd need variant for Elixir. For testing this, I was using an elixir script that executes the reverse_attach code after setting the file_to_module_cb appliction config.

Again, the depends largely on eval being use the debuggee node for the config. Which is probably not the intention for a generic implementation, especially since there's no need to eval with erlang. We could potentially add an eval implementation of edb_dap_language that uses a configured application config, or an environment variable or something for the module to use on the debuggee and wraps all of the behaviour callbacks in :edb:eval()?

However this makes me think that if, say, the Elixir behaviour implemention needs to run code on the debuggee (e.g. via edb:eval()), it may want to add some modules to the list of debugging modules uploaded to the debuggee during bootstrapping, which could be done exposing another callback on the behaviour.

Right, the elixir version is going likely to depend on https://mix.hexdocs.pm/Mix.Task.Compiler.html#manifests/1 which is a large reason why eval is required.

[josevalim]

I have very little detail on this but I think some sort of callback would be best because we can evolve and improve it directly from Elixir. Erlang logic is simple but could be moved to the default callback to stress-test the logic.