Re: [PATCH 2/2] policycoreutils: use pkg-config for libsemanage

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 3679 bytes --]

James Carter <jwcart2@gmail.com> writes:

> On Fri, Jun 27, 2025 at 11:04 AM Alyssa Ross <hi@alyssa.is> wrote:
>>
>> James Carter <jwcart2@gmail.com> writes:
>>
>> > On Wed, Jun 25, 2025 at 6:41 AM Alyssa Ross <hi@alyssa.is> wrote:
>> >>
>> >> libaudit and libbz2 are only required to be in the linker path for
>> >> static builds.  For dynamic builds, they'll be discovered through ELF
>> >> metadata.  pkg-config knows how to do the right thing in both cases,
>> >> so just use it rather than listing libsemanage's dependencies
>> >> manually.
>> >>
>> >> Fixes: da6cd3d8 ("Support static-only builds")
>> >> Closes: https://lore.kernel.org/r/87bjqebpre.fsf@redhat.com
>> >> Signed-off-by: Alyssa Ross <hi@alyssa.is>
>> >> ---
>> >>  policycoreutils/Makefile           | 3 ++-
>> >>  policycoreutils/semodule/Makefile  | 3 +--
>> >>  policycoreutils/setsebool/Makefile | 3 +--
>> >>  3 files changed, 4 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/policycoreutils/Makefile b/policycoreutils/Makefile
>> >> index 7acd51dd..0f3d62f2 100644
>> >> --- a/policycoreutils/Makefile
>> >> +++ b/policycoreutils/Makefile
>> >> @@ -3,7 +3,8 @@ SUBDIRS = setfiles load_policy newrole run_init secon sestatus semodule setseboo
>> >>  PKG_CONFIG ?= pkg-config
>> >>
>> >>  LIBSELINUX_LDLIBS := $(shell $(PKG_CONFIG) --libs libselinux)
>> >> -export LIBSELINUX_LDLIBS
>> >> +LIBSEMANAGE_LDLIBS := $(shell $(PKG_CONFIG) --libs libsemanage)
>> >> +export LIBSELINUX_LDLIBS LIBSEMANAGE_LDLIBS
>> >>
>> >
>> > I think that we actually want something like:
>> > ifeq ($(DISABLE_SHARED),y)
>> >     LIBSELINUX_LDLIBS := $(shell PKG_CONFIG_PATH=../libselinux/src
>> > $(PKG_CONFIG) --libs libselinux --static)
>> >     LIBSEMANAGE_LDLIBS := $(shell PKG_CONFIG_PATH=../libsemanage/src
>> > $(PKG_CONFIG) --libs libsemanage --static)
>> > else
>> >     LIBSELINUX_LDLIBS := $(shell PKG_CONFIG_PATH=../libselinux/src
>> > $(PKG_CONFIG) --libs libselinux)
>> >     LIBSEMANAGE_LDLIBS := $(shell PKG_CONFIG_PATH=../libsemanage/src
>> > $(PKG_CONFIG) --libs libsemanage)
>> > endif
>>
>> > As originally written, it is going to find the installed libselinux.pc
>> > and libsemanage.pc files which are not going to reflect any changes
>> > made to these files in the current build (I noticed this because your
>> > patch does make changes).
>>
>> I see.  It hadn't occurred to me that selinux userspace supported being
>> built all at once like this; I'd only seen all the components built
>> individually, which is evidently also what Petr is doing.  Your proposed
>> solution wouldn't be compatible with different components being
>> installed to different prefixes, which is how we do it in Nixpkgs, where
>> every package is installed to a unique prefix.
>>
>
> Obviously we want to support building everything separately as well as
> everything together.
>
> I think that this should work:
> LIBSELINUX_LDLIBS := $(shell
> PKG_CONFIG_PATH="$(PKG_CONFIG_PATH):../libselinux/src" $(PKG_CONFIG)
> --libs libselinux)
> LIBSEMANAGE_LDLIBS := $(shell
> PKG_CONFIG_PATH="$(PKG_CONFIG_PATH):../libsemanage/src" $(PKG_CONFIG)
> --libs libsemanage)
>
> Based on my testing:
> 1) If PKG_CONFIG_PATH is set, that will be searched first
> 2) If the pc files are not found, then "../libsemange/src" or
> "../libselinux/src" will be searched
> 3) If the pc files are still not found, then pkg-config will search
> using its defaults
>
> This seems like it should work regardless.
>
> BTW, I also tried using PKG_CONFIG_LIBDIR, but if that is set, then
> only that directory will be searched.

Looks sensible to me.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]