Wireshark update to 2.4, enable Qt5 by default, correctly mark dependencies #17298
Conversation
pkg-config was required for autotools, cmake works also without it, so remove it. CMake has no issues with parallel install either. Since Qt5 has become the default, explicitly add ENABLE_QT5=OFF. Since the formula has libgcrypt as dependency, ENABLE_GCRYPT=ON is not needed either (in fact, this option is removed in Wireshark 2.4). Reformat other argument handling for readability.
Optional extcap modules (sshdump and ciscodump) depend on libssh.
When people install Wireshark, they often need more than just tshark. See for example the top-voted answer to "installing Wireshark": https://stackoverflow.com/questions/26242156/install-wireshark-on-macos-x-via-brew While at it, install the Wireshark binary using its lowercase name to match other platforms.
Lua dissectors and scripts can conveniently be added without recompiling Wireshark, since it is a small dependency just mark it as recommended.
Change to xz archive, apply patch to fix the build and make the ENABLE_NGHTTP2 option work. Upstream patches: https://code.wireshark.org/review/22725 (merged) https://code.wireshark.org/review/23247 (under review)
Lekensteyn
force-pushed
the
wireshark-formula
branch
from
710e1fa
to
6be7b4b
Compare
|
Going to pass on this PR because we do not make GUI options default and want to keep the options to a minimum. Also, we've already decided to wait for 2.4.1 as noted here: #16504 (comment) But thank you nonetheless @Lekensteyn! |
|
In order to reduce the number of options, should I mark some as required, rather than optional/recommended? Note that the imagemagick package also has a dozen of options. According to analytics (Install Events), 19520 use I proposed this PR now rather than in a few days to get some feedback and was aware of the two other PRs for 2.4.0. |
|
@Lekensteyn regardless of the analytics, it's a matter of policy: https://github.com/Homebrew/brew/blob/c6290ed45913338160fa5acb84a423612b886960/docs/Acceptable-Formulae.md#stuff-that-builds-a-gui-by-default-but-doesnt-have-to
We want fewer formulae like that, not more.
Please encourage upstream to release 2.4.1 as soon as possible. |
So would you recommending using less recommends/optional and mark small dependencies as required instead? Wireshark 2.4.1 should be released tomorrow, should be fast enough right :) |
Not fast enough! :)
The question is why you want to add them. We want to minimize dependencies where possible but if there is a compelling use-case for a specific dependency, we can consider requiring it. |
|
Also, regarding the GUI, it's worth noting that it's available in Cask, which is the preferred way to install GUI software via Homebrew. |
Most of these dependencies are small (except nghttp2 due to its dependencies) and can be marked as required to reduce the number of options. I would even go as far and mark Lua, c-ares and geoip as required since these are small, but enhance the functionality (name resolution used to be possible without c-ares, but now it is required). |
|
New proposal for enabling more features (remove |
brew install --build-from-source <formula>, where<formula>is the name of the formula you're submitting?brew audit --strict <formula>(after doingbrew install <formula>)?A couple of patches to add new optional dependencies for Wireshark 2.4 and enabling Qt GUI by default. Please see the individual commit messages for details. Squash as needed.
Note that the fixups in the last patch will not be necessary after the upcoming 2.4.1 release (planned for Tuesday 29 August), but I have added them so the build can be tested by CI. (I have already tested each change on 10.11 El Capitan).
Question: should the number of options be reduced if possible? libssh is quite a small dependency. If more configurability is desired, note that gnutls is currently technically optional (and could be marked as such).