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

Use DUK_USE_DATE_NOW_WINDOWS flag for Windows 7 support #3

Closed
efoerster opened this issue Oct 11, 2019 · 5 comments
Closed

Use DUK_USE_DATE_NOW_WINDOWS flag for Windows 7 support #3

efoerster opened this issue Oct 11, 2019 · 5 comments

Comments

@efoerster
Copy link

The library currently only works on Windows 7 if it was compiled on a Windows 7 (or older) machine. See this code from duk_config_default.h:

/* GetSystemTimePreciseAsFileTime() available from Windows 8:
 * https://msdn.microsoft.com/en-us/library/windows/desktop/hh706895(v=vs.85).aspx
 */
#if defined(DUK_USE_DATE_NOW_WINDOWS_SUBMS) || defined(DUK_USE_DATE_NOW_WINDOWS)
/* User forced provider. */
#else
#if defined(_WIN32_WINNT) && (_WIN32_WINNT >= 0x0602)
#define DUK_USE_DATE_NOW_WINDOWS_SUBMS
#else
#define DUK_USE_DATE_NOW_WINDOWS
#endif
#endif

We use this library and ran into this issue, see latex-lsp/texlab#101. Therefore, it would be nice to have a feature flag for ducc to enable Windows 7 support.

@SkylerLipthay
Copy link
Owner

SkylerLipthay commented Oct 11, 2019

Hm, I see... So DUK_USE_DATE_NOW_WINDOWS_SUBMS is being defined during compilation on Windows 8/10 but then GetSystemTimePreciseAsFileTime fails to resolve at runtime under Windows 7. That's an unfortunate consequence. If I were on the Duktape team I would have done it the other way around. (So that the user would specify a flag to activate this poorly supported sub-millisecond accuracy.)

I'm tempted to just #define DUK_USE_DATE_NOW_WINDOWS. If I'm understanding things correctly, this would sacrifice sub-millisecond timer accuracy on Windows 8/10, but at least it would fix this issue. I highly doubt this would affect any current ducc users... Besides, supporting such granular feature flags seems against the nature of ducc, which intentionally hides such implementation details.

Is this OK with you?

(Alternatively it looks like you can supply your own time functions that could perhaps dynamically fallback to GetSystemTime if GetSystemTimePreciseAsFileTime is missing. I'm guessing this is overkill for your purposes.)

@efoerster
Copy link
Author

Sure, thats even better. Thank you!

@SkylerLipthay
Copy link
Owner

Alright give ducc 0.1.1 a shot. Close this issue if that fixes the user's issue or let me know if further work is needed :)

@SkylerLipthay
Copy link
Owner

Apologies @efoerster. It just dawned on me that 0.1.1 probably failed to fix the problem. I misunderstood the logic required of duk_config.h. I have limited testing capabilities with Windows... but it should be fixed now (302d7a4) with 0.1.2.

@efoerster
Copy link
Author

Seems to be fixed, thanks for your fast response.

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

No branches or pull requests

2 participants