Change xref paths to only those of runtime deps

[pablocostass]

This commit changes mainly the rebar_paths module by adding a
new runtime target. When using that target, rebar_paths:get_apps/2
will, for each project app, find all their applications and
included_applicactions and filter those that could be found
in rebar3's state.

[pablocostass]

@ferd IIRC you were the one who reworked how rebar3 handles the paths, so if this change should instead be [deps, runtime] or anything else, let me know :)

[ferd]

The [deps, runtime] approach would basically say "load all the deps, then put the runtime deps after them in the path". We use that form for [deps, plugins] or [plugins, deps] when we need both to be present, but favor one type of dependency if some of the apps in there clash (many versions exist).

Since the objective of this PR is to run only on runtime deps, then I'd expect the current [runtime] to be correct.

[pablocostass]

Also this PR has yet to change (as I think that it should do so) the paths test suite to reflect these changes, but I'm a bit lost. I guess I should be making a new test case with an application that has runtime deps, but I'm afraid I might need some guidance to do that :/

[tsloughter]

I'm not sure there should be any change to the paths when running tests.

[pablocostass]

I'm not sure there should be any change to the paths when running tests.

AFAIK there isn't , but as you and Fred have more knowledge on the subject I wanted to raise the question to be sure.

[ferd]

The [deps, runtime] approach would basically say "load all the deps, then put the runtime deps after them in the path". We use that form for [deps, plugins] or [plugins, deps] when we need both to be present, but favor one type of dependency if some of the apps in there clash (many versions exist).

Since the objective of this PR is to run only on runtime deps, then I'd expect the current [runtime] to be correct.

[ferd]

This looks good. I'm trying to think of a way we could make that work for tests to confirm the change works as desired here.

[ferd]

Since testing the paths is hard, one thing we could do is test the new behaviour of xref explicitly (seeing that it complains about a transitive app not being included). This will end up giving accidental coverage to the rebar_paths changeset.

By extension though it means I'd expect us to be able to write a proper test for rebar_paths as well.

I'd expect we could do something like adding a top-level app in

rebar3/test/rebar_paths_SUITE.erl

Lines 17 to 56 in d11ceb4

	init_per_testcase(Case, Config) ->
	BasePaths = code:get_path(),
	%% This test checks that the right module sets get loaded; however, we must
	%% ensure that we do not have clashes with other test suites' loaded modules,
	%% which we cannot track. As such, we have to ensure all module names here are
	%% unique.
	%%
	%% This is done by hand; if you see this test suite failing on its own, you
	%% probably wrote a test suite that clashes!
	Dir = filename:join([?config(priv_dir, Config), atom_to_list(?MODULE),
	atom_to_list(Case)]),
	InDir = fun(Path) -> filename:join([Dir, Path]) end,
	ADep = fake_app(<<"rp_a">>, <<"1.0.0">>, InDir("_build/default/lib/rp_a/")),
	BDep = fake_app(<<"rp_b">>, <<"1.0.0">>, InDir("_build/default/lib/rp_b/")),
	CDep = fake_app(<<"rp_c">>, <<"1.0.0">>, InDir("_build/default/lib/rp_c/")),
	DDep = fake_app(<<"rp_d">>, <<"1.0.0">>, InDir("_build/default/lib/rp_d/")),
	RelxDep = fake_app(<<"relx">>, <<"1.0.0">>, InDir("_build/default/lib/relx/")),
	APlug = fake_app(<<"rp_a">>, <<"1.0.0">>,
	InDir("_build/default/plugins/lib/rp_a/")),
	RelxPlug = fake_app(<<"relx">>, <<"1.1.1">>,
	InDir("_build/default/plugins/lib/relx")),
	EPlug = fake_app(<<"rp_e">>, <<"1.0.0">>,
	InDir("_build/default/plugins/lib/rp_e/")),
	S0 = rebar_state:new(),
	S1 = rebar_state:all_deps(S0, [ADep, BDep, CDep, DDep, RelxDep]),
	S2 = rebar_state:all_plugin_deps(S1, [APlug, RelxPlug]),
	S3 = rebar_state:code_paths(S2, default, code:get_path()),
	S4 = rebar_state:code_paths(
	S3,
	all_deps,
	[rebar_app_info:ebin_dir(A) || A <- [ADep, BDep, CDep, DDep, RelxDep]]
	),
	S5 = rebar_state:code_paths(
	S4,
	all_plugin_deps,
	[rebar_app_info:ebin_dir(A) || A <- [APlug, RelxPlug, EPlug]]
	),
	[{base_paths, BasePaths}, {root_dir, Dir}, {state, S5} | Config].

which specifically relies on say rp_a and rp_b but none of the other apps. This will let us make sure that both these apps are visible during runtime deps being in scope but not in other cases.

Then we can add a test case like:

runtime_paths(Config) ->
    State = ?config(state, Config),
    RootDir = filename:split(?config(root_dir, Config)),
    %% Take a snapshot of runtime deps being set; to see if your test is valid, this should fail
    %% when you set the [deps] paths nere
    rebar_paths:set_paths([runtime], State),
    RuntimePaths = code:get_path(),
    %% Revert back to regular dep paths
    rebar_paths:set_paths([deps, plugins], State),
    DepPaths = code:get_path(),

    ?assertEqual(
       RootDir ++ ["_build", "default", "lib", "rp_a", "ebin"],
       find_first_instance("rp_a", RuntimePaths)
    ),
    ?assertEqual(
       RootDir ++ ["_build", "default", "lib", "rp_b", "ebin"],
       find_first_instance("rp_b", RuntimePaths)
    ),
    ?assertMatch(
       {not_found, _},
       find_first_instance("rp_c", RuntimePaths)
    ),
    ?assertEqual(
       {not_found, _},
       find_first_instance("rp_d", RuntimePaths)
    ),
    ?assertEqual(
       {not_found, _},
       find_first_instance("rp_e", RuntimePaths)
    ),
    ?assertEqual(
       {not_found, _},
       find_first_instance("relx", RuntimePaths)
    ),
    ok.

You could likely fold that part of the test into the existing set_paths/1 test too by setting runtime paths before checking for plugin paths, which would properly ensure that unsetting paths also works well.

[pablocostass]

@ferd Sorry this one took a bit of time to be updated, I had some errands to finish first. Thanks a lot for the detailed explanation of how to approach the changes to the test suite, it helped a lot! :)

[ferd]

Thanks for this! this is a cool fix to some hairy core utility code.