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

Allow bootstrap with no external IP address #5945

Closed
wants to merge 1 commit into from

Conversation

ihnorton
Copy link
Member

62d0b3d4 introduced a dependency on the availability of an external IP address, via getaddrinfo()
this breaks building when only a loopback address is available, such as on a VM or subway.

This PR fixes the following error during bootstrap, which I saw on the way home this evening and on a VirtualBox-hosted instance over the weekend (but was red-herringed by a Wine error at the time):

multi.jl
ERROR: ErrorException("no active external interfaces")
while loading multi.jl, in expression starting on line 188
while loading /cmn/jldev/base/sysimg.jl, in expression starting on line 138

cc: @loladiro @amitmurthy

@amitmurthy
Copy link
Contributor

lgtm

if externalonly
error("no active external interfaces")
else
return parseip("127.0.0.1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be ip"127.0.0.1"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Branch is updated. That was the first thing I tried, and got this:

multi.jl
ERROR: ip not defined
while loading multi.jl, in expression starting on line 188
while loading /cmn/jldev/base/sysimg.jl, in expression starting on line 138

... but just tried again and it worked. Go figure.

@ihnorton
Copy link
Member Author

@loladiro ok to merge? not my bailiwick, want to make sure you don't mind adding the keyword argument to getipaddr.

@Keno
Copy link
Member

Keno commented Feb 25, 2014

I'm not opposed, but it is somewhat ugly and only used in one place. Maybe a try/catch block there would be better?

error("no active external interfaces")
else
return ip"127.0.0.1"
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just returning ip"127.0.0.1" if no external interfaces are found? Why does getipaddr() only have to detect external interfaces?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about that - presumably we need a distinction between private-only and public ip addresses?

@ihnorton
Copy link
Member Author

@loladiro it feels just as ugly to throw an error when a call could/should return the valid available address...

@Keno
Copy link
Member

Keno commented Feb 25, 2014

I'm much happier with just returning ip"127.0.0.1" rather than introducing an extra keyword.

@Keno
Copy link
Member

Keno commented Feb 25, 2014

On the other hand maybe when this happens it's a bug and the user would very much like to know. Maybe keep the current behavior and rename getipaddr getexternalip or sth like that.

@amitmurthy
Copy link
Contributor

My preference would be for getipaddr to return the loopback address, on a machine disconnected to any external network, provided it is available. It should throw an error only if all networking is down.

It is perfectly reasonable for distributed julia to work properly even on a machine disconnected to a network by communicating via the loopback address.

@JeffBezanson
Copy link
Member

@amitmurthy that sounds good.

Allows building with no external IP address:

62d0b3d4 introduced a dependency on getipaddr(), which previously raised an error if no
external address was found. This broke bootstrap on a loopback-only network.
@ihnorton ihnorton closed this in ec9be75 Feb 26, 2014
@ihnorton
Copy link
Member Author

Ok, done. As far as I can tell, uv_interface_addresses will return an error if there are no addresses available, but this is so pathological I'm not quite sure how to test it.

edit: well there's this, this, but... no.

@ihnorton ihnorton deleted the noip_build branch February 26, 2014 03:57
ihnorton added a commit that referenced this pull request Feb 27, 2014
Allows building with no external IP address:

62d0b3d4 introduced a dependency on getipaddr(), which previously raised an error if no
external address was found. This broke bootstrap on a loopback-only network.

closes #5945
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.

4 participants