rebar3 icon indicating copy to clipboard operation
rebar3 copied to clipboard

Initial version of the Manifest plugin

Open robertoaloi opened this issue 5 months ago • 3 comments

The manifest provider allows users to extract information about a rebar3 project, so that the information can be used by external tools such as language servers to learn about the structure of a project. The format and content of the produced manifest is totally up for debate. This is just to start the discussion. Ideally, the manifest should not be tight to rebar3 as a build tool. This would enable other build tools to produce an equivalent file.

The provider is inspired by the build_info one which is part of the eqWalizer type checker for Erlang, but uses built-in functionality to fetch dependencies.

Having a built-in provider would remove friction for language server users and implementors. ELP users are currently required to install additional plugins to be able to use the language server. Erlang LS implementors are attempting efforts such as this one, to approximate the structure of a rebar3 project. A built-in manifest provider would simplify all of this.

Format

Ideally, one could have used JSON as the standard format for the manifest (the Rust Analyzer language server uses a rust-project.json format for a similar concept. Unfortunately, there's currently no JSON library provided out-of-the-box by Erlang/OTP or rebar3, so I'm defaulting to Erlang terms. I am also adding the ability to use EETF (Erlang External Term Format) as an alternative format, since this is the format currently used by the ELP language server via the build_info plugin.

Output

By default the provider outputs the manifest to stdout. For this reason, logging is disabled by the provider (an alternative would have been to ask users to use the QUIET=1 option or to filter out extraneous log messages). A dedicated option provides the ability to dump the manifest to file. This is mostly done to simplify testing and to avoid playing with group leaders or similar approaches. Again, this is all open for discussion.

Example

Running the command:

rebar3 manifest

For the Erlang LS project would produce something like this.

robertoaloi avatar Jan 25 '24 16:01 robertoaloi

Thanks @ferd for the quick and accurate review! I will try to address all the feedback soon.

  • I assumed the logging hack wouldn't stay in the final version, but I wanted to see if there was a better alternative. I think for the time I will keep it simple and just remove it. There's always the file variant that can be used as an alternative, or the caller will have to skip the extraneous output. We can introduce the delimiter when/if needed
  • I only added basic tests in case you had opinions about the actual format. If not, I will proceed and refine the tests
  • Regarding spaces and formatting, I copied a template from somewhere (don't recall where) and applied a local formatter. Will try to match the desired way

Regarding documentation about the provider (e.g. in the README or the website), maybe we could leave it undocumented until ELP / Erlang LS integrate with it and the output format is refined.

robertoaloi avatar Jan 29 '24 09:01 robertoaloi

Yeah I'm good leaving it undocumented. Alternatively, we have an 'experimental' namespace that lets us change the command as we see fit and announce it and get users but without any guarantees. The issue is of course that once it's stable, if you rely on it already, you need to fallback from default to experimental to unsupported.

ferd avatar Jan 30 '24 21:01 ferd

@ferd Sorry for the delay in the follow-up. I think I addressed all the feedback.

  • Removed logging altogether
  • Leaving tests as they are until we iterate on an actual integration with ELP / Erlang LS
  • Have a question about Unicode (we could address it as a follow up)
  • Fixed the rest

robertoaloi avatar Feb 16 '24 16:02 robertoaloi

@ferd

  • I was referring to your "It's possible you need to keep the type with the data to output it safely?". I was wondering if you meant to store the fact the content was eetf or erlang as well?
  • I am not sure about the experimental namespace, so your advice is welcome here. What are the implications for the user if we go for experimental? Do they need to tweak their rebar config to get access to the plugin? The main goal of the plugin is to avoid a manual installation step when using a language server, so having to tweak the rebar config partially defeats its purpose. The format of the produced build_info (and maybe the flags) are expected to change, especially while we work on the integration itself. But after a short period they should stabilize.

robertoaloi avatar Feb 20 '24 08:02 robertoaloi

@ferd

  • I was referring to your "It's possible you need to keep the type with the data to output it safely?". I was wondering if you meant to store the fact the content was eetf or erlang as well?

Oh yeah. The Erlang format is regular text and you'd want that to be unicode-aware, ideally, to keep showing paths as text rather than lists of integers or bytes. The eetf type is however byte sequences and it may output values that aren't valid unicode or text, so you wouldn't want to use the t qualifier there, only for text.

  • I am not sure about the experimental namespace, so your advice is welcome here. What are the implications for the user if we go for experimental? Do they need to tweak their rebar config to get access to the plugin? The main goal of the plugin is to avoid a manual installation step when using a language server, so having to tweak the rebar config partially defeats its purpose. The format of the produced build_info (and maybe the flags) are expected to change, especially while we work on the integration itself. But after a short period they should stabilize.

The experimental namespace just means that you have to call the task as rebar3 experimental manifest (see the vendor provider for an example) and will show up under that namespace in rebar3 help. By default, anything in the experimental namespace is expected to be unstable and prone to change. This is useful when the whole interface is still not stable, as opposed to when only some options may be worth changing.

Whenever we feel the format is solid and adequate we can promote the task outside the experimental namespace and into the default one, where rebar3 manifest will let it be called with an expectation of stability. Nothing else will need to change.

ferd avatar Feb 20 '24 16:02 ferd

@ferd If I mark the plugin as experimental (and update the tests accordingly), I get a failure due to a dependency not found:

{thrown,{{error,{providers,{provider_not_found,experimental,install_deps}}},
         [{providers,process_dep,2,
                     [{file,"/Users/robertoaloi/git/github/robertoaloi/rebar3/vendor/providers/src/providers.erl"},
                      {line,285}]},
                      [...]

It looks like rebar3 assumes that, if a provider is experimental, all of its dependencies also belong to the same namespace. Is this intentional? Any way to circumvent this?

robertoaloi avatar Feb 20 '24 19:02 robertoaloi

@ferd If I mark the plugin as experimental (and update the tests accordingly), I get a failure due to a dependency not found:

{thrown,{{error,{providers,{provider_not_found,experimental,install_deps}}},
         [{providers,process_dep,2,
                     [{file,"/Users/robertoaloi/git/github/robertoaloi/rebar3/vendor/providers/src/providers.erl"},
                      {line,285}]},
                      [...]

It looks like rebar3 assumes that, if a provider is experimental, all of its dependencies also belong to the same namespace. Is this intentional? Any way to circumvent this?

oh yeah that's because the plugin dependencies look in your namespace by default. Specify the dep as {default, install_deps} and it should work.

ferd avatar Feb 20 '24 19:02 ferd

@ferd Yes, I eventually figured that out from the code. Does it mean the spec is wrong, then?

Updated the PR with your suggestions.

robertoaloi avatar Feb 20 '24 19:02 robertoaloi

@ferd Yes, I eventually figured that out from the code. Does it mean the spec is wrong, then?

Updated the PR with your suggestions.

Yeah I think the spec is wrong, or pre-dates us adding namespaces to providers.

ferd avatar Feb 20 '24 19:02 ferd

@ferd No Dialyzer failure. It looks like the spec fix can be done as a follow-up.

robertoaloi avatar Feb 21 '24 15:02 robertoaloi