Skip to content

time_stubs.c: add fallback for Windows 7#13905

Merged
nojb merged 2 commits intoocaml:mainfrom
nojb:windows7
Mar 24, 2026
Merged

time_stubs.c: add fallback for Windows 7#13905
nojb merged 2 commits intoocaml:mainfrom
nojb:windows7

Conversation

@nojb
Copy link
Copy Markdown
Collaborator

@nojb nojb commented Mar 24, 2026

Adds a fallback for GetSystemTimePreciseAsFileTime for old versions of Windows (< 8).

Fixes #13871

@nojb nojb mentioned this pull request Mar 24, 2026
Closed
nojb added 2 commits March 24, 2026 13:43
@nojb
time_stubs.c: add fallback for Windows 7
2c4b7e7 
Signed-off-by: Nicolas Ojeda Bar <n.oje.bar@gmail.com>
@nojb
Changes
a4c3ffc 
Signed-off-by: Nicolas Ojeda Bar <n.oje.bar@gmail.com>
@Alizter Alizter self-requested a review March 24, 2026 12:45
Alizter
Alizter reviewed Mar 24, 2026
View reviewed changes
otherlibs/stdune/src/time_stubs.c
}
if (GetSystemTime == NULL) { /* < Windows 8 */
GetSystemTime = GetSystemTimeAsFileTime;
}
Copy link
Copy Markdown
Collaborator

@Alizter Alizter Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I added an else and a printf here, however I am seeing that this is getting called quite a few times. Shouldn't this only be happening once? (On Win 11)

Copy link
Copy Markdown
Collaborator Author

@nojb nojb Mar 24, 2026

Choose a reason for hiding this comment

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

Yes, it should be called only once per process. Could there be more than one Dune process active?

Copy link
Copy Markdown
Collaborator

@Alizter Alizter Mar 24, 2026

Choose a reason for hiding this comment

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

I think I've solved the mystery. We have rules that generate the help pages so there were multiple dune processes happening when I did dune build dune.install. :-)

@nojb
Copy link
Copy Markdown
Collaborator Author

nojb commented Mar 24, 2026

CI failure is unrelated.

@nojb nojb merged commit 70c38ef into ocaml:main Mar 24, 2026
28 of 29 checks passed
@nojb nojb deleted the windows7 branch March 24, 2026 16:38
@Alizter Alizter mentioned this pull request Mar 24, 2026
Open
25 tasks
@Alizter
Copy link
Copy Markdown
Collaborator

Alizter commented Mar 24, 2026

We get this new warning on mingw:

time_stubs.c: In function 'dune_clock_gettime_realtime':
time_stubs.c:19:21: warning: assignment to 'void (*)(struct _FILETIME *)' from incompatible pointer type 'FARPROC' {aka 'long long int (*)()'} [-Wincompatible-pointer-types]
   19 |       GetSystemTime = GetProcAddress(h, "GetSystemTimePreciseAsFileTime");
      |                     ^

@Alizter Alizter mentioned this pull request Mar 25, 2026
Merged
@Alizter
Copy link
Copy Markdown
Collaborator

Alizter commented Mar 25, 2026

I've made #13914 to fix the warning.

Alizter added a commit that referenced this pull request Mar 25, 2026
@Alizter
fix(time): address warning on Windows in time_stubs.c (#13914)
d2120ac 
`GetProcAddress` returns `FARPROC` and therefore needs an explicit cast.
This fixes the warning introduced in
#13905.
@Alizter Alizter mentioned this pull request Mar 25, 2026
Closed
@rgrinberg
Copy link
Copy Markdown
Member

rgrinberg commented Mar 26, 2026

Does OCaml 4.14 even support windows 7? If not, then this change is kind of pointless as it will be broken by the next release anyway.

@Alizter
Copy link
Copy Markdown
Collaborator

Alizter commented Mar 26, 2026

@rgrinberg I was able to test the patch in a Windows 7 VM so that's a yes.

shonfeder pushed a commit that referenced this pull request Mar 27, 2026
@nojb @shonfeder
time_stubs.c: add fallback for Windows 7 (#13905)
e5b4dbd 
Adds a fallback for `GetSystemTimePreciseAsFileTime` for old versions of
Windows (< 8).

Fixes #13871

Signed-off-by: Nicolas Ojeda Bar <n.oje.bar@gmail.com>
@shonfeder shonfeder mentioned this pull request Mar 27, 2026
Merged
shonfeder pushed a commit that referenced this pull request Mar 27, 2026
@nojb @shonfeder
time_stubs.c: add fallback for Windows 7 (#13905)
16fb41a 
Adds a fallback for `GetSystemTimePreciseAsFileTime` for old versions of
Windows (< 8).

Fixes #13871

Signed-off-by: Nicolas Ojeda Bar <n.oje.bar@gmail.com>
Signed-off-by: Shon Feder <shon.feder@gmail.com>
shonfeder added a commit that referenced this pull request Mar 27, 2026
@shonfeder
[3.22.1] backport #13905 (#13941)
3121800 
Backport #13905 to the release branch for 3.22.1
shonfeder pushed a commit that referenced this pull request Mar 27, 2026
@Alizter @shonfeder
fix(time): address warning on Windows in time_stubs.c (#13914)
bb5d106 
`GetProcAddress` returns `FARPROC` and therefore needs an explicit cast.
This fixes the warning introduced in
#13905.

(cherry picked from commit d2120ac)
Signed-off-by: Shon Feder <shon.feder@gmail.com>
shonfeder pushed a commit that referenced this pull request Mar 27, 2026
@Alizter @shonfeder
fix(time): address warning on Windows in time_stubs.c (#13914)
1bd0f1f 
`GetProcAddress` returns `FARPROC` and therefore needs an explicit cast.
This fixes the warning introduced in
#13905.

(cherry picked from commit d2120ac)
Signed-off-by: Shon Feder <shon.feder@gmail.com>
shonfeder added a commit to shonfeder/opam-repository that referenced this pull request Apr 1, 2026
@shonfeder
[new release] dune (18 packages) (3.22.1)
2b4c1da 
CHANGES:

### Fixed

- Restore compatibility with Windows 7 (ocaml/dune#13905, @nojb)

- `dune test` now runs tests in the default context only. When there is a
  single context, it is treated as the default. This fixes a crash when the
  workspace has no context named "default".
  (ocaml/dune#13930, fixes ocaml/dune#13904, @Alizter)

- Fix `dune trace cat --chrome-trace` to adhere to the Chrome Trace Event
  Format by providing timestamps and durations at microsecond granularity
  (ocaml/dune#13911, fixes ocaml/dune#13906, @Alizter)
@shonfeder shonfeder mentioned this pull request Apr 1, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Alizter Alizter Alizter approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

dune 3.22.0 bootstrap fails on Windows < 8 — GetSystemTimePreciseAsFileTime not found in KERNEL32.dll

3 participants

@nojb @Alizter @rgrinberg