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

nftables: build documentation #73324

Merged
merged 1 commit into from Nov 13, 2019
Merged

Conversation

thefloweringash
Copy link
Member

@thefloweringash thefloweringash commented Nov 13, 2019

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 @Izorkin who touched the file last, in the absence of a maintainer field

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

nix-review passes on NixOS
diff LGTM

~100k difference in closure size

[2 built, 4 copied (19.2 MiB), 2.1 MiB DL]
https://github.com/NixOS/nixpkgs/pull/73324
1 package were build:
nftables
[23:50:00] jon@jon-desktop /home/jon/projects/nixpkgs (master)
$ nix path-info -Sh ./result
/nix/store/rkja2w44p9xf5rw9wdpyw259smkqmpim-nftables-0.9.2        37.8M
[nix-shell:/home/jon/.cache/nix-review/pr-73324]$ nix path-info -Sh ./results/nftables
/nix/store/5732r7m3n0mfg446m1zpf4ynzjmbk6hn-nftables-0.9.2        37.9M

@jonringer jonringer merged commit 3823eda into NixOS:master Nov 13, 2019
@thefloweringash thefloweringash deleted the nftables-docs branch November 13, 2019 07:56
@Izorkin
Copy link
Contributor

Izorkin commented Nov 13, 2019

Please check this variant

diff --git a/pkgs/os-specific/linux/nftables/default.nix b/pkgs/os-specific/linux/nftables/default.nix
index de3e3f6b37e..052a8dd3b5a 100644
--- a/pkgs/os-specific/linux/nftables/default.nix
+++ b/pkgs/os-specific/linux/nftables/default.nix
@@ -1,8 +1,9 @@
-{ stdenv, fetchurl, pkgconfig, bison, flex
+{ stdenv, fetchurl, pkgconfig, bison, flex, file
 , asciidoc, libxslt, findXMLCatalogs, docbook_xml_dtd_45, docbook_xsl
 , libmnl, libnftnl, libpcap
 , gmp, jansson, readline
 , withXtables ? false , iptables
+, withDebugSymbols ? false
 }:

 with stdenv.lib;
@@ -16,12 +17,8 @@ stdenv.mkDerivation rec {
     sha256 = "1x8kalbggjq44j4916i6vyv1rb20dlh1dcsf9xvzqsry2j063djw";
   };

-  configureFlags = [
-    "--with-json"
-  ] ++ optional withXtables "--with-xtables";
-
   nativeBuildInputs = [
-    pkgconfig bison flex
+    pkgconfig bison flex file
     asciidoc libxslt findXMLCatalogs docbook_xml_dtd_45 docbook_xsl
   ];

@@ -30,6 +27,16 @@ stdenv.mkDerivation rec {
     gmp readline jansson
   ] ++ optional withXtables iptables;

+  preConfigure = ''
+    substituteInPlace ./configure --replace /usr/bin/file ${file}/bin/file
+  '';
+
+  configureFlags = [
+    "--with-json"
+    "--with-mini-gmp"
+  ] ++ optional withXtables "--with-xtables"
+    ++ optional (!withDebugSymbols) "--disable-debug";
+
   meta = {
     description = "The project that aims to replace the existing {ip,ip6,arp,eb}tables framework";
     homepage = "https://netfilter.org/projects/nftables/";

Fixed this error
./configure: line 7055: /usr/bin/file: No such file or directory

Need create new PR?

@jonringer
Copy link
Contributor

I would create a new PR.

At least for me, it created man directories with what looked like correct man pages

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

3 participants