OTP 23 [master] compatibility

[michaelklishin]

Team RabbitMQ tests OTP versions from 20.3 to master in our CI/CD pipelines. Recent OTP master changes introduce additional warnings that make Rebar bootstrap fail.

Unfortunately, there is no URI utility function module that's both available in OTP 20 and not deprecated in OTP 23. So this PR suppresses a few warnings for now. I hope that's acceptable.

[michaelklishin]

This is hilarious.

[tsloughter]

Nice, thanks.

And hah, yea, I guess you have to wrap the compile directive in a -ifdef(OTP_RELEASE).

[tsloughter]

The new module to use is uri_string in stdlib, right?

[michaelklishin]

@tsloughter yup, an ifdef(OTP_RELEASE) helped. The warning does recommend uri_string but it's not available in OTP <= 20 in my tests.

[michaelklishin]

CI is green now 👌

[tsloughter]

Yea, a better change would be to create new functions that wrap http_uri and uri_string and wrap those functions in -ifdef(OTP_RELEASE) so it uses the right ones depending on being OTP 20+ or not.

[tsloughter]

Thanks. I'll merge this. I'm looking to get a release out today but want to replace this change with use of uri_string soon. Let me know if you were looking at doing that or if I should do it.

[michaelklishin]

@tsloughter I'd be happy to do it. Thank you for the quick release!

[michaelklishin]

@tsloughter so I tried and it turns out to be less trivial than it sounds. Getting rid of http_uri:parse/1 is easy, even though not pretty:

%%% @doc multi-OTP version compatibility shim for working with URIs
-module(rebar_uri).

-ifdef (OTP_RELEASE).
    -export([parse/1]).

    -spec parse(URIString) -> URIMap when
        URIString :: uri_string:uri_string(),
        URIMap :: uri_string:uri_map() | uri_string:error().

    parse(URIString) ->
        uri_string:parse(URIString).
-else.
    -export([parse/1]).

    -spec parse(URIString) -> URIMap when
        URIString :: iodata(),
        URIMap :: map() | {error, atom(), term()}.

    parse(URIString) ->
        case http_uri:parse(URIString) of
            {error, Reason} ->
                %% no additional parser/term info available to us,
                %% e.g. see what uri_string returns in
                %% uri_string:parse(<<"h$ttp:::://////lolz">>).
                {error, Reason, ""};
            {ok, {Scheme, UserInfo, Host, Port, Path, Query}} ->
                #{
                    scheme => Scheme,
                    host => Host,
                    port => Port,
                    path => Path,
                    %% http_uri:parse/1 includes the leading question mark
                    %% in query string but uri_string:parse/1 leaves it out
                    query => string:slice(Query, 1),
                    userinfo => UserInfo
                }
        end.
-endif.

but there is no replacement for http_uri:encode/1 exposed in uri_string. This means that r3_hex_api:encode_query_string/1 has to go or be replaced entirely. I'm not familiar enough
with how Rebar API modules are used and what callers may be affected (plugins?)

So if you have a clear idea of how to approach this, go ahead and do it.

[starbelly]

Options:

• A patch release for that version of hex_core can be coordinated with hex team and then re-vendor the patched version into rebar3
• Patch r3_hex_api and send the patch upstream to hex_core

[michaelklishin]

@starbelly thanks. Reimplementing r3_hex_api:encode_query_string/1 is also an option.

I'd also question the need for OTP 18 and 19 support in almost 2020 but OTP 20 is also affected (doesn't ship uri_string) and it still gets patch releases every once in a while.

[michaelklishin]

Alternatively r3_hex_api_package, the sole user of r3_hex_api:encode_query_string/1, could be migrated to use something else.

[tsloughter]

Isn't uri_string:recompose the replacement?

[michaelklishin]

@tsloughter it will encode URI segments but it requires at least a scheme, hostname, and a path to be in the map. So it would be a breaking change to r3_hex_api anyway it seems.

[starbelly]

@michaelklishin I think the best option is to patch r3_hex_api:encode_query_string/1 and then send a patch to hex_core. Path of least resistance and all... and this was part of the motivation behind vendoring it in to begin with.

[michaelklishin]

#2195 is ready for QA.

[starbelly]

@michaelklishin Second thought... we are going to have to re-vendor hex_core anyway, so let's get a patch to hex_core done and then I'll do a PR to re-sync hex_core in rebar3. Are you going to open a PR to hex_core? If not, I can certainly do so based on your code.