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

Can not build library on Windows using TDM-GCC 5.1, missing _wremove, _wrename #35

Closed
Xeverous opened this Issue Oct 29, 2017 · 19 comments

Comments

Projects
2 participants
@Xeverous
Copy link

Xeverous commented Oct 29, 2017

Ouput:

> make install
Scanning dependencies of target CorradeUtilityObjects
[  3%] Building CXX object src/Corrade/Utility/CMakeFiles/CorradeUtilityObjects.dir/Debug.cpp.obj
[  6%] Building CXX object src/Corrade/Utility/CMakeFiles/CorradeUtilityObjects.dir/Directory.cpp.obj
D:\Files\C++\lib_build\corrade\src\Corrade\Utility\Directory.cpp: In function 'bool Corrade::Utility::Directory::rm(const string&)':
D:\Files\C++\lib_build\corrade\src\Corrade\Utility\Directory.cpp:179:33: error: '_wremove' was not declared in this scope
     return _wremove(wpath.data()) == 0;
                                 ^
D:\Files\C++\lib_build\corrade\src\Corrade\Utility\Directory.cpp: In function 'bool Corrade::Utility::Directory::move(const string&, const string&)':
D:\Files\C++\lib_build\corrade\src\Corrade\Utility\Directory.cpp:198:62: error: '_wrename' was not declared in this scope
         _wrename(widen(oldPath).data(), widen(newPath).data())
                                                              ^

No CMake variables seem to affect this problem (fails on multiple variations of C++ standard, debug/release, linkage type, deprecated API on/off)

@mosra mosra added this to TODO in Platforms Nov 5, 2017

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 6, 2017

Hi,

sorry for the late reply, I missed this completely, GitHub somehow decided to not inform me about new issues on my repos at all anymore :(

A moonshot: I compared the wchar.h header of my current MinGW-w64 installation and the header that's shipped with latest TDM-GCC and these two functions seem to be hidden behind #ifndef __STRICT_ANSI__ on TDM. Not sure why and I'm also completely unsure which MinGW headers is TDM using :/

I, however, have no access to a Windows machine right now, so I can't test myself -- could you try compiling with -U__STRICT_ANSI__ added to CMAKE_CXX_FLAGS?

Thank you!

@mosra mosra moved this from TODO to In Progress in Platforms Nov 6, 2017

@Xeverous

This comment has been minimized.

Copy link
Author

Xeverous commented Nov 6, 2017

Sure, I will but currently I'm working in the office. Will try it in around 6-7 hours.

I think the functions are hidden because ... they are not ANSI C. Going by https://sourceforge.net/p/mingw-w64/bugs/207/ MinGW should also disable them.

GCC Documentation:
__STRICT_ANSI__
GCC defines this macro if and only if the -ansi switch, or a -std switch specifying strict conformance to some version of ISO C or ISO C++, was specified when GCC was invoked. It is defined to ‘1’. This macro exists primarily to direct GNU libc’s header files to use only definitions found in standard C.

So I guess you will have to disable this macro altogether or use -std=gnu++XX or find a way to replace non-ansi-C Windows API calls.

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 6, 2017

Hey, thanks for the fast reply! :)

or find a way to replace non-ansi-C Windows API calls.

Well :/ I was happily using the ANSI C functions until recently, but because of the unbelievable insanity that is Unicode support in Windows APIs, I had to switch to those "wchar" APIs (plus some really dark magic to make MinGW work as well). If only there would be a way to use UTF-8 like on all other platforms :(

I think I could use some MoveFileEx etc. API, but I think I rather undef the __STRICT_ANSI__ for MinGW in just this one file right before including the wchar header. There's a lot of other MinGW-specific ugliness anyway.

@Xeverous

This comment has been minimized.

Copy link
Author

Xeverous commented Nov 6, 2017

I had something like this in my work project and it was fairly enough for linux and win

std::string GetPathToExecutable()
{
#ifdef _WIN32
	char result[MAX_PATH];
	return std::string(result, GetModuleFileName(nullptr, result, MAX_PATH));
#else
	char result[PATH_MAX];
	ssize_t count = readlink("/proc/self/exe", result, PATH_MAX);
	return std::string(result, (count > 0) ? count : 0);
#endif
}

However, we blamed project's lead team for their deny of using boost::filesystem. But, I agree, there is definitely something wrong with Windows API - the only platform that has 16-bit wchar_t and disgusting amount of macros.

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 6, 2017

Wait, that snippet is completely unrelated to file removing/renaming and also not really working with Unicode paths on Windows ;)

