| Message ID | 20180103210851.26851-1-marcus.folkesson@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libselinux: add patch to create a proper pkg-config file | expand |
Hello, On Wed, 3 Jan 2018 22:08:51 +0100, Marcus Folkesson wrote: > `includedir` in libselinux.pc is set to $(PREFIX)/include if > not specified. This result in an incorrect include path when using > pkg-config. > > Output from `pkg-config -cflags libselinux` without this patch: > -I/home/marcus/git/buildroot-ostree/output/host/bin/../mips64el-buildroot-linux-gnu/sysroot/home/marcus/git/buildroot-ostree/output/host/mips64el-buildroot-linux-gnu/sysroot/usr/include > -I/home/marcus/git/buildroot-ostree/output/host/bin/../mips64el-buildroot-linux-gnu/sysroot/usr/include > > Output from `pkg-config -cflags libselinux` with this patch: > -I/home/marcus/git/buildroot-ostree/output/host/bin/../mips64el-buildroot-linux-gnu/sysroot/usr/include > > This is normally not an issue unless the depending package is compiled > with `-Werror=missing-include-dirs` as it will be treated as an error. > > Fixes: > http://autobuild.buildroot.net/results/680458dc049d2c286918aeed745515894f8fcefa/ > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> Indeed, the .pc file currently installed is bogus. > diff --git a/package/libselinux/0004-libselinux-introduce-PCPREFIX-substitute-variables-f.patch b/package/libselinux/0004-libselinux-introduce-PCPREFIX-substitute-variables-f.patch > new file mode 100644 > index 0000000000..978d0591f3 > --- /dev/null > +++ b/package/libselinux/0004-libselinux-introduce-PCPREFIX-substitute-variables-f.patch > @@ -0,0 +1,38 @@ > +libselinux: introduce PCPREFIX substitute variable for .pc files > + > +`prefix` in the .pc file may be messed up when using a buildsystem > +that has specified a sysroot as DESTDIR. > +We need to make it possible to override the default `libdir` > +and `includedir`. > + > +`includedir` may be overridden by `INCLUDEDIR` but `libdir` is using > +`PREFIX` to setup the path. > + > +Therefore, introduce PCPREFIX to make it possible to generate a more > +customized .pc file. > + > +Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > + > +--- libselinux.orig/src/Makefile 2018-01-03 21:44:49.548561421 +0100 > ++++ libselinux/src/Makefile 2018-01-03 21:44:35.581894904 +0100 > +@@ -9,9 +9,10 @@ > + > + # Installation directories. > + PREFIX ?= $(DESTDIR)/usr > ++PCPREFIX ?= $(DESTDIR)/usr > + LIBDIR ?= $(PREFIX)/lib > + SHLIBDIR ?= $(DESTDIR)/lib > +-INCLUDEDIR ?= $(PREFIX)/include > ++INCLUDEDIR ?= $(PCPREFIX)/include Meh, this is messy :-/ Can we instead switch to using the correct semantic for DESTDIR and PREFIX ? I.e: - PREFIX defines where the program/library will be installed when executed. - DESTDIR is only passed at installation, to divert the installation to a specific folder. I.e, for the target version, PREFIX=/usr DESTDIR=$(TARGET_DIR), and for the host version, PREFIX=$(HOST_DIR). Using this correct semantic would be a lot more readable. Indeed, with your solution, one wonders why PCPREFIX is used for INCLUDEDIR but not LIBDIR. Could you have a look into this ? Thomas
Hi, I have played around with this a lot today, and I'am not happy with any solution I came up with. On Wed, Jan 03, 2018 at 10:26:26PM +0100, Thomas Petazzoni wrote: > Hello, > > On Wed, 3 Jan 2018 22:08:51 +0100, Marcus Folkesson wrote: > > `includedir` in libselinux.pc is set to $(PREFIX)/include if > > not specified. This result in an incorrect include path when using > > pkg-config. > > > > Output from `pkg-config -cflags libselinux` without this patch: > > -I/home/marcus/git/buildroot-ostree/output/host/bin/../mips64el-buildroot-linux-gnu/sysroot/home/marcus/git/buildroot-ostree/output/host/mips64el-buildroot-linux-gnu/sysroot/usr/include > > -I/home/marcus/git/buildroot-ostree/output/host/bin/../mips64el-buildroot-linux-gnu/sysroot/usr/include > > > > Output from `pkg-config -cflags libselinux` with this patch: > > -I/home/marcus/git/buildroot-ostree/output/host/bin/../mips64el-buildroot-linux-gnu/sysroot/usr/include > > > > This is normally not an issue unless the depending package is compiled > > with `-Werror=missing-include-dirs` as it will be treated as an error. > > > > Fixes: > > http://autobuild.buildroot.net/results/680458dc049d2c286918aeed745515894f8fcefa/ > > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > > Indeed, the .pc file currently installed is bogus. > > > diff --git a/package/libselinux/0004-libselinux-introduce-PCPREFIX-substitute-variables-f.patch b/package/libselinux/0004-libselinux-introduce-PCPREFIX-substitute-variables-f.patch > > new file mode 100644 > > index 0000000000..978d0591f3 > > --- /dev/null > > +++ b/package/libselinux/0004-libselinux-introduce-PCPREFIX-substitute-variables-f.patch > > @@ -0,0 +1,38 @@ > > +libselinux: introduce PCPREFIX substitute variable for .pc files > > + > > +`prefix` in the .pc file may be messed up when using a buildsystem > > +that has specified a sysroot as DESTDIR. > > +We need to make it possible to override the default `libdir` > > +and `includedir`. > > + > > +`includedir` may be overridden by `INCLUDEDIR` but `libdir` is using > > +`PREFIX` to setup the path. > > + > > +Therefore, introduce PCPREFIX to make it possible to generate a more > > +customized .pc file. > > + > > +Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > > + > > +--- libselinux.orig/src/Makefile 2018-01-03 21:44:49.548561421 +0100 > > ++++ libselinux/src/Makefile 2018-01-03 21:44:35.581894904 +0100 > > +@@ -9,9 +9,10 @@ > > + > > + # Installation directories. > > + PREFIX ?= $(DESTDIR)/usr > > ++PCPREFIX ?= $(DESTDIR)/usr > > + LIBDIR ?= $(PREFIX)/lib > > + SHLIBDIR ?= $(DESTDIR)/lib > > +-INCLUDEDIR ?= $(PREFIX)/include > > ++INCLUDEDIR ?= $(PCPREFIX)/include > > Meh, this is messy :-/ Yeah. > > Can we instead switch to using the correct semantic for DESTDIR and > PREFIX ? I.e: > > - PREFIX defines where the program/library will be installed when > executed. > > - DESTDIR is only passed at installation, to divert the installation > to a specific folder. > > I.e, for the target version, PREFIX=/usr DESTDIR=$(TARGET_DIR), and for > the host version, PREFIX=$(HOST_DIR). I have tried this but did not get it to compile properly. I will look at this more tomorrow. > > Using this correct semantic would be a lot more readable. Indeed, with > your solution, one wonders why PCPREFIX is used for INCLUDEDIR but not > LIBDIR. No comment :-) > > Could you have a look into this ? Sure! > > Thomas I will try look more to make it right with PREFIX and DESTDIR. Other solutions I have been looking at is to just set INCLUDEDIR. `pkg-config --libs libselinux` will still report the messy path, but non existing library paths is not an issue. At least it is not worse than it is today. Or just set OT_DEP_SELINUX_CFLAGS in libostree package to skip the whole pkg-config thing for selinux. (libostree is the reason why I'm looking at this). But both is quite hacky. Thank you Best regards Marcus Folkesson
Marcus, On Wed, Jan 3, 2018 at 4:15 PM, Marcus Folkesson <marcus.folkesson@gmail.com> wrote: > > On Wed, Jan 03, 2018 at 10:26:26PM +0100, Thomas Petazzoni wrote: > > Hello, > > [snip] > > > + # Installation directories. > > > + PREFIX ?= $(DESTDIR)/usr > > > ++PCPREFIX ?= $(DESTDIR)/usr > > > + LIBDIR ?= $(PREFIX)/lib > > > + SHLIBDIR ?= $(DESTDIR)/lib > > > +-INCLUDEDIR ?= $(PREFIX)/include > > > ++INCLUDEDIR ?= $(PCPREFIX)/include > > > > Meh, this is messy :-/ > > Yeah. > > > > > Can we instead switch to using the correct semantic for DESTDIR and > > PREFIX ? I.e: > > > > - PREFIX defines where the program/library will be installed when > > executed. > > > > - DESTDIR is only passed at installation, to divert the installation > > to a specific folder. > > > > I.e, for the target version, PREFIX=/usr DESTDIR=$(TARGET_DIR), and for > > the host version, PREFIX=$(HOST_DIR). > This was really similar to some of the other selinux packages I've had to deal with prefix/destdir on, so I took a try at resolving the target piece of this (didn't look at the host build). What I noticed is I was getting double include paths, even when the raw PC file looks correct. Examples below ++++++Proposed patch++++++ https://nopaste.xyz/?c89837863d676de5#0lBP2mX+omzeRvLYNmvgHjxy/ZE1DZcM+21dK2UKVO8= +++++Raw libselinux.pc file it generates++++++ prefix=/usr exec_prefix=${prefix} libdir=${exec_prefix}/lib includedir=/usr/include Name: libselinux Description: SELinux utility library Version: 2.7 URL: http://userspace.selinuxproject.org/ Requires.private: libsepol libpcre Libs: -L${libdir} -lselinux Cflags: -I${includedir} ++++Output from `output/host/bin/pkg-config -cflags libselinux` using my patch. The initial include looks correct now, but I can't find how I'm getting the second.....++++++ -Ioutput/host/bin/../mips64el-buildroot-linux-gnu/sysroot/usr/include -Ioutput/host/bin/../mips64el-buildroot-linux-gnu/sysroot/nvme/rc-buildroot-hardening/output/host/mips64el-buildroot-linux-gnu/sysroot/usr/include Matt
diff --git a/package/libselinux/0004-libselinux-introduce-PCPREFIX-substitute-variables-f.patch b/package/libselinux/0004-libselinux-introduce-PCPREFIX-substitute-variables-f.patch new file mode 100644 index 0000000000..978d0591f3 --- /dev/null +++ b/package/libselinux/0004-libselinux-introduce-PCPREFIX-substitute-variables-f.patch @@ -0,0 +1,38 @@ +libselinux: introduce PCPREFIX substitute variable for .pc files + +`prefix` in the .pc file may be messed up when using a buildsystem +that has specified a sysroot as DESTDIR. +We need to make it possible to override the default `libdir` +and `includedir`. + +`includedir` may be overridden by `INCLUDEDIR` but `libdir` is using +`PREFIX` to setup the path. + +Therefore, introduce PCPREFIX to make it possible to generate a more +customized .pc file. + +Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> + +--- libselinux.orig/src/Makefile 2018-01-03 21:44:49.548561421 +0100 ++++ libselinux/src/Makefile 2018-01-03 21:44:35.581894904 +0100 +@@ -9,9 +9,10 @@ + + # Installation directories. + PREFIX ?= $(DESTDIR)/usr ++PCPREFIX ?= $(DESTDIR)/usr + LIBDIR ?= $(PREFIX)/lib + SHLIBDIR ?= $(DESTDIR)/lib +-INCLUDEDIR ?= $(PREFIX)/include ++INCLUDEDIR ?= $(PCPREFIX)/include + PYINC ?= $(shell $(PKG_CONFIG) --cflags $(PYPREFIX)) + PYLIBS ?= $(shell $(PKG_CONFIG) --libs $(PYPREFIX)) + PYSITEDIR ?= $(DESTDIR)$(shell $(PYTHON) -c 'import site; print(site.getsitepackages()[0])') +@@ -148,7 +149,7 @@ + ln -sf $@ $(TARGET) + + $(LIBPC): $(LIBPC).in ../VERSION +- sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PREFIX):; s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):' < $< > $@ ++ sed -e 's/@VERSION@/$(VERSION)/; s:@prefix@:$(PCPREFIX):; s:@libdir@:$(LIBBASE):; s:@includedir@:$(INCLUDEDIR):' < $< > $@ + + selinuxswig_python_exception.i: ../include/selinux/selinux.h + bash -e exception.sh > $@ || (rm -f $@ ; false) diff --git a/package/libselinux/libselinux.mk b/package/libselinux/libselinux.mk index 8ac8000de5..4d4a9d99e6 100644 --- a/package/libselinux/libselinux.mk +++ b/package/libselinux/libselinux.mk @@ -36,7 +36,8 @@ endif LIBSELINUX_MAKE_OPTS += \ PYINC="$(LIBSELINUX_PYINC)" \ PYSITEDIR=$(TARGET_DIR)/usr/lib/$(LIBSELINUX_PYLIBVER)/site-packages \ - SWIG_LIB="$(HOST_DIR)/share/swig/$(SWIG_VERSION)/" + SWIG_LIB="$(HOST_DIR)/share/swig/$(SWIG_VERSION)/" \ + PCPREFIX=/usr LIBSELINUX_MAKE_INSTALL_TARGETS += install-pywrap
`includedir` in libselinux.pc is set to $(PREFIX)/include if not specified. This result in an incorrect include path when using pkg-config. Output from `pkg-config -cflags libselinux` without this patch: -I/home/marcus/git/buildroot-ostree/output/host/bin/../mips64el-buildroot-linux-gnu/sysroot/home/marcus/git/buildroot-ostree/output/host/mips64el-buildroot-linux-gnu/sysroot/usr/include -I/home/marcus/git/buildroot-ostree/output/host/bin/../mips64el-buildroot-linux-gnu/sysroot/usr/include Output from `pkg-config -cflags libselinux` with this patch: -I/home/marcus/git/buildroot-ostree/output/host/bin/../mips64el-buildroot-linux-gnu/sysroot/usr/include This is normally not an issue unless the depending package is compiled with `-Werror=missing-include-dirs` as it will be treated as an error. Fixes: http://autobuild.buildroot.net/results/680458dc049d2c286918aeed745515894f8fcefa/ Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> --- ...introduce-PCPREFIX-substitute-variables-f.patch | 38 ++++++++++++++++++++++ package/libselinux/libselinux.mk | 3 +- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 package/libselinux/0004-libselinux-introduce-PCPREFIX-substitute-variables-f.patch