Bug 269103 - net/wireshark: Rework port and flavourize
Summary: net/wireshark: Rework port and flavourize
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Joe Marcus Clarke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-01-22 19:39 UTC by Daniel Engberg
Modified: 2023-03-11 20:21 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (marcus)


Attachments
Patch for wireshark (15.74 KB, patch)
2023-01-22 19:39 UTC, Daniel Engberg
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Engberg freebsd_committer freebsd_triage 2023-01-22 19:39:17 UTC
Created attachment 239645 [details]
Patch for wireshark

- Use of helpers and consistent usage
- Refresh dependency list
- Use (lib)speexdsp from tree
- Add brotli as dependency, used for compressing data over HTTP2
- Flavourize port and create two variants, one including UI and one without (-nox11)
- Disable detection of git for Wireshark's build
- Rework and consolidate options, try to be as consistent as possible with the rest of the tree
- Rename DECRYPT option to GNUTLS
- Move various *dump tools to one option as they share the same dependency (libssh)
- Make LTO default option on aarch64 and amd64
- RTP audio codecs are only used with GUI so make all use the same toggle

Compile and runtime tested on 13.1-STABLE (amd64)
Poudriere bulk -t on 12.3-RELEASE (amd64)
Comment 1 Daniel Engberg freebsd_committer freebsd_triage 2023-01-22 19:49:58 UTC
A few additional notes:
This reduces the Makefile with about 100 lines which makes it a lot easier to maintain. Since RTP player is a port of Wireshark GUI I think we can assume that they also have some kind of multimedia related applications installed so it makes so just bundle all codecs into one option instead of cherry-pick a few. From what I can tell there's no obvious reason why we're splitting up wireshark into smaller packages, there's very little gain since you'll pull in most of wireshark anyway.

I forgot to mention that this makes asciidoctor optional :-)

The subports needs to go if we commit this patch as-is.

Potential user experience improvement:
You can build default flavour without wireshark (GUI)
Comment 2 Daniel Engberg freebsd_committer freebsd_triage 2023-01-28 11:14:06 UTC
Friendly ping
Comment 3 Joe Marcus Clarke freebsd_committer freebsd_triage 2023-01-28 11:21:09 UTC
Thanks for this.  FLAVORS is a good improvement here.  It might have made have sense to do a "lite" flavor.  I know there are users of the -lite ports, and this feels like somewhat of a POLA violation for them.  At the very least an UPDATING blurb will be needed to help them convert to a more central OPTIONS approach.

I need to digest this a bit more, but on the surface, this will make maintenance easier (without subports).
Comment 4 Daniel Engberg freebsd_committer freebsd_triage 2023-01-28 12:36:45 UTC
Hi,

The reasoning of dropping the -lite ports is because they make little difference in practice.

https://freebsd.pkgs.org/13/freebsd-amd64/wireshark-4.0.3.pkg.html
https://freebsd.pkgs.org/13/freebsd-amd64/wireshark-lite-4.0.3.pkg.html
There's less than 1Mbyte difference and dependency list is close to identical

tshark is slightly smaller but not by much and I think users would be fine folding that into the non-gui flavor.
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-03-03 19:28:36 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=c362cdf990186d87fc7dfaddde4e71b3b646309b

