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

Handle MacOS 10.11.5+ interface changes and VM use case #33

Merged
merged 2 commits into from
Jul 11, 2016

Conversation

eriktrom
Copy link
Member

@eriktrom eriktrom commented Jul 9, 2016

  • allow portfinder.getPort for all ports w/ os.networkInterfaces().internal === true
  • allow portfinder.getPort from netmask 255.255.255.0, which maps to
    ip address given to default interface of machine (en0)
  • allow portfinder.getPort to work from within a locally running VM assuming
    vnic0 or vnic1 use netmask 255.255.255.0 (aka, shared or bridged mode)
  • handle 0.0.0.0 host that while not output by os.networkInterfaces() is
    nonetheless a bindable host that exists and must be handled accordingly

cc @indexzero or @stefanpenner - need at least one pair of eyeballs on this one. Thanks!

- allow portfinder.getPort for all ports w/ os.networkInterfaces().internal === true
- allow portfinder.getPort from netmask 255.255.255.0, which maps to default
  ip address of machine (en0)
- allow portfinder.getPort to work from within a locally running VM assuming
  vnic0 or vnic1 use netmask 255.255.255.0 (aka, shared or bridged mode)
- handle 0.0.0.0 host that while not output by os.networkInterfaces() is
  nonetheless a bindable host that exists and must be handled accordingly
@eriktrom
Copy link
Member Author

eriktrom commented Jul 9, 2016

cc @dbuhrman - does this fix your use case? (if you have time to check thanks, if not, no biggie)

@stefanpenner
Copy link

@eriktrom it seems good, but without tests its hard to see exactly what use-case this is addressing so Im not sure my feedback is that valuable as I haven't played with 10.11.x yet myself.

@eriktrom
Copy link
Member Author

@stefanpenner - thanks for the review - the tests did get an update - they iterate over the exports._defaultHosts now, so they reflect the current changes.

Gonna merge - if we find issues, I'll make another PR

Thanks again for the review

@eriktrom eriktrom merged commit c0edd2a into master Jul 11, 2016
@eriktrom
Copy link
Member Author

@indexzero - can you make a release to npm? Gratzi!

@Turbo87
Copy link

Turbo87 commented Jul 13, 2016

@indexzero it would also be awesome if you could add @eriktrom as a collaborator on NPM to reduce the bus factor of this project

@indexzero
Copy link
Collaborator

indexzero commented Jul 14, 2016

@eriktrom @Turbo87 done. Feel free to publish at will. Good job here 👍

$ npm owner ls portfinder
npm info it worked if it ends with ok
npm info using npm@3.10.5
npm info using node@v4.4.3
npm info attempt registry request try #1 at 1:17:33 PM
npm http request GET https://registry.npmjs.org/portfinder
npm http 200 https://registry.npmjs.org/portfinder
eriktrom <erik.trom.github@gmail.com>
indexzero <charlie.robbins@gmail.com>
npm info ok 

@eriktrom
Copy link
Member Author

Thanks @indexzero 👍

@indexzero indexzero deleted the smart-interface-detection branch July 15, 2016 00:43
homu added a commit to ember-cli/ember-cli that referenced this pull request Jul 15, 2016
Bump portfinder to v1.0.4

Adds support for/fixes issues outlined in http-party/node-portfinder#33
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.

None yet

4 participants