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

Missing include on windows for using _wgetenv #102

Open
h-vetinari opened this issue Aug 14, 2024 · 2 comments
Open

Missing include on windows for using _wgetenv #102

h-vetinari opened this issue Aug 14, 2024 · 2 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@h-vetinari
Copy link

nvtx uses _wgetenv on windows

#if defined(_WIN32)
#define NVTX_PATHCHAR wchar_t
#define NVTX_STR(x) L##x
#define NVTX_GETENV _wgetenv

which requires either <stdlib.h> or <wchar.h>. From looking around the code under https://github.com/NVIDIA/NVTX/blob/v3.1.0/c/include/nvtx3/ a bit, all the includes happen in c/include/nvtx3/nvtxDetail/nvtxImpl.h, which is missing those headers for windows

#if defined(_WIN32)
#include <Windows.h>
#else

they do get included further down, so there may be some mix-up with the placement of the respective #else / #endif. For packages that need nvtx in conda-forge on windows (with CUDA 12 currently), this predictably fails:

%LIBRARY_INC%\nvtx3\nvtxDetail\nvtxInit.h(164): error C3861: '_wgetenv': identifier not found

Xref conda-forge/cuda-nvtx-feedstock#23

@jcohen-nvidia
Copy link
Contributor

This issue is fixed in the dev branch by commit 909007c. It will be merged into the release-v3 branch in the next release, which will be in the next few weeks. If you are using the release-v3 branch and need a fix now, but are not comfortable switching the dev branch, you can temporarily take the contents of c\include\nvtx3\nvtxDetail\nvtxImpl.h from the dev branch replace your local copy with that. The only code change (i.e. not whitespace/comments) to that file since the last merge was the fix for this issue, by moving some #includes like stdlib.h and wchar.h out from a Linux-only section to be used on all platforms.

@h-vetinari
Copy link
Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants