[Pascal Artho <pascalartho@xxxxxxxxx>]

Hi Anders,

Thank you for your email. Here are some comments:

Hi,
Here's my take on how to do it, please correct me if I'm wrong or fill in any missing details.

Licensing and author files needs to be sorted out before any code can be committed.
I moved license and author information into the individual files as you have asked and pushed it.


Building the code should be optional and depending on availability of the GRAPViz library and configuration of makefiles. This is independent of platform.
Currently, this is not the case as the code is built in any case. The idea was to include the extension by default ;-). Joking aside, what I have tried was to support Windows and Ubuntu. Therefore, I included and changed everything that was needed to make it work.
For sure, it is possible to build it optionally depending on the availability of graphviz.

I would suggest that it's easiest to try this out on a *nix system first as it's a bit more complicated build wise and for the installer on Windows. I don't know much about auto tools or cmake files so someone else needs to help out here. When we have it working on *nix the source code could be committed in my opinion which would make it a bit simpler to work on a separate patch to get the build system in place for Windows and many problems may have been sorted out.
As I understand it the code would have to be protected by #ifdef HAVE_GRAPHVIZ or something like it. No header files from graphviz should be included instead a graphviz dev package would be required on the build system. Once it builds with and without the graphviz library we can move on to windows.
I agree with your explanation, it is not easy to support both systems. Alexis asked me to split the patch and remove the graphviz dll’s. Right now, my patch does not include the dll files and therefore, it supports only *nix systems. Building Wireshark with “Host Flows” on Ubuntu requires the graphviz dev package and the following commands:
./autogen.sh
./configure CXX=g++ CXXFLAGS="-std=c++11 -Wno-error=literal-suffix -Wno-error=unused-parameter" --with-geoip
make

Currently, there is no condition whether the graphviz library is available or not and therefore, whether the feature should be built or not. This needs to be done.
Personally, I do not know how to add a condition to the build process (such as --without-HAPviewer) but I can extend the code by #ifdef HAVE_GRAPHVIZ or something similar. After these changes, the source code should work on *nix.

On windows we need the graphviz windows package in our package storage the nmake setup target should download it and unpack it to a directory where the makefile will find the headers and copy whatever is needed first to the run directory wireshark-gtk2 or a subfolder (with nmake at least). Then the installer will need to include needed files and unpack them to appropriate directories when installing. In your original patch it looked like graphviz came with it's own GTK dlls, is that really needed? it would bloat the installation a bit.
In my original patch, I included the graphviz windows package in the source code (I missed the idea to put the package in the package storage). During the built process, I copied whatever is needed to the run directory or to the appropriate directories when installing. I have to check whether the graphviz GTK dlls are really needed or the standard GTK dlls are sufficient.

Best Regards,
Pascal

2015-09-01 23:22 GMT+02:00 Anders Broman <a.broman@xxxxxxxxxxxx>:

Hi,
Here's my take on how to do it, please correct me if I'm wrong or fill in any missing details.

Licensing and author files needs to be sorted out before any code can be committed.

Building the code should be optional and depending on availability of the GRAPViz library and configuration of makefiles. This is independent of platform.

I would suggest that it's easiest to try this out on a *nix system first as it's a bit more complicated build wise and
for the installer on Windows. I don't know much about auto tools or cmake files so someone else needs to help out here. When we have it working on *nix the source code could be committed in my opinion which would make it a bit simpler to work on a separate patch to get the build system in place for Windows and many problems may have
been sorted out.

As I understand it the code would have to be protected by #ifdef HAVE_GRAPVIZ or something like it. No header files
from grapviz should be included instead a garapviz dev package would be required on the build system. Once it builds with and without the grapwiz library we can move on to windows.

On windows we need the grapviz windows package in our package storage the nmake setup target should download it
and unpack it to a directory where the makefile will find the headers and copy whatever is needed first to the run directory wireshark-gtk2 or a subfolder (with nmake at least). Then the installer will need to include needed files and unpack them to appropriate directories when installing. In your original patch it looked like grapviz came with it's own
GTK dlls, is that really needed? it would bloat the installation a bit.

Best regards
Anders