`rebar3 shell` mangles path after `r3:do(compile)`

eproxus commented 2 years ago

With QuickCheck installed in the lib folder of my Erlang installation (as is the default when running the QuickCheck installer) rebar3 as test shell removes it from the path after subsequent calls to r3:do(compile).

Environment

Add the result of rebar3 report to your message:

$ rebar3 report "as test shell"
Rebar3 report
 version 3.18.0
 generated at 2022-03-02T08:57:20+00:00
=================
Please submit this along with your issue at https://github.com/erlang/rebar3/issues (and feel free to edit out private information, if any)
-----------------
Task: as
Entered as:
  as test shell
-----------------
Operating System: x86_64-apple-darwin21.3.0
ERTS: Erlang/OTP 24 [erts-12.2.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]
Root Directory: /Users/user/.asdf/installs/erlang/24.2.2
Library directory: /Users/user/.asdf/installs/erlang/24.2.2/lib
-----------------
Loaded Applications:
bbmustache: 1.12.2
certifi: 2.9.0
cf: 0.3.1
common_test: 1.22
compiler: 8.0.4
crypto: 5.0.5
cth_readable: 1.5.1
dialyzer: 4.4.3
edoc: 1.1
erlware_commons: 1.5.0
eunit: 2.7
eunit_formatters: 0.5.0
getopt: 1.0.1
inets: 7.5.1
kernel: 8.2
providers: 1.9.0
public_key: 1.11.3
relx: 4.6.0
sasl: 4.1.1
snmp: 5.11
ssl_verify_fun: 1.1.6
stdlib: 3.17
syntax_tools: 2.6
tools: 3.5.2

-----------------
Escript path: /Users/user/Software/rebar3/rebar3
Providers:
  app_discovery as build clean compile compile cover ct cut demo deps dialyzer do edoc escriptize eunit get-deps help install install_deps list lock new organization owner path pkgs publish release relup report repos retire search shell state tar tree unlock update upgrade upgrade upgrade user version xref

Current behaviour

$ rebar3 as test shell
===> Verifying dependencies...
===> Analyzing applications...
===> Compiling my_app
Erlang/OTP 24 [erts-12.2.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Eshell V12.2.1  (abort with ^G)
1> [P || P <- code:get_path(), string:find(P, "eqc") =/= nomatch].
["/Users/user/.asdf/installs/erlang/24.2.2/lib/eqc-1.45.1/ebin"]
2> r3:do(compile).
===> This feature is experimental and may be modified or removed at any time.
Verifying dependencies...
Analyzing applications...
Compiling my_app
ok
3> [P || P <- code:get_path(), string:find(P, "eqc") =/= nomatch].
[]

Expected behaviour

The default Erlang path is not destroyed when re-compiling code.

eproxus commented 2 years ago

If the directory is added back with either code:add_patha/1 or code:add_pathz/1 it is removed again. However, it is possible to add another directory from outside the OTP lib directory which is never removed.

ferd commented 2 years ago

My initial guess is that this is because r3:do(compile) will not contain the test profile on the next invocation. You may get less confusion by calling r3:do(as, "test compile") or something like that first. I'm not quite sure what the syntax was to re-invoke under a profile.

ferd commented 2 years ago

Ugh, I get something working with r3:do(do, "default as test compile"). -- other formats I think don't inherit the magic profile-managing of the rebar3 CLI. Another option would be to set the OS REBAR_PROFILE var.

eproxus commented 2 years ago

The issue seems to be with extra_src_drs. If I configure with {extra_src_dirs, ["test/kernel"]} the path to the OTP kernel application is removed (!).

A workaround for my particular situation is to use {extra_src_dirs, [{"test", [{recursive, true}]}]}. which doesn't exhibit the same issue.

ferd commented 2 years ago

Adding context from the slack debugging session:

The reason the recursive approach likely juggles things around the paths because it ends up in <whatever>/test/ebin rather than <whatever>/test/kernel/ebin and that de-confuses paths. The path is always a sort of <pathname>/ebin -- recursive source compiling goes look into all sub-directories for source files, but the VM only always supports a flat ebin/, so we collapse the recursive source into a single top-level ebin.

I would generally not expect the bug to be in rebar_paths because I recall writing it to really use app-provided paths (https://github.com/erlang/rebar3/blob/main/src/rebar_paths.erl#L200-L206). I'm thinking the bug could actually be in https://github.com/erlang/rebar3/blob/main/src/rebar_agent.erl#L219 where we just call code:replace_path(App, Path) https://github.com/erlang/otp/blob/eccc556e79f315d1f87c10fb46f2c4af50a63f20/lib/kernel/src/code_server.erl#L904-L915

Which seems to be the thing that gets confused:

%% Replace an old occurrence of an directory with name .../Name[-*].
%% If it does not exist, put the new directory last in Path.

so that code in OTP prioritize the selection of the OTP-provided directory and removes it in favor of the newer one that has the same name

so uh, no. I'm not sure it's actually a rebar3 bug? We update the path to force a reload, but the code in OTP is the one that gets confused... (this stuff is messy, that's a cool bug!) I think the discriminating factor for blame will be which app we're trying to swap, maybe, and we should be able to see that with a run that defines DIAGNOSTIC=1

eproxus commented 2 years ago

Here's a (anonymized) diagnostic log:

$ DIAGNOSTIC=1 rebar3 as test shell
===> Load global config file /Users/user/.config/rebar3/rebar.config
===> 24.2.2 satisfies the requirement for minimum OTP version 18
===> Evaluating config script "/Users/user/.cache/rebar3/24.2.2/plugins/hex_core/rebar.config.script"
===> 24.2.2 satisfies the requirement for minimum OTP version 19.3
===> Setting paths to [deps]
===> Compile (apps)
===> Setting paths to [plugins]
===> Setting paths to [deps]
===> Setting paths to [plugins]
===> Setting paths to [plugins]
===> Expanded command sequence to be run: [as]
===> Running provider: as
===> Expanded command sequence to be run: [app_discovery,install_deps,lock,compile,shell]
===> Running provider: app_discovery
===> Found top-level apps: [my_app]
        using config: [{src_dirs,["src"]},{lib_dirs,["apps/*","lib/*","."]}]
===> Evaluating config script "/Users/user/my_app/_build/default/lib/jsx/rebar.config.script"
===> Running provider: install_deps
===> Verifying dependencies...
===> sh info:
        cwd: "/Users/user/my_app"
        cmd: git --version

===>    opts: []

===> Port Cmd: git --version
Port Opts: [exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

===> sh info:
        cwd: "/Users/user/my_app"
        cmd: git rev-parse --short=7 -q HEAD

===>    opts: [{cd,"/Users/user/my_app/_build/default/lib/my_dep"}]

===> Port Cmd: git rev-parse --short=7 -q HEAD
Port Opts: [{cd,"/Users/user/my_app/_build/default/lib/my_dep"},
            exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

===> Comparing git ref d233d5b with d233d5b
===> Running provider: lock
===> Running provider: compile
===> Setting paths to [deps]
===> Compile (apps)
===> Setting paths to [plugins]
===> Setting paths to [deps]
===> Running hooks for compile with configuration:
===>    {pre_hooks, []}.
===> run_hooks("/Users/user/my_app", pre_hooks, compile) -> no hooks defined

===> sh info:
        cwd: "/Users/user/my_app"
        cmd: mkdir -p /Users/user/my_app/_build/test/lib/my_app/test

===>    opts: [{use_stdout,false},abort_on_error]

===> Port Cmd: mkdir -p /Users/user/my_app/_build/test/lib/my_app/test
Port Opts: [exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

===> sh info:
        cwd: "/Users/user/my_app"
        cmd: cp -Rp /Users/user/my_app/test/my_app_eqc_SUITE.erl /Users/user/my_app/test/my_app_SUITE_data /Users/user/my_app/test/my_app_SUITE.erl /Users/user/my_app/test/kernel "/Users/user/my_app/_build/test/lib/my_app/test"

===>    opts: [{use_stdout,true},abort_on_error]

===> Port Cmd: cp -Rp /Users/user/my_app/test/my_app_eqc_SUITE.erl /Users/user/my_app/test/my_app_SUITE_data /Users/user/my_app/test/my_app_SUITE.erl /Users/user/my_app/test/kernel "/Users/user/my_app/_build/test/lib/my_app/test"
Port Opts: [exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

===> sh info:
        cwd: "/Users/user/my_app"
        cmd: mkdir -p /Users/user/my_app/_build/test/lib/my_app/test/kernel

===>    opts: [{use_stdout,false},abort_on_error]

===> Port Cmd: mkdir -p /Users/user/my_app/_build/test/lib/my_app/test/kernel
Port Opts: [exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

===> sh info:
        cwd: "/Users/user/my_app"
        cmd: cp -Rp /Users/user/my_app/test/kernel/my_app_srv2_component.erl /Users/user/my_app/test/kernel/my_app_srv1_component.erl /Users/user/my_app/test/kernel/my_app_display_component.erl /Users/user/my_app/test/kernel/my_app_drv_component.erl "/Users/user/my_app/_build/test/lib/my_app/test/kernel"

===>    opts: [{use_stdout,true},abort_on_error]

===> Port Cmd: cp -Rp /Users/user/my_app/test/kernel/my_app_srv2_component.erl /Users/user/my_app/test/kernel/my_app_srv1_component.erl /Users/user/my_app/test/kernel/my_app_display_component.erl /Users/user/my_app/test/kernel/my_app_drv_component.erl "/Users/user/my_app/_build/test/lib/my_app/test/kernel"
Port Opts: [exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

===> Compile (project_apps)
===> Running hooks for compile in app my_app (/Users/user/my_app) with configuration:
===>    {pre_hooks, [{"(linux)",compile,"make -C c_src"}]}.
===> Running hooks for erlc_compile in app my_app (/Users/user/my_app) with configuration:
===>    {pre_hooks, []}.
===> Setting paths to [deps]
===> Analyzing applications...
===> Compiling my_app
===> compile options: {erl_opts, [debug_info,{d,'TEST'}]}.
===> files to analyze ["/Users/user/my_app/src/my_app_srv2.erl",
                              "/Users/user/my_app/src/my_app_drv.erl",
                              "/Users/user/my_app/src/my_app_ebcontroller.erl",
                              "/Users/user/my_app/src/my_app_mngmt.erl",
                              "/Users/user/my_app/src/my_app_client.erl",
                              "/Users/user/my_app/src/my_app_client_ws.erl",
                              "/Users/user/my_app/src/my_app_display.erl",
                              "/Users/user/my_app/src/my_app_sup.erl",
                              "/Users/user/my_app/src/my_app_inotify.erl",
                              "/Users/user/my_app/src/my_app_cert.erl",
                              "/Users/user/my_app/src/my_app_app.erl",
                              "/Users/user/my_app/src/my_app_tls.erl",
                              "/Users/user/my_app/src/my_app_srv1.erl",
                              "/Users/user/my_app/src/my_app_img.erl"]
===> Starting 0 worker(s)
===> compile options: {erl_opts, [{i,
                                          "/Users/user/my_app/src"},
                                         debug_info,
                                         {d,'TEST'}]}.
===> files to analyze ["/Users/user/my_app/test/my_app_SUITE.erl",
                              "/Users/user/my_app/test/my_app_eqc_SUITE.erl"]
===> Starting 0 worker(s)
===> compile options: {erl_opts, [{i,
                                          "/Users/user/my_app/src"},
                                         debug_info,
                                         {d,'TEST'}]}.
===> files to analyze ["/Users/user/my_app/test/kernel/my_app_drv_component.erl",
                              "/Users/user/my_app/test/kernel/my_app_display_component.erl",
                              "/Users/user/my_app/test/kernel/my_app_srv1_component.erl",
                              "/Users/user/my_app/test/kernel/my_app_srv2_component.erl"]
===> Starting 0 worker(s)
===> Running hooks for erlc_compile in app my_app (/Users/user/my_app) with configuration:
===>    {post_hooks, []}.
===> Running hooks for app_compile in app my_app (/Users/user/my_app) with configuration:
===>    {pre_hooks, []}.
===> Setting paths to [plugins]
===> Setting paths to [deps]
===> Running hooks for app_compile in app my_app (/Users/user/my_app) with configuration:
===>    {post_hooks, []}.
===> Running hooks for compile in app my_app (/Users/user/my_app) with configuration:
===>    {post_hooks, []}.
===> Running hooks for compile with configuration:
===>    {post_hooks, []}.
===> run_hooks("/Users/user/my_app", post_hooks, compile) -> no hooks defined

===> Setting paths to [plugins]
===> Running provider: shell
Erlang/OTP 24 [erts-12.2.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Eshell V12.2.1  (abort with ^G)
1> ===> No script_file specified.

1> [P || P <- code:get_path(), string:find(P, "kernel") =/= nomatch].
["/Users/user/my_app/_build/test/lib/my_app/test/kernel",
 "/Users/user/.asdf/installs/erlang/24.2.2/lib/kernel-8.2/ebin"]

2> r3:do(compile).
===> This feature is experimental and may be modified or removed at any time.
===> Load global config file /Users/user/.config/rebar3/rebar.config
===> 24.2.2 satisfies the requirement for minimum OTP version 18
===> Evaluating config script "/Users/user/.cache/rebar3/24.2.2/plugins/hex_core/rebar.config.script"
===> 24.2.2 satisfies the requirement for minimum OTP version 19.3
===> Setting paths to [deps]
===> Compile (apps)
===> Setting paths to [plugins]
===> Setting paths to [deps]
===> Setting paths to [plugins]
===> Setting paths to [plugins]
Expanded command sequence to be run: [do]
Running provider: do
Expanded command sequence to be run: [app_discovery,install_deps,lock,compile]
Running provider: app_discovery
Found top-level apps: [my_app]
        using config: [{src_dirs,["src"]},{lib_dirs,["apps/*","lib/*","."]}]
Evaluating config script "/Users/user/my_app/_build/default/lib/jsx/rebar.config.script"
Running provider: install_deps
Verifying dependencies...
sh info:
        cwd: "/Users/user/my_app"
        cmd: git rev-parse --short=7 -q HEAD

        opts: [{cd,"/Users/user/my_app/_build/default/lib/my_dep"}]

Port Cmd: git rev-parse --short=7 -q HEAD
Port Opts: [{cd,"/Users/user/my_app/_build/default/lib/my_dep"},
            exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

Comparing git ref d233d5b with d233d5b
Running provider: lock
Running provider: compile
Setting paths to [deps]
Compile (apps)
Setting paths to [plugins]
Setting paths to [deps]
Running hooks for compile with configuration:
        {pre_hooks, []}.
run_hooks("/Users/user/my_app", pre_hooks, compile) -> no hooks defined

sh info:
        cwd: "/Users/user/my_app"
        cmd: mkdir -p /Users/user/my_app/_build/test/lib/my_app/test

        opts: [{use_stdout,false},abort_on_error]

Port Cmd: mkdir -p /Users/user/my_app/_build/test/lib/my_app/test
Port Opts: [exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

sh info:
        cwd: "/Users/user/my_app"
        cmd: cp -Rp /Users/user/my_app/test/my_app_eqc_SUITE.erl /Users/user/my_app/test/my_app_SUITE_data /Users/user/my_app/test/my_app_SUITE.erl /Users/user/my_app/test/kernel "/Users/user/my_app/_build/test/lib/my_app/test"

        opts: [{use_stdout,true},abort_on_error]

Port Cmd: cp -Rp /Users/user/my_app/test/my_app_eqc_SUITE.erl /Users/user/my_app/test/my_app_SUITE_data /Users/user/my_app/test/my_app_SUITE.erl /Users/user/my_app/test/kernel "/Users/user/my_app/_build/test/lib/my_app/test"
Port Opts: [exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

sh info:
        cwd: "/Users/user/my_app"
        cmd: mkdir -p /Users/user/my_app/_build/test/lib/my_app/test/kernel

        opts: [{use_stdout,false},abort_on_error]

Port Cmd: mkdir -p /Users/user/my_app/_build/test/lib/my_app/test/kernel
Port Opts: [exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

sh info:
        cwd: "/Users/user/my_app"
        cmd: cp -Rp /Users/user/my_app/test/kernel/my_app_srv2_component.erl /Users/user/my_app/test/kernel/my_app_srv1_component.erl /Users/user/my_app/test/kernel/my_app_display_component.erl /Users/user/my_app/test/kernel/my_app_drv_component.erl "/Users/user/my_app/_build/test/lib/my_app/test/kernel"

        opts: [{use_stdout,true},abort_on_error]

Port Cmd: cp -Rp /Users/user/my_app/test/kernel/my_app_srv2_component.erl /Users/user/my_app/test/kernel/my_app_srv1_component.erl /Users/user/my_app/test/kernel/my_app_display_component.erl /Users/user/my_app/test/kernel/my_app_drv_component.erl "/Users/user/my_app/_build/test/lib/my_app/test/kernel"
Port Opts: [exit_status,
            {line,16384},
            use_stdio,stderr_to_stdout,hide,eof,binary]

Compile (project_apps)
Running hooks for compile in app my_app (/Users/user/my_app) with configuration:
        {pre_hooks, [{"(linux)",compile,"make -C c_src"}]}.
Running hooks for erlc_compile in app my_app (/Users/user/my_app) with configuration:
        {pre_hooks, []}.
Setting paths to [deps]
Analyzing applications...
Compiling my_app
compile options: {erl_opts, [debug_info,{d,'TEST'}]}.
files to analyze ["/Users/user/my_app/src/my_app_srv2.erl",
                  "/Users/user/my_app/src/my_app_drv.erl",
                  "/Users/user/my_app/src/my_app_ebcontroller.erl",
                  "/Users/user/my_app/src/my_app_mngmt.erl",
                  "/Users/user/my_app/src/my_app_client.erl",
                  "/Users/user/my_app/src/my_app_client_ws.erl",
                  "/Users/user/my_app/src/my_app_display.erl",
                  "/Users/user/my_app/src/my_app_sup.erl",
                  "/Users/user/my_app/src/my_app_inotify.erl",
                  "/Users/user/my_app/src/my_app_cert.erl",
                  "/Users/user/my_app/src/my_app_app.erl",
                  "/Users/user/my_app/src/my_app_tls.erl",
                  "/Users/user/my_app/src/my_app_srv1.erl",
                  "/Users/user/my_app/src/my_app_img.erl"]
Starting 0 worker(s)
compile options: {erl_opts, [{i,"/Users/user/my_app/src"},
                             debug_info,
                             {d,'TEST'}]}.
files to analyze ["/Users/user/my_app/test/my_app_SUITE.erl",
                  "/Users/user/my_app/test/my_app_eqc_SUITE.erl"]
Starting 0 worker(s)
compile options: {erl_opts, [{i,"/Users/user/my_app/src"},
                             debug_info,
                             {d,'TEST'}]}.
files to analyze ["/Users/user/my_app/test/kernel/my_app_drv_component.erl",
                  "/Users/user/my_app/test/kernel/my_app_display_component.erl",
                  "/Users/user/my_app/test/kernel/my_app_srv1_component.erl",
                  "/Users/user/my_app/test/kernel/my_app_srv2_component.erl"]
Starting 0 worker(s)
Running hooks for erlc_compile in app my_app (/Users/user/my_app) with configuration:
        {post_hooks, []}.
Running hooks for app_compile in app my_app (/Users/user/my_app) with configuration:
        {pre_hooks, []}.
Setting paths to [plugins]
Setting paths to [deps]
Running hooks for app_compile in app my_app (/Users/user/my_app) with configuration:
        {post_hooks, []}.
Running hooks for compile in app my_app (/Users/user/my_app) with configuration:
        {post_hooks, []}.
Running hooks for compile with configuration:
        {post_hooks, []}.
run_hooks("/Users/user/my_app", post_hooks, compile) -> no hooks defined

Setting paths to [plugins]
Removing [deps,plugins] paths
reloading [my_app_srv2_component,my_app_display_component,my_app_drv_component,
           my_app_srv1_component] from /Users/user/my_app/_build/test/lib/my_app/test/kernel
ok

3> [P || P <- code:get_path(), string:find(P, "kernel") =/= nomatch].
["/Users/user/my_app/_build/test/lib/my_app/test/kernel"]

ferd commented 2 years ago

Okay, nice. So like all annoying bugs, it's a bit of everything:

https://github.com/erlang/rebar3/blob/76039f91025eb9c2c94653261ae590f430f45cdd/src/rebar_agent.erl#L194-L220

Lines 195-196 misidentify the app being reloaded as kernel because it uses the trailing end of the path as the app name. Mostly there's little choice there because the extra dirs do not have a proper app name containing them. Then once that path is refreshed for the modules at lines 216-219, the right modules in the right path are identified, but since the app name clashes (kernel), the OTP code drops the other one.

I'm surprised directory stickiness does not prevent this from happening
I have no idea how we can fix this safely (we could probably not call code:replace_path() if it's not about an OTP app but I don't know if that would break reloading there)
I also have a hard time thinking of a way to safely warn the user there.

erlang / rebar3