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

Port patches and fixes over from node #124

Closed
wants to merge 0 commits into from

Conversation

piscisaureus
Copy link
Contributor

No description provided.

@piscisaureus piscisaureus changed the title Merge WIP: Land the libuv upgrade and async-listener removal Dec 9, 2014
@rvagg
Copy link
Member

rvagg commented Dec 9, 2014

I'm not a fan of the flaky test stuff in f6433a9 and da0ef90 and would rather see those be tackled properly

@piscisaureus
Copy link
Contributor Author

@rvagg The list with flaky tests in particular?
Or do you also prefer f6433a9 not to land?

@rvagg
Copy link
Member

rvagg commented Dec 9, 2014

just the fact that we need the list in the first place; you are probably more familiar with these tests than me and why they are being marked as flaky, do you think it's justified and the best way forward with this?

@bnoordhuis
Copy link
Member

Marking test-net-GH-5504 as flaky is reasonable, it hits a bug with some (old) Linux kernels. The others seem rather suspect, though.

That said, it's a test runner switch and we don't have to use it.

@piscisaureus
Copy link
Contributor Author

I'll land both patches, but I'll make sure only GH-5504 ends up on the flaky test list.

@piscisaureus piscisaureus force-pushed the merge branch 2 times, most recently from 7e44f01 to 8b56072 Compare December 9, 2014 15:39
@piscisaureus piscisaureus changed the title WIP: Land the libuv upgrade and async-listener removal Port patches and fixes over from node Dec 9, 2014
@piscisaureus
Copy link
Contributor Author

Merge conflicts made many of these not apply cleanly, I cut up the async listener removal into two parts; one where Async Listener itself is removed, and another where the tracing module is removed. If desired I can squash those two commits again, but I kind of like the way it ended up.

@piscisaureus
Copy link
Contributor Author

@bnoordhuis or @rvagg, would you mind taking another look at the unreviewed commits (the ones with that cute little yellow puppet in front of them).

@bnoordhuis
Copy link
Member

@piscisaureus Reviewed.

piscisaureus added a commit to piscisaureus/node2 that referenced this pull request Dec 9, 2014
On Windows a long integer is always 32-bits, even when the target
architecture uses 64-bit pointers.

PR-URL: nodejs#124
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
piscisaureus added a commit to piscisaureus/node2 that referenced this pull request Dec 9, 2014
uv_thread_t is a HANDLE (void pointer) on Windows, which means that
on 64-bit windows it cannot be stored with CRYPTO_THREADID_set_numeric
without potential data loss.

PR-URL: nodejs#124
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
piscisaureus added a commit to piscisaureus/node2 that referenced this pull request Dec 9, 2014
PR-URL: nodejs#124
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
piscisaureus added a commit to piscisaureus/node2 that referenced this pull request Dec 9, 2014
That test can trigger a bug on some older Linux kernels.

PR-URL: nodejs#124
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
piscisaureus added a commit to piscisaureus/node2 that referenced this pull request Dec 9, 2014
PR-URL: nodejs#124
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
piscisaureus added a commit that referenced this pull request Dec 9, 2014
On Windows a long integer is always 32-bits, even when the target
architecture uses 64-bit pointers.

PR-URL: #124
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
piscisaureus added a commit that referenced this pull request Dec 9, 2014
uv_thread_t is a HANDLE (void pointer) on Windows, which means that
on 64-bit windows it cannot be stored with CRYPTO_THREADID_set_numeric
without potential data loss.

PR-URL: #124
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
piscisaureus added a commit that referenced this pull request Dec 9, 2014
PR-URL: #124
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
piscisaureus added a commit that referenced this pull request Dec 9, 2014
That test can trigger a bug on some older Linux kernels.

PR-URL: #124
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
piscisaureus added a commit that referenced this pull request Dec 9, 2014
PR-URL: #124
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@piscisaureus
Copy link
Contributor Author

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.

3 participants