[waffle] waffle vs msvc: Round 1, Fight !!!

[Emil Velikov]

On 2 July 2014 00:22, Chad Versace <chad.versace at intel.com> wrote:
> On Mon, Jun 30, 2014 at 01:05:27AM +0100, Emil Velikov wrote:
>> Hello all,
>>
>> Another day another long email :)
>>
>> As usual, a list of issues, possible solutions, and my personal
>> preference for each. At the end I have tried to assign a priority for
>> most.
>>
>> Any input would be greatly appreciated.
>>
>> Highlights:
>>
>> General (nitbits):
>>  * C "Hello world" does not work OOTB with VC12 :)
>>  ** Have to switch the platform toolset to v120_xp, as the default
>> v120 complains about missing headers, libraries... (SDKDDKVer.h...),
>>
>>  * Compiling ninja - fails due to the missing toolset.
>>  ** Planning to use VC for now.
>>
>> Jose are you having similar experience with VC12 ? Pardon but I'm
>> using VC12 for the first time, and the documentation out there is
>> rather... :\
>>
>>
>> Build system related:
>>  * Tests fail to build due to the missing toolset (+ others?)
>>  Possible fix(es)
>>  ** if (windows && !mingw) hardcode generator and toolset across whole
>> of waffle endif ()
>>
>>  * cmake does not find some headers and libs - GL/gl.h windows.h opengl32.lib
>>  Possible fix(es)
>>  ** Hardcode the libname, and ignore the headers' location as they
>> should just work (tm)
>
> On you GSOC2014 branch, I see that you're searching the headers with:
>
>   if(waffle_on_windows)
>       find_path(opengl_INCLUDE_DIR GL/gl.h REQUIRED)
>       ...
>   endif()
>
> What happens if you instead do this?
>
>   if(waffle_on_windows)
>       find_path(OpenGL REQUIRED)
>       ...
>   endif()
>
Hmm that looks rather strange according to the documentation. Either
way, cmake still failed to find the paths/library, yet it seems that
hard coding the library name was the only thing required :)

>> POSIX only
>>  * getopt (utils/wflinfo, examples/gl_basic) - reuse one of the
>> following (license implications?)
>>  Current implementations
>>  ** http://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/pkgtools/libnbcompat/files/getopt.c
>> (BSD 3-clause)
>>  ** http://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-crt/misc/getopt.c
>> (unknown + BSD 2-clause)
>>  Leaning towards reusing the NetBSD's implementation.
>
> I also prefer to reuse NetBSD's getopt. As long as VisualStudio can
> compile it :/
>
With a couple of small changes it seems to be working :)

>
>>  * pthreads - reuse one of the following (license implications?)
>>  Current implementations
>>  ** http://locklessinc.com/downloads/winpthreads.h (BSD 3-clause)
>>  ** https://www.sourceware.org/pthreads-win32/ (GNU Lesser GPL)
>>  ** http://cgit.freedesktop.org/mesa/mesa/tree/include/c11/threads_win32.h
>> (Boost Software License, Version 1.0)
>>  ** http://sourceforge.net/p/mingw-w64/mingw-w64/ci/master/tree/mingw-w64-libraries/winpthreads
>> (unknown [2]) - allegedly uses undocumented Windows API's
>>  Leaning towards reusing locklessinc's implementation.
>
> For now, I also prefer to use a permissively licensed simulation of the pthreads
> API. Ideally, using a pthreads wrapper means you could simply drop in the
> winpthreads.h header and waffle's pthreads usage should just work.
>
> If time were not a decision factor, I would also consider Mesa's
> simulation of the standard C11 threads API. But that would require you
> to make more changes to Waffle, and therefore take more time.
>
I do not really mind spending the extra time this week, if that
benefits waffle in the future.

>> Other
>>  * Symbols must be identically annotated in both declaration and
>> definition (i.e. we need to bring WAFFLEAPI back into the public
>> header ?)
>>  Possible fix(es)
>>  ** Pre-parse the header and ship one without the keyword ?
>>  ** Define an empty WAFFLEAPI when using MSVC and use "module
>> definition file (.def)"
>>  Leaning towards using a .def
>
> I want to avoid mangling the header sources. Instead, let's use
> the preprocessor to solve this problem.
>
> I suggest examining /usr/include/KHR/khrplatform.h for possible
> solutions.
>
Perhaps I was not clear enough, the first option involves mangling
with the header and parsing it through cmake.

While the latter is just a version-script like file (lists the
exported functions), and a one line change

--- a/src/waffle/api/api_priv.h
+++ b/src/waffle/api/api_priv.h
 #if defined(_WIN32)
-#   define WAFFLE_API __declspec(dllexport)
+#   define WAFFLE_API
 #elif defined(__GNUC__) && __GNUC__ >= 4
#endif

Pardon for going against your suggestion here, option which seems
cleaner imho. Will take a look at the former if you feel strongly
about it.

FWIW every header coming from Khronos provides its own keyword for
exported symbols which I hope is not what we want in waffle (read
"bring back WAFFLEAPI in the public header").

>>  * struct waffle_* {} wfl - not strictly supported by C99 ?
>> http://stackoverflow.com/questions/755305/empty-structure-in-c
>>  MSVC message "C requires that a struct or union has at least one member"
>>  Possible fix(es)
>>  ** Add a dummy param inside the struct. We do not rely on the struct
>> having a zero size but use it as a pointer to the start of the parent
>> struct ?
>>
>> Chad, does the last point make sense or did I completely miss out the
>> plot while reading through the internal API ?
>
> Ugh. I regret doing that little bit of nastiness in the internal API.
> I just submitted a patch to the list, you cc'd, to fix this.
>
Well considering that you wanted the member offset to be calculated
prior to the cast (container_of macro) I believe that it made sense at
the time :)

Long story short so far:
- msvc spotted a few trivial typos(bugs) in waffle.
- msvc c99 support is far from complete (missing a few functions,
inline, restrict ...).
- apart from pthreads (and testing of course) everything else is done :)

We're almost at the final stage of the project wohoo  \o/

Big thanks for the help.
Emil

>> Priority (high to low)
>>  * struct waffle_* {} wfl
>>  * pthreads
>>  * getopt
>>  * WAFFLEAPI
>>  * everything else
>>
>>
>> Cheers,
>> Emil