Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zeek: 2.5.5 -> 3.0.0 #70046

Merged
merged 3 commits into from Oct 2, 2019
Merged

zeek: 2.5.5 -> 3.0.0 #70046

merged 3 commits into from Oct 2, 2019

Conversation

marsam
Copy link
Contributor

@marsam marsam commented Sep 30, 2019

Motivation for this change

superseeds and closes #62695


feedback from @tobim in #62695:

Add caf to buildInputs and "-DCAF_ROOT_DIR=${caf}" to cmakeFlags, otherwise the internal version will be used adding unnecessary compilation time.

unfortunately, I can't get it to built with the caf from nixpkgs

Add rocksdb to buildInputs to enable brokers store module.

done

The broker python bindings require the ipaddress module which was introduced in python-3.3, so the python parameter should be switched to python3.

Not necessary, python2 users can still install python-ipaddress from pypi

It would be nice to have a toPythonModule entry in pythonPackages.nix, otherwise the module will be hidden from users.

Since I'm not familiar with the toPythonModule behavior, I’m going to tackle this later

The geoip dependency can be replaced with libmaxminddb, this also enables a requirement for ncurses in one of the tests.

done

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @tobim, I'd appreciate it you could review this PR

Copy link
Contributor

@tobim tobim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I did some light testing and the main executable worked as expected.

Add caf to buildInputs and "-DCAF_ROOT_DIR=${caf}" to cmakeFlags, otherwise the internal version will be used adding unnecessary compilation time.

unfortunately, I can't get it to built with the caf from nixpkgs

I think that is because caf got updated to 0.17 in the meantime, but zeek is still on 0.16..4

You can add to the list of maintainers.

pkgs/applications/networking/ids/zeek/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@tobim
Copy link
Contributor

tobim commented Oct 2, 2019

Sorry I meant to say add me as maintainer.

@marsam
Copy link
Contributor Author

marsam commented Oct 2, 2019

@tobim thanks for reviewing. I've addressed your comments

@marsam
Copy link
Contributor Author

marsam commented Oct 2, 2019

no problem, I added you as maintainer

@tobim
Copy link
Contributor

tobim commented Oct 2, 2019

I'll try to get around to write a function to build with packages from https://github.com/zeek/packages soon.

@marsam marsam merged commit 81e65ca into NixOS:master Oct 2, 2019
@marsam marsam deleted the update-zeek branch October 2, 2019 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants