Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extended_bin: Ignore ERL_DIST_PORT on OTP 22 #890

Closed
wants to merge 1 commit into from

Conversation

weiss
Copy link
Contributor

@weiss weiss commented Nov 9, 2021

Don't set the kernel options inet_dist_listen_{min,max} on Erlang/OTP releases prior to 23.0. On those versions, the remote_console VM and nodetool would attempt to listen on the same port as the main VM (because -dist_listen false is not yet supported), and therefore fail to start up.

Don't set the kernel options 'inet_dist_listen_{min,max}' on Erlang/OTP
versions prior to 23.0.  On those versions, the 'remote_console' VM and
nodetool would attempt to listen on the same port and therefore fail to
start up.
@tsloughter
Copy link
Member

Hey, sorry for the delay on review.

I'm not sure this is right. ERL_DIST_PORT is set by the user and so that code is only used if the user is trying to use that feature which only exists in 23+. It feels like the script should instead exit with an error message rather than ignore the user set port.

@weiss
Copy link
Contributor Author

weiss commented Dec 20, 2021

It feels like the script should instead exit with an error message rather than ignore the user set port.

Yes, makes sense. So just replace the current fallback solution with such an error exit? (Shall I (force-)push a new patch?)

@tsloughter
Copy link
Member

Actually, maybe a warning instead of an exit? Not everyone needs the local remote shell, so exit'ing may be too disruptive,but letting them know that remote shell, rpc, eval aren't going to work would be important.

@weiss
Copy link
Contributor Author

weiss commented Dec 20, 2021

Actually, maybe a warning instead of an exit?

Sounds good! I'm easy to convince 🤣

@tsloughter
Copy link
Member

@weiss I was hoping to make a release today so a new rebar3 can be released with optional_application support. Should I go ahead or do you think you'll have this patched to do a warning soon?

@weiss
Copy link
Contributor Author

weiss commented Dec 31, 2021

Sorry, wasn't clear to me whether you're waiting for an update. I can certainly update the patch this weekend but not sure if today.

@weiss
Copy link
Contributor Author

weiss commented Dec 31, 2021

Not everyone needs the local remote shell, so exit'ing may be too disruptive,but letting them know that remote shell, rpc, eval aren't going to work would be important.

BTW, my own use case was shipping a setup where ERL_DIST_PORT is always specified and will or will not have an effect depending on OTP version, i.e. either epmd is started or not, but remsh/rpc will work either way. So for me, a warning that says ERL_DIST_PORT is ignored because OTP is too old would work better than a warning that says ERL_DIST_PORT is honored, but OTP is too old, so remsh/rpc aren't going to work. But I can totally see the latter being preferable for others.

@tsloughter
Copy link
Member

@weiss ah, I see. Yea, sadly I think since it has been being honored we shouldn't change that behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants