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 DNS lookup on Android #4580

Closed
wants to merge 1 commit into from
Closed

Conversation

daguej
Copy link
Contributor

@daguej daguej commented Jan 8, 2016

V4MAPPED isn't supported by Android either (as of 6.0).

Fixes #3771.

`V4MAPPED` isn't supported by Android either (as of 6.0)
@mscdex mscdex added dns Issues and PRs related to the dns subsystem. arm Issues and PRs related to the ARM platform. labels Jan 8, 2016
@jasnell
Copy link
Member

jasnell commented Jan 8, 2016

LGTM but a test case would likely be helpful also (we do not currently run CI on Android, of course, but it would be handy to have).

@jasnell
Copy link
Member

jasnell commented Jan 8, 2016

@mscdex

@cjihrig
Copy link
Contributor

cjihrig commented Jan 8, 2016

@daguej what is the value of dns.V4MAPPED on Android by chance?

@daguej
Copy link
Contributor Author

daguej commented Jan 8, 2016

@cjihrig It's 2048.

@cjihrig
Copy link
Contributor

cjihrig commented Jan 8, 2016

Ah, thanks. I was hoping it was undefined or something so we didn't have to explicitly check platform names.

@cjihrig
Copy link
Contributor

cjihrig commented Jan 8, 2016

LGTM.

We might want to consider removing the implicit use of V4MAPPED in a future version. It seems to just cause problems.

@mscdex
Copy link
Contributor

mscdex commented Jan 8, 2016

LGTM

jasnell pushed a commit that referenced this pull request Jan 8, 2016
`V4MAPPED` isn't supported by Android either (as of 6.0)

PR-URL: #4580
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell
Copy link
Member

jasnell commented Jan 8, 2016

Landed in c64018e

@jasnell jasnell closed this Jan 8, 2016
@daguej
Copy link
Contributor Author

daguej commented Jan 9, 2016

Thanks -- close #3771?

@cjihrig
Copy link
Contributor

cjihrig commented Jan 9, 2016

Thanks. Closed it.

MylesBorins pushed a commit that referenced this pull request Jan 11, 2016
`V4MAPPED` isn't supported by Android either (as of 6.0)

PR-URL: #4580
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 11, 2016
MylesBorins pushed a commit that referenced this pull request Jan 12, 2016
`V4MAPPED` isn't supported by Android either (as of 6.0)

PR-URL: #4580
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 28, 2016
`V4MAPPED` isn't supported by Android either (as of 6.0)

PR-URL: #4580
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
`V4MAPPED` isn't supported by Android either (as of 6.0)

PR-URL: #4580
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
`V4MAPPED` isn't supported by Android either (as of 6.0)

PR-URL: nodejs#4580
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 11, 2016
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
`V4MAPPED` isn't supported by Android either (as of 6.0)

PR-URL: nodejs#4580
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@rvagg
Copy link
Member

rvagg commented Mar 22, 2016

I'm seeking some feedback about how Node.js is used on Android if anyone watching this thread would like to tell us about it: nodejs/build#359

scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
`V4MAPPED` isn't supported by Android either (as of 6.0)

PR-URL: nodejs#4580
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arm Issues and PRs related to the ARM platform. dns Issues and PRs related to the dns subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants