rebar can't compile a plugin

[tothlac]

If I have two plugins:

{project_plugins, [plugin1, plugin2]}.

And both of them use an include file defined in an application, let's call it plug_lib.

If I would have only plugin1 in rebar.config, it would work.

Now with two plugins I have the following:

Let's say plugin1 is compiled first. Now it can find the plug_lib.hrl file defined in plug_lib.

Now it wants to fetch/compile plugin2.
In order to compile the plugin rebar_plugins compiles its dependencies first.
Dependencies which have been earlier compiled won't be compiled again, so now plug_lib is not in the ToBuild variable in rebar_plugins:handle_plugin/4.

Unfortunately when rebar_prv_compile:compile/3 is called as plug_lib is in all_plugin_deps, it's unloaded when it calls rebar_utils:remove_from_code_path(PluginDepsPaths),

As plug_lib is not in the code path anymore because of the above reason when it reaches rebar_erlc_compiler:internal_erl_compile/6, it's complaining:

xxx.erl:43: can't find include lib "plug_lib/include/rplug_lib.hrl"; Make sure rplug_lib is in your app file's 'applications' list

This is false, since the plug_lib, with the include file is already there, as it was compiled when plugin1 was compiled, and it already worked with the other plugin.

[ferd]

Does plugin2 depend on plug_lib? Usually setting the dependency in the .app.src file's applications tuple solves it, as the error message implies.

Usually setting that value ensures that everything and all paths are reintroduced in the right order. You said that it is false, but the way you worded the text implies that you haven't tried it ("it should be here") but really, setting the value in the applications tuple has resolved this every single time I ever tried it.

If you can confirm that you tried that and it didn't work then we'll need to investigate, and providing a sample project to reproduce it (if possible) would be very helpful.

[tothlac]

I've created a minimal app to reproduce the same problem: https://github.com/tothlac/rebar_include_fail

If you checkout {tag, "1.0"}, where only incl_plug1 is used from rebar_incl_fail, it will work.

Now if you move to {tag, "2.0}, you will have this result:

tothlac:rebar_include_fail rm -rf _build/; rebar3 compile
===> Fetching incl_plug1 ({git,"git@github.com:tothlac/incl_plug1.git",
                                      {branch,"master"}})
===> Fetching plug_lib ({git,"git@github.com:tothlac/plug_lib.git",
                                    {branch,"master"}})
===> Compiling plug_lib
===> Compiling incl_plug1
incl_plug1 18 Answer: '84'
===> Fetching incl_plug2 ({git,"git@github.com:tothlac/incl_plug2.git",
                                      {branch,"master"}})
===> Fetching nop_plug ({git,"git@github.com:tothlac/nop_plug.git",
                                    {branch,"master"}})
===> Compiling nop_plug
===> Compiling plug_lib
===> Compiling incl_plug2
===> Compiling _build/default/plugins/incl_plug2/src/incl_plug2.erl failed
_build/default/plugins/incl_plug2/src/incl_plug2.erl:11: can't find include lib "plug_lib/include/plug_lib.hrl"
_build/default/plugins/incl_plug2/src/incl_plug2.erl:17: undefined macro 'ANSWER'

_build/default/plugins/incl_plug2/src/incl_plug2.erl:13: function init/1 undefined
_build/default/plugins/incl_plug2/src/incl_plug2.erl:15: spec for undefined function init/1

===> Plugin {incl_plug2,{git,"git@github.com:tothlac/incl_plug2.git",

It works like this:

        ---------> rebar_incl_fail  <--------
        |                                   |
        |                                   |
  incl_plug1                            incl_plug2 <------
       |                                  |              |
       |                                  |              |                   
   plug_lib                            plug_lib       nop_plug

plug_lib contains include/plug_lib.hrl file with a define in it.
This include file is used in incl_plug1.erl and in incl_plug2.erl.

Rebar3 will fetch&compile incl_plug1 first, then incl_plug2 is the next.
Most likely the reason is the following. When it compile incl_plug2, it starts with nop_plug, (without nop_plug actually it would work).
Now

        ToBuild = rebar_prv_install_deps:cull_compile(Sorted, []),

in rebar_plugins:handle_plugin does not give back rplug_lib, as cull_compile gives back only dependencies which has not been compiled earlier. So in the following line:

       code:add_pathsa(PreBuiltPaths)

ebin dir of rplug_lib won't be added to code path. Then in rebar_prv_compile:compile/3 code:add_pathsa(PuglinDepsPaths) , will add the same plugin once more, and rebar_utils:remove_from_code_path(PluginsPaths) will delete all plugin dirs from code path.

Now after nop_plug has been compiled it won't compile plug_lib as in the result from cull_compile it was not present, tries to compile incl_plug2, but rebar_utils:remove_from_code_path/1 has already removed all plugin directories from code path, hence we have the error.

I actually moved the code:add_pathsa call to the beginning of rebar_prv_compile:compile/3, and with that change it works perfectly.
I've created a PR: #1859

[ferd]

Confirmed that even with the applications tuple, this did not work.

[tothlac]

Unfortunately I see there are some failing tests on the PR, so must be an other solution for this.

[tothlac]

Is there any progress on this?

In the meantime I've made a small change. It does not modify rebar3's behaviour at all, if the unload_plugins option is not set. By default it is true. However, if there are problems with the plugins, so the plugin is accidentally unloaded, as it happens quite frequently, {unload_plugins, false} can guarantee rebar_utils:remove_from_code_path won't unload the plugin.

I've updated the old pr, and the tests are passing: #1859
bdfdc26

[ferd]

No progress on my end. I've basically been rushing things to make sure I can meet deadlines for my book and other real life interruptions and I have had zero time to give to open source work unrelated to my job that wouldn't jeopardize more important stuff IRL.

[tothlac]

The only thing why we would need this small modification is that:

• we have lots of plugins, so sometimes they fail with the plugin unloaded from code path error
• the mirroring plugin will never work without fixing this problem
• we can have our modifications in a separate repo, and we can use that but then we will continously have to rebase against your changes

I know this is not the proper fix, but I still don't know your original ideas about why and when plugins should be exactly unloaded. If I would know it, I would be able to make a proper fix, but without this....

On the other hand the modification I made is very small, and if you don't add {unload_plugins, false} option it won't have any impact on the old behaviour at all, but it fixes all of our issues. Would it be possible to temporarily add these changes and when we find a better solution this option could be deleted.

[ferd]

It's a small modification in terms of code, but that's a huge one in terms of behaviour and a public interface to provide. Ideally plugins are loaded and unloaded transparently. The bug is that apparently this is not done well within the current plugin compiler, which reuses the dependency compiler, but maybe it shouldn't. So your patch is one that cements that implementation detail into a user-controllable behaviour.

What if you set {unload_plugins, true} and that the compiler plugin work @tsloughter is doing right now makes that option difficult to spot in terms of which plugins it refers to? What if when we eventually do a good fix and the option is no longer necessary, leaving it in becomes a problem? As I've mentioned before, this stuff is currently being refactored, so it's even trickier to add a quickfix amidst the refactor and make sure it's right.

As a maintainer, I do prefer if you keep the changes on your end in the meanwhile and keep rebasing, especially if your projects are the only ones having these problems.

I know it is not a fun response to receive, and I know it is a bigger problem with the design of rebar3 that's ultimately our fault and not yours. However, I am really not enthusiastic about adding more cruft to it in a way that publicly enshrines a design problem as part of the API we expose. This is a rapid fix that ultimately prevents us from actually fixing anything before the next backwards-incompatible release.

[ferd]

@tothlac can you give this WIP branch a spin? https://github.com/ferd/rebar3/tree/refactor-env-paths
My own attempts with it seem to show the bug you mentioned being fixed by it on the test app.