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

Fix ip binary not being found on some flavors of Linux #178

Closed
wants to merge 2 commits into from
Closed

Fix ip binary not being found on some flavors of Linux #178

wants to merge 2 commits into from

Conversation

GitSquared
Copy link

Pull Request

This is a fix for an issue reported downstream at GitSquared/edex-ui#227

Changes proposed:

  • Fix
  • Enhancement
  • Remove
  • Update

Description (what is this PR about)

Following this issue that I reported back in March, support was added for using ip instead of ifconfig to detect network interfaces on Linux (9d13697, shipped in v3.37.5).

However, on some distros, ip isn't located at /sbin, and because of this the library tries to use ifconfig instead, which fails because most of these bleeding-edge systems don't have it available.

This PR replaces the /sbin/ip file-exists check by a command -v one that will succeed if ip is available in $PATH.

@sebhildebrandt
Copy link
Owner

sebhildebrandt commented Nov 23, 2018

@GitSquared : Thank you for your pull request. I made a little investigation and now tried to modified it to use the which command ... should also work. Generally the function getMacAddresses() was introduced to a problem in node 7.10 to 8.1 (uv_interface_address - nodejs/node#13581). So as this should no longer be a problem, I have to think, how long I will keep this workaround in place ;-)

So I just released a fix for this v3.50.2 ... can you test it on your side?

@GitSquared
Copy link
Author

@sebhildebrandt Nice one using which. This works on my side, I'm gonna pull the update downstream.

And uh, sorry for the spike in downloads.

@GitSquared GitSquared closed this Nov 23, 2018
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