Skip to content

darwin: use clock_gettime in macOS 10.12#1073

Closed
saghul wants to merge 1 commit intolibuv:v1.xfrom
saghul:darwin-clock_gettime
Closed

darwin: use clock_gettime in macOS 10.12#1073
saghul wants to merge 1 commit intolibuv:v1.xfrom
saghul:darwin-clock_gettime

Conversation

@saghul
Copy link
Copy Markdown
Member

@saghul saghul commented Sep 28, 2016

Looks like macOS Sierra has some nice additions :-)

Potential caveat: a binary compiled on 10.12 will not run on earlier systems. Since we already have some OSX versions checks in place I thought that's ok.

R= @libuv/collaborators and explicit ping to the in-house OSX expert: @indutny

@indutny
Copy link
Copy Markdown
Member

indutny commented Sep 28, 2016

Looks fine to me, but why is it better?

@imran-iq
Copy link
Copy Markdown
Contributor

imran-iq commented Sep 28, 2016

On a somewhat similar note this should cause failures for a binary compiled on AIX 7.1 and run on lower versions

imran-iq
imran-iq approved these changes Sep 28, 2016
View reviewed changes
Copy link
Copy Markdown
Contributor

@imran-iq imran-iq left a comment

Choose a reason for hiding this comment

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

LGTM FWIW

@saghul
Copy link
Copy Markdown
Member Author

saghul commented Sep 28, 2016

More POSIX, moar better!

On Sep 28, 2016 19:08, "Fedor Indutny" notifications@github.com wrote:

Looks fine to me, but why is it better?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1073 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AATYGHT5-dXMPjhcqVMMXzGoTHG1TeIGks5quq0GgaJpZM4KJE7Q
.

@indutny
Copy link
Copy Markdown
Member

indutny commented Sep 28, 2016

@saghul got it, but really... what's the benefit of this?

indutny
indutny approved these changes Sep 28, 2016
View reviewed changes
Copy link
Copy Markdown
Member

@indutny indutny left a comment

Choose a reason for hiding this comment

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

LGTM, let's land it. On a second thought we may get rid of the old implementation eventually, which will lead to a cleaner code here. 👍

@saghul
Copy link
Copy Markdown
Member Author

saghul commented Sep 29, 2016

That was my main idea. Eventually most BSDs and Darwin would share the
implementation.

We can land this on master if anyone really feels strongly about it.

Also, it would be nice to benchmark it, but since we use uv_hrtime for
benchmarking, how do we measure uv_hrtime itself?

On Sep 28, 2016 23:17, "Fedor Indutny" notifications@github.com wrote:

@indutny approved this pull request.

LGTM, let's land it. On a second thought we may get rid of the old
implementation eventually, which will lead to a cleaner code here. 👍

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1073 (review), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AATYGPPjb6zKqNX5i7oyIO1MiMFiaqxCks5quud9gaJpZM4KJE7Q
.

saghul added a commit that referenced this pull request Dec 6, 2016
@saghul
darwin: use clock_gettime in macOS 10.12
f8d4805 
PR-URL: #1073
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Imran Iqbal <imran@imraniqbal.org>
@saghul
Copy link
Copy Markdown
Member Author

saghul commented Dec 6, 2016

Oops, this one fell through the cracks. Landed in f8d4805.

@saghul saghul closed this Dec 6, 2016
@cjihrig
Copy link
Copy Markdown
Contributor

cjihrig commented Jan 9, 2017

I'm running into problems with the 1.10.2 release that just got cut, and I think it's related to this change. I'm running OS X 10.11.6, which shouldn't be effected by this PR. However, I'm getting the following error, even after make clean:

 dyld: lazy symbol binding failed: Symbol not found: _clock_gettime
   Referenced from: /Users/cjihrig/programming/libuv/.libs/libuv.1.dylib (which was built for Mac OS X 10.12)
   Expected in: /usr/lib/libSystem.B.dylib
 
 dyld: Symbol not found: _clock_gettime
   Referenced from: /Users/cjihrig/programming/libuv/.libs/libuv.1.dylib (which was built for Mac OS X 10.12)
   Expected in: /usr/lib/libSystem.B.dylib

Note that it's saying libuv was built for 10.12.

@cjihrig cjihrig mentioned this pull request Jan 9, 2017
Closed
@cjihrig
Copy link
Copy Markdown
Contributor

cjihrig commented Jan 9, 2017 •
edited
Loading

I think I am encountering this problem after recently updating xcode: http://stackoverflow.com/questions/39224905/how-do-i-install-xcode-8-without-updating-macos

EDIT: After updating to 10.12, everything is fine again.

@cjihrig cjihrig mentioned this pull request Jan 9, 2017
Merged
2 tasks
@saghul
Copy link
Copy Markdown
Member Author

saghul commented Jan 10, 2017

@cjihrig Can you paste the command line that was executed? (make V=1)

@cjihrig
Copy link
Copy Markdown
Contributor

cjihrig commented Jan 10, 2017

Sorry. I have since updated to 10.12 and recompiled.

@chris-araman chris-araman mentioned this pull request Jan 20, 2017
Closed
cjihrig pushed a commit that referenced this pull request Jan 29, 2017
@chris-araman @cjihrig
Revert "darwin: use clock_gettime in macOS 10.12"
f35674f 
This reverts commit f8d4805,
which causes run-time link errors when compiled with macOS 10.12
SDK, but run on 10.11 or earlier.

Refs: #1073
PR-URL: #1202
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
ghostoy pushed a commit to nwjs/node that referenced this pull request Feb 21, 2017
@chris-araman
Revert "darwin: use clock_gettime in macOS 10.12"
43092ca 
This reverts commit f8d4805087944cb8543d98109f5f83f4938eb4b5,
which causes run-time link errors when compiled with macOS 10.12
SDK, but run on 10.11 or earlier.

Refs: libuv/libuv#1073
PR-URL: libuv/libuv#1202
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@cjihrig cjihrig mentioned this pull request Dec 8, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@indutny indutny indutny approved these changes

+1 more reviewer

@imran-iq imran-iq imran-iq approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@saghul @indutny @imran-iq @cjihrig