Rebar3 can't call Module:download/3 from rebar_fetch download_source_/3

[tothlac]

The download function is implemented by a plugin (mirroring). It works in most cases, but sometimes when there are lots of plugins/deps the mirroring plugin is unlodaded hence I get

{undef,  rebar3_git_mirroring, download, ....}

in the logs.

If I debug it, the result from code:get_path() does not contain the plugin. On the other hand mirroring plugin was in the code path in the previous step when an other plugin was mirrored. Now for some reason it was deleted from the code path by rebar3.

If I add all already downloaded plugins back to the code path before calling Module:download/3 in download_source_, like this:

download_source_(AppDir, Source, State) ->
    Resources = rebar_state:resources(State),
    Module = get_resource_type(Source, Resources),
    TmpDir = ec_file:insecure_mkdtemp(),
    AppDir1 = rebar_utils:to_list(AppDir),
++    PluginDepsPaths = rebar_state:code_paths(State, all_plugin_deps),
++    code:add_pathsa(PluginDepsPaths),
    case Module:download(TmpDir, Source, State) of

, it works because the plugin is in the code path. Maybe it would be an even better solution to add it back to the code path right from rebar_plugins:project_plugins_install/1 and rebar_plugins:top_level_install/1.

[ferd]

This is on 3.6.1?

We had a very long series of fixes about this and the code paths... It is not necessarily safe to do it there, but it might be fine. The problem is that the set of plugins is tracked in multiple places because the compiler requires global paths to be set to do its job, so there are cases where this can be risky around plugins' dependencies handling vs. files handling.

See #1650, #1668, #1678, #1681, #1686, #1698 for relevant issues. Fixes and issues here have to be coordinated with these to make sure we don't have regressions on other behaviours and plugins. These things should mostly be tested, so any patch should at the very least pass the test suites.

I'm starting to think more and more that shelling out to a compiler would become a smarter issue since we could decouple rebar3's path from the compiler's path. This was a big surprise to us later down the line; we got a speedup from using the internal compiler, but the complexity got much much higher, and doing all of that dynamic path management is probably starting to take its toll in terms of performance.

[tothlac]

That happened with this version:

tothlac:cfg rebar3 --version
rebar 3.6.1+build.4118.ref940b1c59 on Erlang/OTP 20 Erts 9.3

This is as I see the same issue: #1858

rebar_utils:remove_from_code_path(PluginDepsPaths),

deletes the plugins from the code path, that's why it was not able to find the hrl file in the other ticket, and the same reason here. BTW, why is it deleted from the code path after downloading a plugin?

[ferd]

It is likely deleted from the path after compilation is done or as part of the hooks. Each compile job sets up its environment and then cleans it up after it was required (you don't want the plugin for app A to impact the include path of app B if plugin for A has dependencies that are shared with user app B, since user dependencies and plugins dependencies should not interfere with each other).

That was part of why I thought the issue would have been resolved by adding dependencies, but something funky took place there. Possibly the problem is that the expectation in the compiler as we have it today is that all required deps are rebuilt at once, which is not true of plugins compiled piecemeal. In the context of a plugin getting built, we may need to explicitly re-add other plugins to the state.

[tothlac]

In this special case actually some other plugins were still in the code path and some other have been already deleted. Mirroring plugin was one of them. Would it be helpful to create a similar app having the same problem?

[ferd]

Yeah probably. Any reproducible test case is good to have.

[tothlac]

I will need to understand how it happened, then hopefully I will be able to create a similar app soon.

[garazdawi]

@ferd if it would help you, I don't see why we couldn't add an option to the compiler to not use the code path but instead use a supplied code path. Open an issue at bugs.erlang.org and we'll see what Björn has to say.

[ferd]

Hell yeah I will do that. I think it could do some major good to how we manage all the things

[ferd]

I think we would still have to deal with paths for parse transforms and so on though, so I'll have to think it through

[garazdawi]

Hmm yes, I'll see if I can think of a good way to manage that. Maybe we can play with the error_handler to make a per process code path... hmmm

[ferd]

https://bugs.erlang.org/browse/ERL-711 is the basic thing there anyway.

We could possibly play with the plugin metadata and xref to find whether we actually have to load/unload paths as well, rather than doing it all the time. If the subset of plugins does not clash with the subset of deps, then there should be no problem. It's possible that 99% of cases would have little overlap; it wouldn't be a complete solution, but would probably drastically decrease the number of moving parts around paths most projects would experience in a regular build.

[tothlac]

This small modification have fixed all problems we were facing, and the ci job also looks fine:

#1859

Can you take a look?

[tothlac]

Unfortunately it looks like even after the tests were not failing, and it solved the current problems there are still problems with this modification in even more complex cases.

[ferd]

Pretty sure we had this fixed when updating the path handling in 3.7.0. Reopen if I was wrong.