@Xeverous

This comment has been minimized.

Copy link
Author

Xeverous commented Nov 6, 2017

I briefly read the commit you made, just wanted to look at some low-level code. And yes, it does not work with unicode indeed. It's one of those fun parts that I experienced in work when trying to write something multi-platform but without recreating a large library. Due to simple implementations, I often ended in problems such as inability of Windows to understand C:/files/tests/../../../D:/reports/test_results/

@Xeverous

This comment has been minimized.

Copy link
Author

Xeverous commented Nov 6, 2017

I have added -U__STRICT_ANSI__ and it works! Builds without any warnings.

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 6, 2017

Great! Can I ask you to try this patch as well?

diff --git a/src/Corrade/Utility/Directory.cpp b/src/Corrade/Utility/Directory.cpp
index cec69bbb..dbe048e1 100644
--- a/src/Corrade/Utility/Directory.cpp
+++ b/src/Corrade/Utility/Directory.cpp
@@ -51,6 +51,11 @@
 /** @todo remove the superfluous includes when mingw is fixed (otherwise causes undefined EXTERN_C error) */
 #ifdef CORRADE_TARGET_WINDOWS
 #ifdef __MINGW32__
+
+/* Otherwise _wrename() and _wremove() is not defined on TDM-GCC 5.1 */
+#undef __STRICT_ANSI__
+#include <wchar.h>
+
 #include <wtypes.h>
 #include <windef.h>
 #include <wincrypt.h>

This would be better than undefing it globally, if it works. I hope TDM-GCC defines __MINGW32__ as well, though.

@Xeverous

This comment has been minimized.

Copy link
Author

Xeverous commented Nov 6, 2017

gcc -x c++ -std=c++17 -dM -E - < /dev/null | sort | grep __MINGW the magic of git bash

defined to 1

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 6, 2017

So does the above patch work, then? :) (with __STRICT_ANSI__ no longer being in CMAKE_CXX_FLAGS, of course)

@Xeverous

This comment has been minimized.

Copy link
Author

Xeverous commented Nov 6, 2017

No, it does not. Same error. This might be the hint:

<wchar.h>

/* All the headers include this file. */
#include <_mingw.h>

<_mingw.h>

/* Activation of MinGW specific extended features:
 */
#ifndef __USE_MINGW_ANSI_STDIO
/*
 * If user didn't specify it explicitly...
 */
# if   defined __STRICT_ANSI__  ||  defined _ISOC99_SOURCE \
   ||  defined _POSIX_SOURCE    ||  defined _POSIX_C_SOURCE \
   ||  defined _XOPEN_SOURCE    ||  defined _XOPEN_SOURCE_EXTENDED \
   ||  defined _GNU_SOURCE      ||  defined _BSD_SOURCE \
   ||  defined _SVID_SOURCE
   /*
    * but where any of these source code qualifiers are specified,
    * then assume ANSI I/O standards are preferred over Microsoft's...
    */
#  define __USE_MINGW_ANSI_STDIO    1

I suspect that GCC indents to use compiler flags rather than macros in code

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 6, 2017

Huh. I have no idea, haha. Trial and error? What if you put the #undef __STRICT_ANSI__ as the first thing in the file?

Sorry for using you as a remote debugger... :/

@Xeverous

This comment has been minimized.

Copy link
Author

Xeverous commented Nov 6, 2017

In which file? Going by the amount of macros that can switch strict ANSI on, I think they intent us to use command line switches. Same here: https://gcc.gnu.org/onlinedocs/gcc-3.3.5/cpp/Common-Predefined-Macros.html where they explain implicit differences between -std=gnu and -std=c++

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 6, 2017

Oh. In the same file as the patch was for, so src/Corrade/Utility/Directory.cpp.

I don't want to apply a command-line switch globally, because that would affect more compilers than just TDDM-GCC and might have some nasty consequences in other places (or I would need to detect it somehow, which I have no idea how to do). That's why I wanted to change the define only in this one particular file because that limits the possible harm quite a lot.

@Xeverous

This comment has been minimized.

Copy link
Author

Xeverous commented Nov 6, 2017

Yes, If I put it first it works. Probbaly interacting with standard library headers. Every header in TDM-GCC implementation includes <_mingw.h> first, which acts as a config file. Any later includes will rely on first invocation due to include guards and so I guess any macros, like USE_MATH_DEFINES must be before any header.

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 6, 2017

Cool! So I'll push that as a fix :)

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 6, 2017

Pushed as 7d94e08, it's now in the next branch, waiting for the CIs to get green, then I merge to master and close this.

Thank you a lot for helping me through this :)

@Xeverous

This comment has been minimized.

Copy link
Author

Xeverous commented Nov 6, 2017

Just for info, there are multiple blocks in <wchar.h> looking like this:

/* These differ from the ISO C prototypes, which have a maxlen parameter like snprintf.  */
#ifndef __STRICT_ANSI__
_CRTIMP int __cdecl __MINGW_NOTHROW	swprintf (wchar_t*, const wchar_t*, ...);
_CRTIMP int __cdecl __MINGW_NOTHROW	vswprintf (wchar_t*, const wchar_t*, __VALIST);
#endif

#ifdef __MSVCRT__
_CRTIMP wchar_t* __cdecl __MINGW_NOTHROW fgetws (wchar_t*, int, FILE*);
_CRTIMP int __cdecl __MINGW_NOTHROW	fputws (const wchar_t*, FILE*);
_CRTIMP wint_t __cdecl __MINGW_NOTHROW	getwc (FILE*);
_CRTIMP wint_t __cdecl __MINGW_NOTHROW	getwchar (void);
_CRTIMP wint_t __cdecl __MINGW_NOTHROW	putwc (wint_t, FILE*);
_CRTIMP wint_t __cdecl __MINGW_NOTHROW	putwchar (wint_t);
#ifndef __STRICT_ANSI__
_CRTIMP wchar_t* __cdecl __MINGW_NOTHROW _getws (wchar_t*);
_CRTIMP int __cdecl __MINGW_NOTHROW	_putws (const wchar_t*);
_CRTIMP FILE* __cdecl __MINGW_NOTHROW	_wfdopen(int, const wchar_t *);
_CRTIMP FILE* __cdecl __MINGW_NOTHROW	_wfopen (const wchar_t*, const wchar_t*);
_CRTIMP FILE* __cdecl __MINGW_NOTHROW	_wfreopen (const wchar_t*, const wchar_t*, FILE*);
_CRTIMP FILE* __cdecl __MINGW_NOTHROW	_wfsopen (const wchar_t*, const wchar_t*, int);
_CRTIMP wchar_t* __cdecl __MINGW_NOTHROW _wtmpnam (wchar_t*);
_CRTIMP wchar_t* __cdecl __MINGW_NOTHROW _wtempnam (const wchar_t*, const wchar_t*);
_CRTIMP int __cdecl __MINGW_NOTHROW	_wrename (const wchar_t*, const wchar_t*);
_CRTIMP int __cdecl __MINGW_NOTHROW	_wremove (const wchar_t*);
_CRTIMP void __cdecl __MINGW_NOTHROW	_wperror (const wchar_t*);
_CRTIMP FILE* __cdecl __MINGW_NOTHROW	_wpopen (const wchar_t*, const wchar_t*);
#endif  /* __STRICT_ANSI__ */
#endif	/* __MSVCRT__ */

And as I looked on other macros, it's pretty easy to trigger on __STRICT_ANSI__ by other macros or some compiler commands. AFAIK -std=c++ will always disable platform-specific code because it's not in the standard.

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Nov 7, 2017

Pushed to master now, closing.

@mosra mosra closed this Nov 7, 2017

Platforms automation moved this from In Progress to Done Nov 7, 2017

@mosra mosra added this to the 2018.02 milestone Feb 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.