Hello, Am Mittwoch, 16. Dezember 2015 schrieb Tyler Hicks: > diff --git a/README b/README > index 4ebd25d..4797b78 100644 > --- a/README > +++ b/README
> @@ -104,7 +112,7 @@ $ make check # depends on the parser having been > built first $ make install > > > -[Note that for the parser and the utils, if you only with to > build/use > +[Note that for the parser, binutils, and utils, if you > only with to unrelated, but please s/with/wish/ > build/use some of the locale languages, you can override > the default by passing the LANGS arguments to make; e.g. make all > install "LANGS=en_US fr".] > > diff --git a/binutils/Makefile b/binutils/Makefile > new file mode 100644 > index 0000000..c4c4c2e > --- /dev/null > +++ b/binutils/Makefile My general note that this looks like a copy of parser/Makefile and splitting out common parts to something like common/Make-c.rules would make sense still stands - but this can be done as a separate patch. Also, please make sure that the (mostly acked, but not yet commited) changes for parser/Makefile also get applied to binutils/Makefile to keep the difference small. > +.PHONY: install-rhel4 > +install-rhel4: install-redhat > + > +.PHONY: install-redhat > +install-redhat: > + > +.PHONY: install-suse > +install-suse: > + > +.PHONY: install-slackware > +install-slackware: > + > +.PHONY: install-debian > +install-debian: > + > +.PHONY: install-unknown > +install-unknown: > + > +INSTALLDEPS=arch > + > +ifndef DISTRO > +DISTRO=$(shell if [ -f /etc/slackware-version ] ; then \ > + echo slackware ; \ > + elif [ -f /etc/debian_version ] ; then \ > + echo debian ;\ > + elif which rpm > /dev/null ; then \ > + if [ "$(rpm --eval '0%{?suse_version}')" != "0" ] ; then \ > + echo suse ;\ > + elif [ "$(rpm --eval '%{_host_vendor}')" = redhat ] ; then > \ + echo rhel4 ;\ > + elif [ "$(rpm --eval '0%{?fedora}')" != "0" ] ; then \ > + echo rhel4 ;\ > + else \ > + echo unknown ;\ > + fi ;\ > + else \ > + echo unknown ;\ > + fi) > +endif > + > +ifdef DISTRO > +INSTALLDEPS+=install-$(DISTRO) > +endif All the install-$distribution (and also the distro detection) seems to be unused. Do we really need it? Note that we should probably keep "INSTALLDEPS=arch". (Again, this can be done in a follow-up patch.) > +clean: pod_clean > + rm -f core core.* *.o *.s *.a *~ *.gcda *.gcno > + rm -f gmon.out > + rm -f $(TOOLS) $(TESTS) > + rm -f $(NAME)*.tar.gz $(NAME)*.tgz Show me where this tarball gets built, and I'll accept that we need to delete it ;-) (I'd guess this sniplet is from the good old days when you [1] still had separate tarballs for profiles, utils, parser etc.) > diff --git a/binutils/aa-enabled.pod b/binutils/aa-enabled.pod > new file mode 100644 > index 0000000..217bc65 > --- /dev/null > +++ b/binutils/aa-enabled.pod > +=head1 DESCRIPTION > + > +B<aa-enabled> is used to determine if AppArmor is enabled and > enforcing +policy. With the (in comparison to aa-status --enabled) removal of the check if at least one profile is loaded, the "and enforcing policy" part looks a bit confusing to me. Removing that check is ok, I just want to be sure nobody misunderstands what aa-enabled does. The release notes should also make that clear, and explain that aa-enabled mostly replaces aa-status --enabled, but will return 0 even if no profiles are loaded (which is probably an improvement when used in package scripts). > +=head1 EXIT STATUS ... > +=item 2: > + > +intentionally not used as an B<aa-enabled> exit status. I wonder if we should explain the reason for this (see above). > diff --git a/binutils/aa_enabled.c b/binutils/aa_enabled.c > new file mode 100644 > index 0000000..3ca84b0 > --- /dev/null > +++ b/binutils/aa_enabled.c Looks good - but I'll let acking to someone who understands C better ;-) Regards, Christian Boltz [1] the switch to a single tarball happened before I started to work on AppArmor, therefore I'm not using "we" here ;-) -- Es gibt einen alten chinesischen Fluch, Captain: Mögest Du in interessanteren Zeiten leben! Inzwischen ist es höchst interessant, wenn Neelix in der Küche brutzelt. ["Harry Kim" in Star Trek: Voyager - Der mysteriöse Nebel] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor