Add support for project-local plugins #2697

ferd · 2022-04-09T22:42:34Z

This puts the basic scaffolding in place to allow project-local plugin
definitions. This, in turn, can be usable for vendoring mechanisms.

A few things that are still tricky with this approach:

There are some limited tests
There is no great way to only making this work with umbrella projects,
since plugins are installed before the discovery phase that decides
whether a project is an umbrella project or not, so we use an
imperfect heuristic (eg. using project_src_app directories
except the project root)
Whether it clashes with things such as global plugins' paths and the
general usage of rebar_paths to switch things in and out of visibility
for projects is still untested
Dependencies appear well-handled.
How overrides should work on this one. I'm assuming they shouldn't.
If profiles are working well with this (and whether they even should,
since they don't with other plugins?) -- I decided to not support them much.
It seems to work fine with recursive plugins and local definitions
upgrades are bypassed if a locally-defined plugin is found

I'm committing and putting it in a draft PR for awareness.

This puts the basic scaffolding in place to allow project-local plugin definitions. This, in turn, can be usable for vendoring mechanisms. A few things that are still tricky with this approach: - This is not tested aside from manual execution with a toy case - There is no great way to only making this work with umbrella projects, since plugins are installed before the discovery phase that decides whether a project is an umbrella project or not, so there may be a need for an imperfect heuristic (eg. using project_src_app directories except the project root) - Whether it clashes with things such as global plugins' paths and the general usage of rebar_paths to switch things in and out of visibility for projects. - If dependencies are well handled. - How overrides should work on this one. I'm assuming they shouldn't. - If profiles are working well with this (and whether they even should, since they don't with other plugins) - If it works with recursive plugins and local definitions Manual executions however seem to show that this works. I'm committing and putting it in a draft PR for awareness.

This sounds weird as hell, but essentially, when a local plugin relies on an external plugin, make sure that we fetch and build that one in an orderly manner. This was found by adding initial tests.

This adds a test confirming the implemented behaviour, which states that a local plugin that declares a project_plugin won't see it fetched from the root project. This would generally be a thing ignored by a dependency and it makes no sense to have it declared in the local plugin and also impact the top project.

Only umbrella projects can have these.

ferd · 2022-04-10T04:04:45Z

Agh I now realize there's a whole similar code path for plugins upgrade that I'll need to double check.

Otherwise you can run into funny situations where externally-defined plugins sharing the same name as the local one get downloaded and clobbers the definition in _build (even if it gets copied over later). Locally defined plugins are just not upgradable, the same way top level dependencies are not.

Turns out that using local plugins as deps means we don't do recompilation checks automatically, so instead I reuse some of the compiler util functions to do a quick timestamp comparison in order to retrigger a build, which will then go through the usual DAG mechanism once deps are built.

starbelly · 2022-04-10T16:28:05Z

I did a basic test in an umbrella, looks good 🚀

Edit:

Note I tested using rebar3_hex. One case where I didn't have rebar3_hex as a global plugin, and one where I did. In the case where I tested with rebar3_hex as a global plugin and as a local plugin, while we still fetch rebar3_hex (which is to be expected), the local plugin wins.

tsloughter · 2022-04-26T15:59:19Z

src/rebar_plugins.erl

+
+top_level_deps(Apps) ->
+ RawDeps = lists:foldl(fun(App, Acc) ->
+ %% Only support the profiles we would with regular plugins?


The ? makes it sound like this does support profiles besides default and prod? But the code looks like it is filtering by those 2 profiles?

Preferably we'd support all profiles since this is potentially used not for vendoring but for normal development. Like if I wanted a plugin to help with development on OpenTelemetry which is an umbrella.

Yeah, I was expecting that default plugins would only fetch the plugins in default+prod mode, like any dependency. The top-level plugins are already selected by the time we run this here, this is about hoisting the plugins' own deps and plugins into the project as well. I'm unsure whether we need to handle more profiles than that, because I'm not sure we want to get dependencies' test plugins into a regular space.

aaah, the plugins deps, ok.

Tho if I'm developing a project and want a plugin in the umbrella I'd expect its tests to run if I ran rebar3 ct at the top level if the plugin is under apps, as opposed to another directory like plugins where I would expect a separation of concerns.

Yeah that's fair. I'll have to look at bringing in the profile expanding of deps for plugins I guess.

Take a look again, made it fetch more profiles.

tsloughter · 2022-04-26T21:19:28Z

Some uncertainty still but approving to move forward.

New features: - [Add --offline option and REBAR_OFFLINE environment variable](#2643) - [Add support for project-local plugins](#2697) - [Add eunit --test flag](#2684) Experimental features for which we promise no backwards compatibility in the near future: - [Experimental vendoring provider](#2689) - [Support plugins in experimental vendor provider](#2702) Other changes: - [Support OTP 23..25 inclusively](#2706) - [Bump Relx to 4.7.0](#2718) - [Use `erlexec` directly in relx helper functions](erlware/relx#902) - [Make rlx_util:parse_vsn parse integer versions](erlware/relx#913) - [fix awk script check_name() in extended_bin](erlware/relx#915) - [avoid crash when overlay is malformed](erlware/relx#916) - [keep attributes when stripping beams](erlware/relx#906) - [Fix {include_erts,true} in Windows releases](erlware/relx#914) - [ensure the erl file is writable before copying dyn_erl to it](erlware/relx#903) - Various tests added - [Properly carry overlay_vars settings for files in relx](#2711) - [Track mib compilation artifacts](#2709) - [Attempt to find apps in git subdirs (sparse checkouts)](#2687) - [Do not discard parameters --system_libs and --include-erts when duplicate values exist](#2695) - [Use default `depth` parameter for SSL](#2690) - [Fix global cache config overriding](#2683) - [Error out on unknown templates in 'new' command](#2676) - [Fix a typo](#2674) - [Bump certifi to 2.9.0](#2673) - [add a debug message in internal dependency fetching code](#2672) - [Use SPDX id for license in template and test](#2668) - [Use default branch for git and git_subdir resources with no revision](#2663) Signed-off-by: Fred Hebert <[email protected]>

ferd force-pushed the support-project-local-plugins branch from e770176 to dabfa48 Compare April 9, 2022 22:56

ferd force-pushed the support-project-local-plugins branch from dabfa48 to d4cd09e Compare April 9, 2022 23:11

ferd added 2 commits April 10, 2022 00:30

Fix local plugin external plugin dependency

bbbe561

This sounds weird as hell, but essentially, when a local plugin relies on an external plugin, make sure that we fetch and build that one in an orderly manner. This was found by adding initial tests.

ferd force-pushed the support-project-local-plugins branch from 9a4f98c to 35d92ae Compare April 10, 2022 01:48

Prevent non-umbrella project-local plugins

c835049

Only umbrella projects can have these.

ferd force-pushed the support-project-local-plugins branch from 35d92ae to c835049 Compare April 10, 2022 01:49

ferd marked this pull request as ready for review April 10, 2022 02:22

ferd requested a review from tsloughter April 10, 2022 02:22

ferd added 3 commits April 10, 2022 04:31

normalize debug output

17abf02

ferd force-pushed the support-project-local-plugins branch 9 times, most recently from 1cd5e01 to 32099f6 Compare April 10, 2022 21:09

give deps for shelltests

7667b7b

ferd force-pushed the support-project-local-plugins branch from 32099f6 to 7667b7b Compare April 10, 2022 22:27

tsloughter reviewed Apr 26, 2022

View reviewed changes

tsloughter approved these changes Apr 26, 2022

View reviewed changes

Going for more profiles than default and prod

2af0af1

ferd merged commit d574535 into erlang:main Apr 27, 2022

ferd mentioned this pull request Jul 12, 2022

Support local plugins with is_umbrella settings #2729

Closed

ferd mentioned this pull request Nov 11, 2022

Experimental vendoring provider #2689

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for project-local plugins #2697

Add support for project-local plugins #2697

ferd commented Apr 9, 2022 •

edited

Loading

ferd commented Apr 10, 2022

starbelly commented Apr 10, 2022 •

edited

Loading

tsloughter Apr 26, 2022

ferd Apr 26, 2022

tsloughter Apr 26, 2022

tsloughter Apr 26, 2022

ferd Apr 27, 2022

ferd Apr 27, 2022

tsloughter commented Apr 26, 2022

Add support for project-local plugins #2697

Add support for project-local plugins #2697

Conversation

ferd commented Apr 9, 2022 • edited Loading

ferd commented Apr 10, 2022

starbelly commented Apr 10, 2022 • edited Loading

tsloughter Apr 26, 2022

Choose a reason for hiding this comment

ferd Apr 26, 2022

Choose a reason for hiding this comment

tsloughter Apr 26, 2022

Choose a reason for hiding this comment

tsloughter Apr 26, 2022

Choose a reason for hiding this comment

ferd Apr 27, 2022

Choose a reason for hiding this comment

ferd Apr 27, 2022

Choose a reason for hiding this comment

tsloughter commented Apr 26, 2022

ferd commented Apr 9, 2022 •

edited

Loading

starbelly commented Apr 10, 2022 •

edited

Loading