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

nanonng 0.21.9 build issue with MinGW-w64 MSVCRT #922

Open
brechtsanders opened this issue Apr 18, 2024 · 6 comments
Open

nanonng 0.21.9 build issue with MinGW-w64 MSVCRT #922

brechtsanders opened this issue Apr 18, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@brechtsanders
Copy link

When building nanonng version 0.21.9 for Windows with an MSVCRT MinGW-w64 toolchain the build fails in src/platform/windows/win_clock.c because timespec_get() is not defined (it does exist in UCRT MinGW-w64).

The solution is to modify nni_time_get() in src/platform/windows/win_clock.c to only use the timespec_get() implementation when building against UCRT (using #ifdef _UCRT) and otherwise fall back to another implementation, for example the same fallback based in gettimeofday() used in src/platform/posix/posix_clock.c.

On a side note, the line int rv; in nni_time_get() in src/platform/windows/win_clock.c is unused.

I was able to build when after modifying nni_time_get() in src/platform/windows/win_clock.c as follows:

int
nni_time_get(uint64_t *seconds, uint32_t *nanoseconds)
{
#ifdef _UCRT
	struct timespec ts;
	if (timespec_get(&ts, TIME_UTC) == TIME_UTC) {
		*seconds     = ts.tv_sec;
		*nanoseconds = ts.tv_nsec;
		return (0);
	}
	return (nni_win_error(GetLastError()));
#else
	int rv;
	struct timeval tv;
	if ((rv = gettimeofday(&tv, NULL)) == 0) {
		*seconds     = tv.tv_sec;
		*nanoseconds = tv.tv_usec * 1000;
		return (0);
	}
	return (nni_plat_errno(errno));
#endif
}

Note that I haven't tested it yet, and maybe there is a better way (with a more granular clock) to do this.

@JaylinYu
Copy link
Member

thats why i labled it as a pre-release, you know. Will discuss with Garrett on it.

@JaylinYu
Copy link
Member

JaylinYu commented May 5, 2024

related to nanomsg/nng#1826
you can try port the code if you have your own fork... seems like garrett not being happy with MinGW

@brechtsanders
Copy link
Author

Sorry to hear that.

I'm offering the solution on a plate here . If it doesn't get incorporated I guess will just keep patching my builds.

Just trying to help - as an open source developer.

@JaylinYu
Copy link
Member

JaylinYu commented May 5, 2024

Sorry to hear that.

I'm offering the solution on a plate here . If it doesn't get incorporated I guess will just keep patching my builds.

Just trying to help - as an open source developer.

But we definitely could figure it out in NanoNNG fork.

@JaylinYu JaylinYu added the enhancement New feature or request label Jul 3, 2024
@JaylinYu
Copy link
Member

JaylinYu commented Jul 3, 2024

Tried with your way works.
Tried without your way, also works.

I am using Win10 with MinGW

@brechtsanders
Copy link
Author

@JaylinYu The issue is specifically with MSVCRT toolchain, if you're using UCRT the issue will not happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants