janestreet / time_now

Reports the current time
MIT License
7 stars 7 forks source link

Use timespec_get if compiling with cl on Windows #2

Closed nanis closed 3 years ago

nanis commented 3 years ago

Signed-off-by: A. Sinan Unur sinan@unur.com

I built ocaml and opam using the MSVC + Cygwin tools instructions. When I ran opam install core, most dependent libraries installed without any problems, but time_now failed to install:

$ opam install time_now
[NOTE] External dependency handling not supported for OS family 'windows'.
       You can disable this check using 'opam option --global depext=false'
The following actions will be performed:
  * install time_now v0.14.0

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
▼ retrieved time_now.v0.14.0  (cached)
[ERROR] The compilation of time_now.v0.14.0 failed at "dune build -p time_now -j 1".

#=== ERROR while compiling time_now.v0.14.0 ===================================#
# context     2.1.0~rc2 | win32/x86_64 | ocaml.4.12.0 | https://opam.ocaml.org#6609b442
# path        ~\.opam\default\.opam-switch\build\time_now.v0.14.0
# command     ~\.opam\default\bin\dune.exe build -p time_now -j 1
# exit-code   1
# env-file    ~\.opam\log\time_now-8632-6ce4ee.env
# output-file ~\.opam\log\time_now-8632-6ce4ee.out
### output ###
#           cl src/time_now_stubs.obj (exit 2)
# (cd _build/default/src && "C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\
VC\Tools\MSVC\14.29.30037\bin\HostX64\x64\cl.exe" -nologo -O2 -Gy- -MD -D_CRT_SECURE_NO_DEP
RECATE -nologo -O2 -Gy- -MD -I c:/opt/ocaml/lib/ocaml -I C:\opt\cygwin64\home\user\.opam\d
efault\lib\base -I C:\opt\cygwin64\home\user\.opam\default\lib\base\base_internalhash_type
s -I C:\opt\cygwin64\home\user\.[...]
# time_now_stubs.c
# time_now_stubs.c(25): fatal error C1083: Cannot open include file: 'sys/time.h': No such
file or directory

<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
┌─ The following actions failed
│ λ build time_now v0.14.0
└─
-─ No changes have been performed

The apparent reason is that Visual Studio does not provide a sys/time.h. However, since VS 2015, timespec_get has been available. Therefore, I added a preprocessor check for whether cl is being used to compile the C source and implemented time_now_nanoseconds_since_unix_epoch_or_zero using that function.

The check uses _MSC_VER so that the code continues to compile as usual with gcc using MinGW and Cygwin.

bcc32 commented 3 years ago

Thanks @nanis. Are you able to get time_now compiling on your machine with this patch? I don't have a WIndows machine to test this on, but the patch certainly looks reasonable.

nanis commented 3 years ago

@bcc32 Thank you. Yes, I am able to build it with the patch.

$ pwd
/cygdrive/c/Users/user/src/time_now

$ git branch
  master
* msvc-timenow

$ eval $(opam env|dos2unix)

$ ~/.opam/default/bin/dune.exe build -p time_now -j 1
          cl src/time_now_stubs.obj
time_now_stubs.c
nanis commented 3 years ago

@bcc32 I should emphasize that I did not set up separate Cygwin and MinGW environments to check those. However, I am assuming if the source continues to compile with gcc, those environments should be no worse off with this patch.

bcc32 commented 3 years ago

Got it, that makes sense. I've imported this into our internal repo for review. Thanks for contributing!

nanis commented 3 years ago

Thank you for your work and making these tools available.