wireshark: explicitly disable the Qt build when building Gtk #33477
Conversation
|
The package was updated cd7ce7c, probably caused by wireshark/wireshark@09ba63e. |
| @@ -23,7 +23,7 @@ in stdenv.mkDerivation { | |||
| sha256 = "0bpiby916k3k8bm7q8b1dflva6zs0a4ircskrck0d538dfcrb50q"; | |||
| }; | |||
|
|
|||
| cmakeFlags = optional withGtk "-DBUILD_wireshark_gtk=TRUE"; | |||
| cmakeFlags = optional withGtk "-DBUILD_wireshark_gtk=TRUE -DBUILD_wireshark=OFF"; | |||
There was a problem hiding this comment.
You can have both withGtk and withQt enabled. It would be better to turn them on/off separately.
|
Please fix |
|
@orivej Respecting |
In a recent Cmake(?) or wireshark release the default behaviour did change. A failing build log can be seen at hydra [1]. [1] https://hydra.nixos.org/build/67179559/nixlog/1
| @@ -23,7 +23,8 @@ in stdenv.mkDerivation { | |||
| sha256 = "0bpiby916k3k8bm7q8b1dflva6zs0a4ircskrck0d538dfcrb50q"; | |||
| }; | |||
|
|
|||
| cmakeFlags = optional withGtk "-DBUILD_wireshark_gtk=TRUE"; | |||
| cmakeFlags = (if withGtk then ["-DBUILD_wireshark_gtk=TRUE"] else ["-DBUILD_wireshark_gtk=OFF"]) ++ | |||
| (if withQt then ["-DBUILD_wireshark=TRUE"] else ["-DBUILD_wireshark=OFF"]); | |||
There was a problem hiding this comment.
It would probably be nicer to factor this out to a function, see for example
https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/libraries/stxxl/default.nix#L6
There was a problem hiding this comment.
Including the use of ON/OFF instead of TRUE/OFF.
There was a problem hiding this comment.
With only two options inline is also fine, but it can be simplified to cmakeFlags = [ "-DBUILD_wireshark_gtk=${if withGTK then "ON" else "OFF"}" … ].
There was a problem hiding this comment.
I still think 0–1–∞ rule results in more aesthetically pleasing code but this will work too.
There was a problem hiding this comment.
That rule is pretty, but such duplication as this one is slightly easier to reason about both for the author of the change and for the readers. (Because it is local and it does not introduce any new names or functions.)
This should also fix the wireshark-cli variant
|
updates to the variant proposed in #33477 (comment) |
orivej
changed the title
|
Thank you! |
Motivation for this change
In a recent CMAKE(?) or wireshark release the default behavior did
change. A failing build log can be seen at hydra [1].
[1] https://hydra.nixos.org/build/67179559/nixlog/1
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)