Greetings. I apologize in advance if this is not the right place to discuss this. If it's not, please point me the right direction and I will move it there. :) I am packaging up libnftables/nftables for Fedora. libnftables has already passed review: https://bugzilla.redhat.com/show_bug.cgi?id=1036319 and nftables has yet to be reviewed: https://bugzilla.redhat.com/show_bug.cgi?id=1036320 but has some comments. (more always welcome) I have some questions/comments/suggestions based on the packaging that I thought would be good to run by nftables developers. 1. Completely minor, but noted in review of both packages that the COPYING file has the old fsf address in it. Would be great if you could update to the new one. https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address 2. There is some question about the /etc/nftables/* scripts. In Fedora land, things in /etc/ should be config files, but these aren't really config files. They call nft without a full path (/usr/sbin/nft, etc). Should these really be in /usr/share ? or is it expected users will modify them? Could you clarify the use case there? 3. nftables hard codes installing as root, which is no good for building packages (patch attached that just removes the owner/group setting there). 4. nftables sets make to '-s' (ie, silent) on subdirs. It's good for building packages to have verbose output in build logs. Would it be possible to remove that? If not I can patch it out here. 5. Is there currently, or planned any version coupling between libnftables and nftables? Obviously we want them to stay somewhat close, but down the road will compat be just by soname/version? 6. I recently enabled the xml stuff in libnftables and am seeing a number of tests fail: parsing xmlfiles/55-rule-real.xml: [31mFAILED[0m (Invalid argument) and parsing xmlfiles/74-set.xml: [31mFAILED[0m (Invalid argument) mxml: <!-- nft add rule filter output ct secmark 0 counter --> cannot be a second root node after <nftables> Full build log at: http://kojipkgs.fedoraproject.org//packages/libnftables/0/0.4.20140111git.fc21/data/logs/x86_64/build.log Are these expected? The Invalid argument might be because it doesn't have nftables available in the build kernel? But the json tests work. :( Thanks. Again, if I should send this somewhere else instead, just let me know. Comments welcome here, direct email and/or in the above review bugs. ;) kevin
diff -Nur nftables-0.0.orig/doc/Makefile.in nftables-0.0/doc/Makefile.in
--- nftables-0.0.orig/doc/Makefile.in 2013-11-29 03:29:41.000000000 -0700
+++ nftables-0.0/doc/Makefile.in 2013-11-30 14:31:33.159267834 -0700
@@ -10,11 +10,11 @@
@echo -e " INSTALL\tdoc"
if test -n "$(mandocs-y)"; then \
$(MKDIR_P) $(DESTDIR)/${mandir}/man8 ;\
- $(INSTALL) -m 755 -o root -g root $(mandocs-y) \
+ $(INSTALL) -m 755 -p $(mandocs-y) \
$(DESTDIR)/${mandir}/man8/ ;\
fi
if test -n "$(pdfdocs-y)"; then \
$(MKDIR_P) $(DESTDIR)/${pdfdir} ;\
- $(INSTALL) -m 755 -o root -g root $(pdfdocs-y) \
+ $(INSTALL) -m 755 -p $(pdfdocs-y) \
$(DESTDIR)/${pdfdir}/ ;\
fi
diff -Nur nftables-0.0.orig/files/Makefile.in nftables-0.0/files/Makefile.in
--- nftables-0.0.orig/files/Makefile.in 2013-11-29 03:29:41.000000000 -0700
+++ nftables-0.0/files/Makefile.in 2013-11-30 14:30:35.440421941 -0700
@@ -1,4 +1,4 @@
install:
@echo -e " INSTALL\tfiles"
$(MKDIR_P) $(DESTDIR)/$(confdir)
- $(INSTALL) -m 755 -o root -g root $(SUBDIR)nftables/* $(DESTDIR)/$(confdir)/
+ $(INSTALL) -m 755 -p $(SUBDIR)nftables/* $(DESTDIR)/$(confdir)/
diff -Nur nftables-0.0.orig/Makefile.rules.in nftables-0.0/Makefile.rules.in
--- nftables-0.0.orig/Makefile.rules.in 2013-11-29 03:29:41.000000000 -0700
+++ nftables-0.0/Makefile.rules.in 2013-11-30 14:26:03.461158244 -0700
@@ -61,7 +61,7 @@
$(1)-install:
@echo -e " INSTALL\t$1"
$(MKDIR_P) $$(DESTDIR)/$$($(1)-destdir)
- $(INSTALL) -m 755 -o root -g root \
+ $(INSTALL) -m 755 -p \
$(SUBDIR)$(1) \
$$(DESTDIR)/$$($(1)-destdir)/$(1)
install_targets += $(1)-install
Attachment:
signature.asc
Description: PGP signature
