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

Add packages for suricata #33886

Closed
wants to merge 5 commits into from
Closed

Add packages for suricata #33886

wants to merge 5 commits into from

Conversation

dhess
Copy link
Contributor

@dhess dhess commented Jan 15, 2018

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@adisbladis
Copy link
Member

Are you willing to maintain these packages? If so you should add yourself as a maintainer in the meta.maintainers attribute.

meta = with stdenv.lib; {
homepage = https://01.org/hyperscan;
description = "A high-performance multiple regex matching library";
license = [ licenses.bsd3 licenses.bsd2 licenses.boost ];
Copy link
Member

Choose a reason for hiding this comment

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

This could be simplified as license = with licenses; [ bsd3 bsd2 boost ];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

with licenses; [ ... boost ] does not evaluate, FYI, because boost is also included in the function arguments.

Copy link
Member

Choose a reason for hiding this comment

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

with clause will take precedence and shadow function arguments in that case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Try it for yourself.

https://gist.github.com/dhess/ec32c05364c73a69b91813d0d4e7e71d

$ nix-build pkgs/top-level/release.nix -A hyperscan
trace: WARNING: `addPassthru` is deprecated, replace with `extendDerivation true`
trace: lib.zip is deprecated, use lib.zipAttrsWith instead
trace: `mkStrict' is obsolete; use `mkOverride 0' instead.
trace: `types.list` is deprecated; use `types.listOf` instead
trace: types.optionSet is deprecated; use types.submodule instead
error: Package ‘python2.7-dosage-2016.03.17’ in /home/dhess/git/nixpkgs/pkgs/top-level/python-packages.nix:1555 is marked as broken, refusing to evaluate.

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowBroken = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowBroken = true; }
to ~/.config/nixpkgs/config.nix.

(use ‘--show-trace’ to show detailed location information)

Copy link
Member

Choose a reason for hiding this comment

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

You are right; interesting.

This prints 2:

let
  a = { foo = 1; };
  b = { foo = 2; };
in

with a;
with b;

builtins.trace foo foo

And this prints 1:

{ foo ? 1 }:

let
  b = { foo = 2; };
in

with b;

builtins.trace foo foo

I'm not sure if that's intended; that doesn't seem to follow proper scoping rules. If there is no ticket against NixOS/nix on this, perhaps one should be opened.

Copy link
Member

Choose a reason for hiding this comment

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

Found it, see NixOS/nix#490.

meta = with stdenv.lib; {
homepage = https://github.com/sam-github/libnet;
description = "Portable framework for low-level network packet construction";
license = [ licenses.bsd2 licenses.bsd3 ];
Copy link
Member

Choose a reason for hiding this comment

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

Same here. Can be simplified.

, swig
}:

let
Copy link
Member

Choose a reason for hiding this comment

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

Empty let

gnutls
];

propagatedBuildInputs = [
Copy link
Member

Choose a reason for hiding this comment

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

Should all of these really be propagated?
I think these should probably be separate packages in their respective languages (eg python module in pkgs/development/python-modules).

# - CUDA support (note: requires driver API, not the runtime API, so cudatoolkit doesn't work)

{ stdenv
, lib
Copy link
Member

Choose a reason for hiding this comment

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

Better use stdenv.lib instead of lib.

, pcre
, python
, zlib
, redisSupport ? false, redis, hiredis
Copy link
Member

Choose a reason for hiding this comment

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

If Redis support is optional, it would make sense to mark redis and hiredis attributes optional as well:

, redisSupport ? false, redis ? null, hiredis ? null

Same with rustSupport.

name = "suricata-${version}";

src = fetchurl {
name = "${name}.tar.gz";
Copy link
Member

Choose a reason for hiding this comment

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

This seemingly can be omitted.


postInstall = ''
wrapProgram "$out/bin/suricatasc" \
--prefix PYTHONPATH : $PYTHONPATH:$(toPythonPath "$out")
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be --prefix PYTHONPATH : $(toPythonPath $out), seemingly $PYTHONPATH is not set. Also, consider using wrapPython.


let

in
Copy link
Member

Choose a reason for hiding this comment

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

Empty let binding.

boost
cmake
pkgconfig
python
Copy link
Member

Choose a reason for hiding this comment

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

Are boost, python and ragel build-time only dependencies in this case? Perhaps they are, I just wanted to make sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

meta = with stdenv.lib; {
homepage = https://01.org/hyperscan;
description = "A high-performance multiple regex matching library";
license = [ licenses.bsd3 licenses.bsd2 licenses.boost ];
Copy link
Member

Choose a reason for hiding this comment

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

From https://01.org/hyperscan it seems that hyperscan is only under BSD 3-Clause, and not under BSD 2-Clause or Boost licenses.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

It's common for big companies to list all dependencies' licenses in the license file. The package itself is still under BSD 3-Clause.

Copy link
Contributor Author

Choose a reason for hiding this comment

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


let

in
Copy link
Member

Choose a reason for hiding this comment

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

Empty let binding.

gnutls
];

propagatedBuildInputs = [
Copy link
Member

Choose a reason for hiding this comment

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

Why are interpreters propagated?

meta = with stdenv.lib; {
homepage = https://www.prelude-siem.org/projects/libprelude;
description = "IDMEF transport library used by all Prelude agents";
license = licenses.gpl2;
Copy link
Member

Choose a reason for hiding this comment

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

@dhess
Copy link
Contributor Author

dhess commented Jan 17, 2018

OK, thanks for all the feedback. I don't have time to fix the major issues. Maybe another time.

@dhess dhess closed this Jan 17, 2018
@dhess dhess deleted the suricata branch January 17, 2018 07:16
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.

4 participants