Remove duplicated message when trying to upgrade a checkout dep

[pablocostass]

When running rebar3 upgrade on a project that has checkout dependencies,
a message of "App $NAME is a checkout dependency and cannot be locked."
will be printed by the lock provider.

As this provider is called twice (first to check the locked dependencies,
and then from the upgrade provider after updating the dependencies),
the message will show duplicated. To avoid this, this commit marks
all checkout dependencies as regular ones, after checking whether it makes
sense to do so, before calling the lock provider from the upgrade
provider.

Fixes #2466.

[pablocostass]

This was already addresses by #2335, I think, but I forgot to remove it then.

[pablocostass]

Sometimes the checkouts list will have duplicated dep names, this should also help remove duplicated messages.

[ferd]

I believe this will now hide status of whether a dep is a checkout dep to any provider that relies on locking (i.e. compiling). This may break a lot of stuff.

[pablocostass]

Yeah, I thought so too. If the state that lock uses has no checkout apps, it will definitely be passed onto other providers that lock calls/relies on.

Thanks for bringing it up, because for a second I forgot that although the code executed by lock will not have any issues, the provider returns its own state for others to use.

Would it make sense then to, in line 115, match on {ok, State7} = rebar_prv_lock:do(State6) and patch again the state by marking again as checkouts accordingly the dependencies?

[ferd]

That might work. It could possibly be simpler to just keep a quick cache entry about it. The rebar agent does it internally by carrying explicit state:

rebar3/src/rebar_agent.erl

Lines 166 to 172 in c102379

	%% @private function to display a warning for the feature only once
	-spec maybe_show_warning(#state{}) -> #state{}.
	maybe_show_warning(S=#state{show_warning=true}) ->
	?WARN("This feature is experimental and may be modified or removed at any time.", []),
	S#state{show_warning=false};
	maybe_show_warning(State) ->
	State.

The config handler uses the app env values to do it:

rebar3/src/rebar_config.erl

Lines 97 to 112 in ba50a08

	%% @private outputs a warning for a newer lockfile format than supported
	%% at most once.
	%% The warning can also be cancelled by configuring the `warn_config_vsn'
	%% OTP env variable.
	-spec warn_vsn_once() -> ok.
	warn_vsn_once() ->
	Warn = application:get_env(rebar, warn_config_vsn) =/= {ok, false},
	application:set_env(rebar, warn_config_vsn, false),
	case Warn of
	false -> ok;
	true ->
	?WARN("Rebar3 detected a lock file from a newer version. "
	"It will be loaded in compatibility mode, but important "
	"information may be missing or lost. It is recommended to "
	"upgrade Rebar3.", [])
	end.

I like that one because it also allows people to tweak or change the cache behaviour in weird cases like tests.

The pdict could work in more restrained cases.

[pablocostass]

Yeah using app environment is a nice thing that rebar3 uses in some features. I do not think, however, it would translate well into removing a duplicated message :/

As for the cache, the idea is good, but given that the lock provider does not modify the list of dependencies, the simplest approach IMO is to keep said list in a variable and after calling that provider from upgrade, setting the list of dependencies of the new state to that. No need to introduce a record/cache to keep track of the list when the use case is this minimal, although I will keep it in mind for other providers :)