Skip to content

fix(time): old macos fallback#13918

Merged
Alizter merged 3 commits intoocaml:mainfrom
Alizter:push-tnxqtuzvykty
Mar 26, 2026
Merged

fix(time): old macos fallback#13918
Alizter merged 3 commits intoocaml:mainfrom
Alizter:push-tnxqtuzvykty

Conversation

@Alizter
Copy link
Collaborator

@Alizter Alizter commented Mar 25, 2026

No description provided.

@Alizter Alizter force-pushed the push-tnxqtuzvykty branch from 7f6cada to a98a887 Compare March 25, 2026 15:02
@barracuda156
Copy link
Contributor

There is another instance here:

clock_gettime(CLOCK_REALTIME, &tp);

@Alizter Alizter force-pushed the push-tnxqtuzvykty branch 2 times, most recently from 5aa5134 to 42b0f5f Compare March 25, 2026 15:51
@Alizter Alizter requested a review from barracuda156 March 25, 2026 16:00
@Alizter Alizter force-pushed the push-tnxqtuzvykty branch from 5441abb to 4b1f4aa Compare March 25, 2026 16:03
@Alizter Alizter requested a review from barracuda156 March 25, 2026 16:03
Copy link
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

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

It's bad enough to add these ifdefs, I don't think they should be copy pasted. You should extract the definition of this function into a header.

@Alizter
Copy link
Collaborator Author

Alizter commented Mar 25, 2026

@rgrinberg Are you suggesting I put the time stubs in a foreign archive or something?

@barracuda156
Copy link
Contributor

@Alizter The patch from 352ccf2 worked.

Copy link
Contributor

@barracuda156 barracuda156 left a comment

Choose a reason for hiding this comment

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

This build works on 10.6 now.

@rgrinberg
Copy link
Member

@rgrinberg Are you suggesting I put the time stubs in a foreign archive or something?

That would work, but it would be overkill. Instead, I'm suggesting that you put the implementation inside of a header file that you can include in both C sources. As long as the implementation is static, it should be fine.

@rgrinberg
Copy link
Member

@barracuda156 by the way, what version of OCaml are you using?

@barracuda156
Copy link
Contributor

@barracuda156 by the way, what version of OCaml are you using?

4.14.3

OCaml 5 is not available in MacPorts (or in my fork). It is also not present in OpenBSD ports etc.

Alizter added 3 commits March 26, 2026 12:07
Signed-off-by: Ali Caglayan <alizter@gmail.com>
Signed-off-by: Ali Caglayan <alizter@gmail.com>
We move the platform specific timing functions to a common header and
share the definition.

Signed-off-by: Ali Caglayan <alizter@gmail.com>
@Alizter Alizter force-pushed the push-tnxqtuzvykty branch from 4b34424 to 06d33fc Compare March 26, 2026 11:08
@Alizter Alizter requested a review from rgrinberg March 26, 2026 11:09
@Alizter
Copy link
Collaborator Author

Alizter commented Mar 26, 2026

@rgrinberg I've introduced a common header with all the platform specific timing code.

@Alizter Alizter merged commit 0e5c3e4 into ocaml:main Mar 26, 2026
29 checks passed
@Alizter Alizter deleted the push-tnxqtuzvykty branch March 26, 2026 18:36
@barracuda156
Copy link
Contributor

Thank you very much!

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