On Sun, Mar 1, 2020 at 1:25 PM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote: > > On Fri, Feb 28, 2020 at 2:32 PM <bill.c.roberts@xxxxxxxxx> wrote: > > > > From: William Roberts <william.c.roberts@xxxxxxxxx> > > > > If libsepol is older than version 3.0, the required routine > > sepol_policydb_optimize will not be present. Use pkg-config to get the > > modversion and check that it is 3.0 or higher. > > > > This can manifest as a compilation issue: > > direct_api.c: In function ‘semanage_direct_commit’: > > direct_api.c:1466:13: error: implicit declaration of function ‘sepol_policydb_optimize’; did you mean ‘sepol_policydb_to_image’? [-Werror=implicit-function-declaration] > > retval = sepol_policydb_optimize(out); > > > > Which is not really clear on how to check. > > >From my point of view, this kind of dependency management is something > that is more suited in a package management system than in a Makefile. > It makes sense to check for libsepol version if there is some kind of > optional features that gets enabled according to it (in a similar way > as a --with-... statement in build systems using autoconf) or if there > is a fall back to another compatible source code (and the Makefile > would "route" the building process to the right .c file). But these > reasons do not match what you are doing in this change. Since we don't use autotools, selinux makefiles have to be smarter. We already use PKG_CONFIG to get various flags, which would normally be handled in a configure script, which if we had autotools, would also be a place to check dependency versions. > > Why do you need this patch, instead of stating in the package to use > for libsemanage that it depends on libsepol>=3.0? We should document that as well, but make software smarter, not people. If we can provide a better error message, without a huge burden of work or maintenance I always vote to do it. In this case, it's pretty simple to do, since we already have the infrastructure for PKG_CONFIG in the Makefiles. > > Thanks, > Nicolas > > > Signed-off-by: William Roberts <william.c.roberts@xxxxxxxxx> > > --- > > libsemanage/src/Makefile | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/libsemanage/src/Makefile b/libsemanage/src/Makefile > > index f6780dc6048e..a329797fe1cc 100644 > > --- a/libsemanage/src/Makefile > > +++ b/libsemanage/src/Makefile > > @@ -65,6 +65,14 @@ SWIG = swig -Wall -python -o $(SWIGCOUT) -outdir ./ > > > > SWIGRUBY = swig -Wall -ruby -o $(SWIGRUBYCOUT) -outdir ./ > > > > +sepol-version-check: > > + @v=$$($(PKG_CONFIG) --modversion libsepol); \ > > + major=$$(echo "$$v" | cut -d"." -f 1-1); \ > > + if [ "$$major" -lt 3 ]; then \ > > + >&2 echo "libsepol is required to be version 3.0 or higher, got: $$v"; \ > > + exit 1; \ > > + fi > > + > > all: $(LIBA) $(LIBSO) $(LIBPC) > > > > pywrap: all $(SWIGSO) > > @@ -83,12 +91,12 @@ $(SWIGSO): $(SWIGLOBJ) > > $(SWIGRUBYSO): $(SWIGRUBYLOBJ) > > $(CC) $(CFLAGS) $(LDFLAGS) -L. -shared -o $@ $^ -lsemanage $(RUBYLIBS) > > > > -$(LIBA): $(OBJS) > > - $(AR) rcs $@ $^ > > +$(LIBA): sepol-version-check $(OBJS) > > + $(AR) rcs $@ $(filter-out $<,$^) > > $(RANLIB) $@ > > > > -$(LIBSO): $(LOBJS) > > - $(CC) $(CFLAGS) $(LDFLAGS) -shared -o $@ $^ -lsepol -laudit -lselinux -lbz2 -Wl,-soname,$(LIBSO),--version-script=libsemanage.map,-z,defs > > +$(LIBSO): sepol-version-check $(LOBJS) > > + $(CC) $(CFLAGS) $(LDFLAGS) -shared -o $@ $(filter-out $<,$^) -lsepol -laudit -lselinux -lbz2 -Wl,-soname,$(LIBSO),--version-script=libsemanage.map,-z,defs > > ln -sf $@ $(TARGET) > > > > $(LIBPC): $(LIBPC).in ../VERSION > > @@ -163,4 +171,4 @@ distclean: clean > > indent: > > ../../scripts/Lindent $(filter-out $(GENERATED),$(wildcard *.[ch])) > > > > -.PHONY: all clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean > > +.PHONY: all clean pywrap rubywrap swigify install install-pywrap install-rubywrap distclean sepol-version-check > > -- > > 2.17.1 > > >
