On 19/01/18 13:07, Marcus Folkesson wrote: > Hi Nicolas! > > On Wed, Jan 17, 2018 at 11:12:56PM +0100, Nicolas Iooss wrote: >> On Tue, Jan 16, 2018 at 9:23 PM, Marcus Folkesson >> <[email protected]> wrote: >>> This patch solves the following issues: >>> - The pkg-config files generates odd paths when using DESTDIR without PREFIX >>> - DESTDIR is needed during compile time to compute library and header paths >>> which it should not. >>> - Installing with both DESTDIR and PREFIX set gives us odd paths >>> - Make usage of DESTDIR and PREFIX more standard >>> >>> Signed-off-by: Marcus Folkesson <[email protected]> >>> --- >>> libselinux/include/Makefile | 4 ++-- >>> libselinux/man/Makefile | 7 ++++--- >>> libselinux/src/Makefile | 12 +++++------- >>> libselinux/src/libselinux.pc.in | 2 +- >>> libselinux/utils/Makefile | 6 ++---- >>> 5 files changed, 14 insertions(+), 17 deletions(-) >>> >>> diff --git a/libselinux/include/Makefile b/libselinux/include/Makefile >>> index 757a6c9c..3b51f5ce 100644 >>> --- a/libselinux/include/Makefile >>> +++ b/libselinux/include/Makefile >>> @@ -1,6 +1,6 @@ >>> # Installation directories. >>> -PREFIX ?= $(DESTDIR)/usr >>> -INCDIR ?= $(PREFIX)/include/selinux >>> +PREFIX ?= /usr >>> +INCDIR = $(DESTDIR)$(PREFIX)/include/selinux >>> >>> all: >>> >>> diff --git a/libselinux/man/Makefile b/libselinux/man/Makefile >>> index 0643e6af..233bfaa9 100644 >>> --- a/libselinux/man/Makefile >>> +++ b/libselinux/man/Makefile >>> @@ -1,7 +1,8 @@ >>> # Installation directories. >>> -MAN8DIR ?= $(DESTDIR)/usr/share/man/man8 >>> -MAN5DIR ?= $(DESTDIR)/usr/share/man/man5 >>> -MAN3DIR ?= $(DESTDIR)/usr/share/man/man3 >>> +PREFIX ?= /usr >>> +MAN8DIR ?= $(DESTDIR)$(PREFIX)/share/man/man8 >>> +MAN5DIR ?= $(DESTDIR)$(PREFIX)/share/man/man5 >>> +MAN3DIR ?= $(DESTDIR)$(PREFIX)/share/man/man3 >>> >>> all: >>> >>> diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile >>> index 18df75c8..18a58164 100644 >>> --- a/libselinux/src/Makefile >>> +++ b/libselinux/src/Makefile >>> @@ -8,8 +8,8 @@ RUBYPREFIX ?= $(notdir $(RUBY)) >>> PKG_CONFIG ?= pkg-config >>> >>> # Installation directories. >>> -PREFIX ?= $(DESTDIR)/usr >>> -LIBDIR ?= $(PREFIX)/lib >>> +PREFIX ?= /usr >>> +LIBDIR ?= $(DESTDIR)$(PREFIX)/lib >>> SHLIBDIR ?= $(DESTDIR)/lib >>> INCLUDEDIR ?= $(PREFIX)/include >>> PYINC ?= $(shell $(PKG_CONFIG) --cflags $(PYPREFIX)) >>> @@ -19,8 +19,6 @@ PYCEXT ?= $(shell $(PYTHON) -c 'import imp;print([s for >>> s,m,t in imp.get_suffixe >>> RUBYINC ?= $(shell $(RUBY) -e 'puts "-I" + >>> RbConfig::CONFIG["rubyarchhdrdir"] + " -I" + >>> RbConfig::CONFIG["rubyhdrdir"]') >>> RUBYLIBS ?= $(shell $(RUBY) -e 'puts "-L" + RbConfig::CONFIG["libdir"] + " >>> -L" + RbConfig::CONFIG["archlibdir"] + " " + >>> RbConfig::CONFIG["LIBRUBYARG_SHARED"]') >>> RUBYINSTALL ?= $(DESTDIR)$(shell $(RUBY) -e 'puts >>> RbConfig::CONFIG["vendorarchdir"]') >>> -LIBBASE ?= $(shell basename $(LIBDIR)) >>> -LIBSEPOLA ?= $(LIBDIR)/libsepol.a >>> >>> VERSION = $(shell cat ../VERSION) >>> LIBVERSION = 1 >>> @@ -148,7 +146,7 @@ $(LIBSO): $(LOBJS) >>> ln -sf $@ $(TARGET) >>> >>> $(LIBPC): $(LIBPC).in ../VERSION >>> - sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; >>> s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):; >>> s:@PCRE_MODULE@:$(PCRE_MODULE):' < $< > $@ >>> + sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; >>> s:@libdir@:$(LIBDIR):; s:@includedir@:$(INCLUDEDIR):; >>> s:@PCRE_MODULE@:$(PCRE_MODULE):' < $< > $@ >>> >>> selinuxswig_python_exception.i: ../include/selinux/selinux.h >>> bash -e exception.sh > $@ || (rm -f $@ ; false) >>> @@ -156,8 +154,8 @@ selinuxswig_python_exception.i: >>> ../include/selinux/selinux.h >>> $(AUDIT2WHYLOBJ): audit2why.c >>> $(CC) $(filter-out -Werror, $(CFLAGS)) $(PYINC) -fPIC -DSHARED -c >>> -o $@ $< >>> >>> -$(AUDIT2WHYSO): $(AUDIT2WHYLOBJ) $(LIBSEPOLA) >>> - $(CC) $(CFLAGS) $(LDFLAGS) -L. -shared -o $@ $^ -lselinux $(PYLIBS) >>> +$(AUDIT2WHYSO): $(AUDIT2WHYLOBJ) >>> + $(CC) $(CFLAGS) $(LDFLAGS) -L. -shared -o $@ $^ -lselinux $(PYLIBS) >>> -l:libsepol.a >> >> Hello, >> This change makes audit2why.so no longer being rebuilt when libsepol's >> code change. This is an issue when debugging issues in libsepol, which >> is why I added $(LIBSEPOLA) to the dependencies of $(AUDIT2WHYSO) in >> commit dcd135cc06ab ("Re-link programs after libsepol.a is updated" >> [1]). >> By the way, I like the change from using a "hardcoded" path to >> libsepol.a to telling the compiler to look into directories specified >> with option -L in LDFLAGS. This would ease the packaging a little bit, >> as it makes defining LIBSEPOLA no longer necessary (if I understood >> the changes correctly, I have not tested this point). Is there a way >> to ask the compiler for the resolved location of a static library, in >> a way which can be used to compute the value of LIBSEPOLA? ("gcc >> -Wl,--trace ..." displays it but it is not easily usable). > > First of all, thank you for your review. > > Actually, I do not have any "good" way to address this issue. > Is $(LIBSEPOLA) mostly used during debug/development? > > What do you think about this change: > > -----------------8<-------------------------------------------- > > diff --git a/libselinux/src/Makefile b/libselinux/src/Makefile > index 18df75c8..b722a584 100644 > --- a/libselinux/src/Makefile > +++ b/libselinux/src/Makefile > @@ -20,7 +20,11 @@ RUBYINC ?= $(shell $(RUBY) -e 'puts "-I" + > RbConfig::CONFIG["rubyarchhdrdir"] + > RUBYLIBS ?= $(shell $(RUBY) -e 'puts "-L" + RbConfig::CONFIG["libdir"] + " > -L" + RbConfig::CONFIG["archlibdir"] + " " + > RbConfig::CONFIG["LIBRUBYARG_SHARED"]') > RUBYINSTALL ?= $(DESTDIR)$(shell $(RUBY) -e 'puts > RbConfig::CONFIG["vendorarchdir"]') > LIBBASE ?= $(shell basename $(LIBDIR)) > -LIBSEPOLA ?= $(LIBDIR)/libsepol.a > + > +# If no specific libsepol.a is specified, fall back on LDFLAGS search path > +ifeq ($(LIBSEPOLA),) > + LDFLAGS += -l:libsepol.a > +endif > > VERSION = $(shell cat ../VERSION) > LIBVERSION = 1 > > -----------------8<-------------------------------------------- > > This will search for libsepol.a in paths specified in LDFLAGS, but keep > the possibility to "track" a specific libsepol.a to make dependent subprojects > recompile if libsepol.a is changed.
Adding "-l:libsepol.a" to LDFLAGS breaks compiling with "make
'LDFLAGS=-Wl,--as-needed,--no-undefined'" (which helps detecting shared
libraries linking issues), because of two issues:
* "LDFLAGS += ..." does nothing when LDFLAGS is defined on the command
line. This is why "override LDFLAGS += ..." is used in other places in
the Makefile.
* After adding "override", -l:libsepol.a appears before $(AUDIT2WHYLOBJ)
in the linker invocation, so its objects get ignored because of
--as-needed (which is why LDLIBS exists independently of LDFLAGS).
This issue can be easily reproduced by running in libselinux/src the
following command:
make clean && make 'LDFLAGS=-Wl,--as-needed,--no-undefined' pywrap
In order to address this issue, I suggest defining a new variable,
LDLIBS_LIBSEPOLA, defined by something like:
# If no specific libsepol.a is specified, fall back on LDFLAGS search path
# Otherwise, as $(LIBSEPOLA) already appears in the dependencies, there
is no need to define a value for LDLIBS_LIBSEPOLA
ifeq ($(LIBSEPOLA),)
LDLIBS_LIBSEPOLA := -l:libsepol.a
endif
#....
$(AUDIT2WHYSO): $(AUDIT2WHYLOBJ) $(LIBSEPOLA)
$(CC) $(CFLAGS) $(LDFLAGS) -L. -shared -o $@ $^ -lselinux
$(LDLIBS_LIBSEPOLA) $(PYLIBS)
What do you think of this?
Best regards,
Nicolas
signature.asc
Description: OpenPGP digital signature
