rebar3 release doesn't parse --relnames arguments correctly when specifying a profile

[miquecg]

Environment

rebar3 3.22.0
Erlang/OTP 24
macOS Ventura (but probably every other platform is affected)

$ rebar3 as foo release -m one,two
...
Assembling of release one
===> Release successfully assembled: _build/foo/rel/one
===> Command two not found

Current behaviour

Release building with -m|--relnames option doesn't work when a profile is explicitly set (see example above). Name after first , is interpreted as a command. This behaviour is not observed when running on default profile.

Expected behaviour

Comma separated list of releases should be parsed correctly.

[miquecg]

Sorry for the barebones report. I can add a couple of examples next week, but I think it's pretty easy to reproduce.

[ferd]

does it work with rebar3 as foo release -m "one,two" ? Because otherwise, yeah that might require an impossible/incompatible overhaul of how command line options work in rebar3.

[miquecg]

It doesn't make any difference :(

How big of a change that would be? Just curious. Maybe for next big release, at this point I'm not even concerned by the issue.

[ferd]

I'm not 100% sure but my understanding is that this is due to how we implemented two providers: do and as (the latter of which calls to do). These take over the whole line argument parsing and break it down to allow sequences of calls such as rebar3 do compile,ct,dialyzer,release and rebar3 as profile do eunit, release and they visibly split a bit too aggressively.

Chances are we could better cover some cases, but maybe not all of them, without breaking other functionality. As it turns out, the comment in there specifically warned of this issue:

rebar3/apps/rebar/src/rebar_utils.erl

Lines 253 to 255 in c46bec6

	%% note: this does not handle the case where you have an argument that
	%% was enclosed in quotes and might have commas but should not be split.
	args_to_tasks(Args) -> new_task(Args, []).

rebar3/apps/rebar/src/rebar_utils.erl

Lines 871 to 883 in c46bec6

	new_task([], Acc) -> lists:reverse(Acc);
	new_task([TaskList|Rest], Acc) ->
	case re:split(TaskList, ",", [{return, list}, {parts, 2}, unicode]) of
	%% `do` consumes all remaining args
	["do" = Task] ->
	lists:reverse([{Task, Rest}|Acc]);
	%% single task terminated by a comma
	[Task, ""] -> new_task(Rest, [{Task, []}|Acc]);
	%% sequence of two or more tasks
	[Task, More] -> new_task([More|Rest], [{Task, []}|Acc]);
	%% single task not terminated by a comma
	[Task] -> arg_or_flag(Rest, [{Task, []}|Acc])
	end.

I'm guessing we could actually do some lookahead or lookbehind parsing without using regexes and deal with quoting better without breaking compatibility, but I'm not 100% sure.

[ferd]

So I've been adding tests, and it appears that the parsing at least currently works fine with long form arguments, which cancel out the ambiguity -- calling with --relname=one,two parses without problem. I figure the issue with -m one,two is that it's a bit more impossible to know if the second argument is to a switch or not. So that's at least an immediate workaround.

[miquecg]

Yeah, I've just tried with long option and it works as long as using = between option and arguments. So at least there's a workaround, thanks for finding that 🙂

The only missing thing would be maybe fixing rebar3 help release (or any other long option examples) to account for that.

e.g. --relnames rel1,rel2

[ferd]

See #2813 for a potential fix. Turns out we had a bunch of tests and I think it can work, but there are some backwards compat risks there that makes it possible we wouldn't include it even if it works there. We'll need to think about it.