rebar3 icon indicating copy to clipboard operation
rebar3 copied to clipboard

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

Open miquecg opened this issue 11 months ago • 7 comments

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 avatar Jul 28 '23 14:07 miquecg

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

miquecg avatar Jul 28 '23 14:07 miquecg

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.

ferd avatar Jul 28 '23 18:07 ferd

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.

miquecg avatar Jul 31 '23 10:07 miquecg

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:

https://github.com/erlang/rebar3/blob/c46bec6abe6fcf62d63343df5c266aa014454ff5/apps/rebar/src/rebar_utils.erl#L253-L255

https://github.com/erlang/rebar3/blob/c46bec6abe6fcf62d63343df5c266aa014454ff5/apps/rebar/src/rebar_utils.erl#L871-L883

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 avatar Jul 31 '23 13:07 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.

ferd avatar Jul 31 '23 14:07 ferd

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

miquecg avatar Jul 31 '23 14:07 miquecg

See https://github.com/erlang/rebar3/pull/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.

ferd avatar Jul 31 '23 15:07 ferd