commit c362cdf990186d87fc7dfaddde4e71b3b646309b
Author:     Joe Marcus Clarke <marcus@FreeBSD.org>
AuthorDate: 2023-03-03 19:18:37 +0000
Commit:     Joe Marcus Clarke <marcus@FreeBSD.org>
CommitDate: 2023-03-03 19:18:37 +0000

    net/wireshark:  Update to 4.0.4 and refactor the port logic

    See https://www.wireshark.org/docs/relnotes/wireshark-4.0.4.html for a
    list of changes in wireshark.

    This port features the following modifications:

    - Use of helpers and consistent usage
    - Refresh dependency list
    - Use (lib)speexdsp from tree
    - Add brotli as dependency, used for compressing data over HTTP2
    - Flavourize port and create two variants, one including UI and one without (-nox11)
    - Disable detection of git for Wireshark's build
    - Rework and consolidate options, try to be as consistent as possible with the rest of the tree
    - Rename DECRYPT option to GNUTLS
    - Move various *dump tools to one option as they share the same dependency (libssh)
    - Make LTO default option on aarch64 and amd64
    - RTP audio codecs are only used with GUI so make all use the same toggle

    Thanks to diizzy for his refactoring work.  One thing to note for uses
    of the -lite ports is that they are going away.  Flavors should be used
    to build wireshark with the GUI (default) or without the GUI (nox11).

    PR:             269103
    Submitted by:   diizzy [port refactoring]

 net/wireshark/Makefile  | 227 ++++++++++++++----------------------------------
 net/wireshark/distinfo  |   6 +-
 net/wireshark/pkg-plist | 127 +++++++++++++--------------
 3 files changed, 134 insertions(+), 226 deletions(-)
Comment 6 Joe Marcus Clarke freebsd_committer freebsd_triage 2023-03-03 19:30:12 UTC
Thanks, diizzy.  Ultimately, I agree with you.  Tshark and wireshark are the biggies, and they can still be built using FLAVORs.
Comment 7 Boris Korzun 2023-03-09 13:40:10 UTC
(In reply to Daniel Engberg from comment #0)
Why are you exclude the ANDROIDDUMP option from the Makefile?

There's a very slow starting of Wireshark after upgrade to 4.0.4 without Android environment.
Comment 8 Joe Marcus Clarke freebsd_committer freebsd_triage 2023-03-09 19:54:27 UTC
(In reply to Boris Korzun from comment #7)

The reason it was removed is because it didn't bring with it any other dependencies.  Moreover, in my testing a "time" of wireshark 4.0.3 starting vs. 4.0.4 showed no differences.  Both started in about three seconds.
Comment 9 Boris Korzun 2023-03-10 08:17:22 UTC
(In reply to Joe Marcus Clarke from comment #8)
4.0.3 is starting in three seconds. But 4.0.4 is starting in ~75 seconds and trying connect to ADB while starting.
Comment 10 Boris Korzun 2023-03-10 08:24:33 UTC
Also, I've tried to remove lib/wireshark/extcap/androiddump and wireshark 4.0.4 had began to starting for 3 seconds.
Comment 11 Joe Marcus Clarke freebsd_committer freebsd_triage 2023-03-10 14:16:48 UTC
(In reply to Boris Korzun from comment #9)

This is not happening for me, so what is different?  How do you know it's trying to connect to ADB?  Do you have an "adb" in your path?
Comment 12 Boris Korzun 2023-03-11 14:21:36 UTC
(In reply to Joe Marcus Clarke from comment #11)
I'm seeing attempts to connect to closed port in my system logs while starting wireshark (with existed androiddump):

<6>1 17:15:43.962472+03:00 kernel - TCP: [127.0.0.1]:48337 to [127.0.0.1]:5037 tcpflags 0x2<SYN>; tcp_input_with_port: Connection attempt to closed port
<6>1 17:15:59.613306+03:00 syslogd - last message repeated 4 times
<6>1 17:16:48.179029+03:00 syslogd - last message repeated 2 times
Comment 13 Daniel Engberg freebsd_committer freebsd_triage 2023-03-11 18:59:35 UTC
I also tried it very briefly too but couldn't reproduce the issue. It might be worth mentioning that other distros also ships androiddump by default.
Comment 14 Boris Korzun 2023-03-11 19:16:52 UTC
(In reply to Daniel Engberg from comment #13)

Plz set net.inet.tcp.blackhole=2 and try to reproduce again.
Comment 15 Joe Marcus Clarke freebsd_committer freebsd_triage 2023-03-11 20:21:30 UTC
(In reply to Boris Korzun from comment #14)
I can confirm this does cause wireshark to pause for about 70 seconds trying to load the capture plugins.