[Joerg Mayer <jmayer@xxxxxxxxx>]

It took some preparational time before I could test your patches:
- I needed to get a working Windows build first
- I've finally managed to be able to compile via cygwin-ssh, so no more
  remote desktop sessions :-)
- Finish some other small issues first so I didn't have any uncommitted
  stuff in my tree.

On Sun, Nov 24, 2013 at 07:45:07PM +0000, Graham Bloice wrote:
> I've now got tshark to build from a VS solution file, had to do some hacks
> to get there though, patch files attached for others to peruse, as I'm not
> sure if they are the optimal solutions:

Before I comment on the individual things in this patch: With your stuff
committed so far I've been able to build on my Windows with both
cmake -> nmake and cmake -> vs (msbuild). Great!

>    1. I had to add some MSC version definitions to CMakeLists.txt

There are two independent parts to this patch:

1) I applied the MSC_VER_REQUIRED part (after I understood the cmake specific
   part of it).

2)
-                       set( CMAKE_PREFIX_PATH "${QT5_BASE_PATH}\\msvc2010" )
+#                      set( CMAKE_PREFIX_PATH "${QT5_BASE_PATH}\\msvc2010" )
+                       set( CMAKE_PREFIX_PATH "${QT5_BASE_PATH}" )

I have no idea why/how this is supposed to work: Is Qt5 supposed to automagically
add the right msvc version back into the path?
After applying and testing this it didn't fail right away but it no longer
found Qt5LinguistTools and Qt5PrintSupport.

>    2. I had to remove some definitions from cmakeconfig.h.in for windows.
>    The windows config.h produced for the normal nmake build is quite a bit
>    different than the cmake produced one.  I've attached my patch to get the
>    cmake build to work. Folks can check config.h.win32 to see the starting
>    point for the normal nmake build.

--- cmakeconfig.h.in    (revision 53524)
+++ cmakeconfig.h.in    (working copy)
+#if !defined(_WIN32)
 /* Define to 1 if you have the <stdlib.h> header file. */
 #cmakedefine HAVE_STDLIB_H 1
+#endif

+#if !defined(_WIN32)
 /* Define to 1 if you have the <string.h> header file. */
 #cmakedefine HAVE_STRING_H 1
+#endif

What is the problem here? I only get a warning for these two symbols. Also,
if we do not include kerberos then these symbols will not be defined at all,
which is also not a good idea. I see two cleaner solutions to this:
- Rename our HAVE_STRING_H to WS_HAVE_STRING_H
- Undef HAVE_STRING_H etc. in the outer file before including the (broken) includes
  of krb/krb5.h, not without adding a comment about bad taste in putting the
  values of the included win-mac.h unprotected into a global include file.
I'll probably go with the second one.

>    3. As mentioned in my previous message, the VS solution chops out every
>    8192nd byte from the command line passed to make-dissector-reg.py.  My
>    patch (make-reg.patch) gets CMake to write out the required source file
>    list to a file and modifies the python script to read in the file.  The
>    python changes *should* be backwards compatible.

I committed your patch but it seems to break on older python. Can you please take
a look at the OS X buildbot?

>    4. As I've moved over to building the GTK3 version, some CMake FindXXX
>    modules had to be fixed, not entirely convinced by my changes here, but it
>    works for me (Findxxx.patch).

I do my builds with GTK3 as well and they seem to build just fine. I agree that
using gtk[23] is a hack and you cleaned that up properly. What I don't understand
is why you removed the "IF( NOT GMODULE2_FOUND  )" (or similar) in
FindGMODULE2.cmake and FindGTHREAD2.cmake.

>    5. The current setpath method for fixing up paths to all the dll's and
>    executables doesn't work for VS solution builds, as a solution will usually
>    have multiple build configurations (Debug, Release, etc.) and the build
>    artefacts go in different places depending on the build config used, e.g.
>    builddir\Debug\tshark.exe, builddir\lib\Debug\libwireshark.dll.  As the
>    build config is chosen at build time, not cmake generation time the paths
>    required can't be generated by cmake.  I think we'll have to move to the
>    normal nmake option of copying everything into a target directory.

I'll look some more into this but it looks doable: After a complete build
with msbuild everything is in ./Debug/ except the dlls, which are in
./lib/Debug/, so I'm optimistic I can find a solution.

>    6. I have one warning in the tshark build:
> 
> Build succeeded.
>        "E:\Wireshark\build\Wireshark.sln" (Executables\tshark target) (1) ->
>        "E:\Wireshark\build\tshark.vcxproj.metaproj" (default target) (2) ->
>        "E:\Wireshark\build\tshark.vcxproj" (default target) (16) ->
>        (ClCompile target) ->
>          ..\trunk\capture-wpcap.c(906): warning C4003: not enough actual
> parameters for macro 'G_STRINGIFY_ARG' [E:\Wireshark\build\tshark.vcxproj]
>     1 Warning(s)
>     0 Error(s)
> Time Elapsed 00:01:26.80

I get that warning as well. It's also happening with cmake/nmake build but not
with the nmake build. Looking into the source makes it clear: I'll have to find
a way to determine the value of and assign a value to PCAP_VERSION.

> Onwards and upward, now to make dumpcap compile.

This is from svn head, so no extra patches:

C:\wireshark\build\msbuild-x86\Debug>dir *.exe
[...]
30.11.2013  23:49            57.344 capinfos.exe
30.11.2013  23:49            35.840 dftest.exe
30.11.2013  23:49           145.920 dumpcap.exe
30.11.2013  23:49            87.552 editcap.exe
30.11.2013  23:52            47.616 mergecap.exe
30.11.2013  23:54         3.823.616 qtshark.exe
30.11.2013  23:54            35.840 randpkt.exe
30.11.2013  23:54            93.184 rawshark.exe
30.11.2013  23:54            40.448 reordercap.exe
30.11.2013  23:54            78.336 text2pcap.exe
30.11.2013  23:54           385.536 tshark.exe
30.11.2013  23:54         3.016.192 wireshark.exe

Ciao
    Jörg
-- 
Joerg Mayer                                           <jmayer@xxxxxxxxx>
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.