Increase OTP 24 readiness

paulo-ferraz-oliveira commented 3 years ago

What this pulls in:

https://github.com/uwiger/parse_trans/pull/41
https://github.com/uwiger/parse_trans/pull/42
https://github.com/uwiger/parse_trans/pull/43
[ ] don't merge until you decide to up min. to OTP 21

paulo-ferraz-oliveira commented 3 years ago

Yeah, so this seems to be OTP-21 min. Probably it could stay open until you decide to bump min. on your end too.

benoitc commented 3 years ago

does it replace 9d446da9d6e09b883691ef07c45cc09d5ceffda3 ? I was about to cut a release with it

benoitc commented 3 years ago

Yeah, so this seems to be OTP-21 min. Probably it could stay open until you decide to bump min. on your end too.

ok let's cut 0.17.1 and from it start to support a greater erlang version. That will land with the new pooling and connection handling I am testing

paulo-ferraz-oliveira commented 3 years ago

@benoitc: it doesn't replace it. It actually forces you to bump to OTP 21 (I didn't remember this), so not sure it's a good idea to merge in yet. I can leave this here hanging, and you release what you have to, cool?

benoitc commented 3 years ago

i released 0.17.1 . I will take care of erlang bumping in next release. I am not sure to not simply jump to OTP 23 as the minimum version and keep maintenance on this 1.x branch for others versions.

On Mon, Mar 15, 2021 at 11:57 AM Paulo F. Oliveira @.***> wrote:

@benoitc https://github.com/benoitc: it doesn't replace it. It actually forces you to bump to OTP 21 (I didn't remember this), so not sure it's a good idea to merge in yet. I can leave this here hanging, and you release what you have to, cool?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/benoitc/hackney/pull/678#issuecomment-799323726, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADRITIW6DAJJDYNM2H3P3TDXRYZANCNFSM4ZGJXJLA .

paulo-ferraz-oliveira commented 3 years ago

@benoitc, something seems off. If I do rebar3 do unlock,upgrade and then rebar3 tree it seems we're pulling parse_trans 3.4.0, though rebar.config states ~>3.3. Can you check this on your end?

paulo-ferraz-oliveira commented 3 years ago

I guess that syntax means gte?

paulo-ferraz-oliveira commented 3 years ago

I see, in 0.17.1 you didn't run rebar3 do unlock,upgrade prior to release, which is why your rebar.config is now not sync'ed with your rebar.lock. Basically, it's the same issue I reported+fixed in https://github.com/benoitc/hackney/pull/676.

paulo-ferraz-oliveira commented 3 years ago

Build is failing on master. Let me know when it's fixed, so I can rebase here. I was mislead into thinking my changes could have introduced issues.

benoitc commented 3 years ago

This is a GitHub CI issue... not really reliable. I will relaunch the tests

On Mon 15 Mar 2021 at 12:47, Paulo F. Oliveira @.***> wrote:

Build is failing on master. Let me know when it's fixed, so I can rebase here. I was mislead into thinking my changes could have introduced issues.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/benoitc/hackney/pull/678#issuecomment-799355636, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADRIQZCKGF2ITIKNJH2LTTDXXWRANCNFSM4ZGJXJLA .

-- Sent from my Mobile

benoitc commented 3 years ago

@paulo-ferraz-oliveira done

paulo-ferraz-oliveira commented 3 years ago

Cool, thanks.

paulo-ferraz-oliveira commented 3 years ago

(also, as per parse_trans's recent changes I think it would be best to either use a fixed version there os maybe change to "~>3.3.0", since 3.4.0 will force you to migrate to OTP 21)

benoitc commented 3 years ago

(also, as per parse_trans's recent changes I think it would be best to either use a fixed version there os maybe change to "~>3.3.0", since 3.4.0 will force you to migrate to OTP 21)

good idea. I am cutting a minor version with it. Btw no worries for the next major version, It will only support OTP 23 & sup:. WIP will be found this https://github.com/benoitc/hackney/pull/679 for a release this month

benoitc commented 3 years ago

i woudl suggest to use our own parse_trans like in certifi instead of using a dependency. Thoughts?

paulo-ferraz-oliveira commented 3 years ago

It will only support OTP 23 & sup:

Will this be explicit in rebar.config (e.g. via minimum_otp_vsn)? I hope not, because I'd hate to have to stop being able to update it (hackney) because of being forced to bump. What made you bump to such a recent version (current, that is, since 24 official is not out yet)?

i woudl suggest to use our own parse_trans like in certifi instead of using a dependency. Thoughts?

For what purpose? I don't understand this. If hackney doesn't use it, it shouldn't depend on it, right? If it does, sure, it should be locked.

benoitc commented 3 years ago

@paulo-ferraz-oliveira thanks for the feedback. I should have said that it will be optimised for OTP 23 and sup (I will fix that). Support for older version will still be kept in different ways:

1.x branch will be supported during the next 2 years and will keep support for anything >= 19.3 to 24 as of today. Only fixes will go to it and some changes related to them
2.x branch will target modern Erlang and will be optimised for it. I think we can relax the versions and start at 21.3 or better 22. I will add it to the test suite, but let say it makes things complicated to handle with different breaking changes in OTP versions. There are different reason to uses a modern version of OTP for hackney:
- for the pooling , I would like to use the new counter API when it's possible, I am also looking at the usage of persistent_terms
- gen_statem: will improve stream handling and connection monitoring
- new ssl API
- soon , new socket API

As for parse_trans, it's not a dependency anymore for certifi, and I will probably do the changes in Hackney too, sometimes this week to ease the support.

If you want we can continue the discussion on irc or slack. I would love to understand what is your limitations in term of Erlang version and current usage patterns you have :)

paulo-ferraz-oliveira commented 3 years ago

say it makes things complicated to handle with different breaking changes

I know it does. We all suffer from that 😄 But 21 min. should be OK, since it's mostly compatible with 24.

counters were added in 21.2
gen_statem is 19
ssl I wouldn't know
socket will stop being experimental only in 24, which means you'll have to support for prior versions too (I know the interface and behaviour broke, I've opened issues on this, before)

paulo-ferraz-oliveira commented 3 years ago

You seem to have deleted rebar.lock from master. Is this on purpose? If so, you should probably add it to .gitignore, though I fail to understand why one wouldn't push it. I was rebasing this pull request and saw that, so I won't push rebar.lock either.

paulo-ferraz-oliveira commented 3 years ago

I'm closing this pull request, because this seems unwanted/unnecessary, at the moment.

benoitc commented 3 years ago

You meant the rebar.lock ? No , I have to check a script there. About parse_trans I followed your advise to set the version for now but I was waiting to bump the minimum Erlang version to maybe apply this PR since upgrading parse_trans required it. Anyway thanks for it.

paulo-ferraz-oliveira commented 3 years ago

Yeah, sorry. I meant rebar.lock. It's not there anymore. (it works, but it's unusual - personally I prefer to lock to a Very Specific Version; I've seen stuff break between versions like 1.1.2 and 1.1.3, and can't actually go around trying to understand how everybody works their versioning 😄)

This PR, according to the recent changes in master, is void of useful content (it basically just updated some CI versions). 😄

benoitc / hackney

Increase OTP 24 readiness #678