Re: Integrating the HAPViewer code.

[Pascal Artho <pascalartho () gmail com>]

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 () bredband net>:

> 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
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe