libselinux: set PREFIX to generate proper .pc file

Message ID 20180106145249.3090-1-marcus.folkesson@gmail.com
State Superseded
Headers show
Series
  • libselinux: set PREFIX to generate proper .pc file
Related show

Commit Message

Marcus Folkesson Jan. 6, 2018, 2:52 p.m.
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 package/libselinux/libselinux.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Matthew Weber Jan. 7, 2018, 4:34 a.m. | #1
Marcus,

On Sat, Jan 6, 2018 at 8:52 AM, Marcus Folkesson
<marcus.folkesson@gmail.com> wrote:

Please add a link to the autobuilder failure for libostree that this resolves.

I tested both one of the build failures and test-pkg with the
following as checkpolicy depends on libsepol/libselinux:
BR2_PACKAGE_HOST_CHECKPOLICY=y
BR2_PACKAGE_CHECKPOLICY=y
BR2_PACKAGE_LIBSEMANAGE=y

Tested-by: Matt Weber <matt@thewebers.ws>
Thomas Petazzoni Jan. 7, 2018, 2:16 p.m. | #2
Hello,

Thanks for continuing the work on this topic!

On Sat,  6 Jan 2018 15:52:49 +0100, Marcus Folkesson wrote:
> Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> ---
>  package/libselinux/libselinux.mk | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/package/libselinux/libselinux.mk b/package/libselinux/libselinux.mk
> index 8ac8000de5..bf33da7f75 100644
> --- a/package/libselinux/libselinux.mk
> +++ b/package/libselinux/libselinux.mk
> @@ -43,17 +43,19 @@ LIBSELINUX_MAKE_INSTALL_TARGETS += install-pywrap
>  # dependencies are broken and result in file truncation errors at link
>  # time if the Python bindings are built through the same make
>  # invocation as the rest of the library.
> +# DESTDIR is needed during the compile to compute library and header paths.
>  define LIBSELINUX_BUILD_PYTHON_BINDINGS
>  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \
>  		$(LIBSELINUX_MAKE_OPTS) DESTDIR=$(STAGING_DIR) swigify pywrap
>  endef
>  endif # python || python3
>  
> +# DESTDIR is needed during the compile to compute library and header paths.
>  define LIBSELINUX_BUILD_CMDS
>  	# DESTDIR is needed during the compile to compute library and
>  	# header paths.
>  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \
> -		$(LIBSELINUX_MAKE_OPTS) DESTDIR=$(STAGING_DIR) all
> +		$(LIBSELINUX_MAKE_OPTS) DESTDIR=$(STAGING_DIR) PREFIX=/usr all
>  	$(LIBSELINUX_BUILD_PYTHON_BINDINGS)
>  endef

How can this work without passing PREFIX=/usr also at install time ?
(Same question for libsemanage and libsepol).

Thanks,

Thomas
Marcus Folkesson Jan. 7, 2018, 4:24 p.m. | #3
Hi Thomas,

On Sun, Jan 07, 2018 at 03:16:18PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> Thanks for continuing the work on this topic!
> 
> On Sat,  6 Jan 2018 15:52:49 +0100, Marcus Folkesson wrote:
> > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
> > ---
> >  package/libselinux/libselinux.mk | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/package/libselinux/libselinux.mk b/package/libselinux/libselinux.mk
> > index 8ac8000de5..bf33da7f75 100644
> > --- a/package/libselinux/libselinux.mk
> > +++ b/package/libselinux/libselinux.mk
> > @@ -43,17 +43,19 @@ LIBSELINUX_MAKE_INSTALL_TARGETS += install-pywrap
> >  # dependencies are broken and result in file truncation errors at link
> >  # time if the Python bindings are built through the same make
> >  # invocation as the rest of the library.
> > +# DESTDIR is needed during the compile to compute library and header paths.
> >  define LIBSELINUX_BUILD_PYTHON_BINDINGS
> >  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \
> >  		$(LIBSELINUX_MAKE_OPTS) DESTDIR=$(STAGING_DIR) swigify pywrap
> >  endef
> >  endif # python || python3
> >  
> > +# DESTDIR is needed during the compile to compute library and header paths.
> >  define LIBSELINUX_BUILD_CMDS
> >  	# DESTDIR is needed during the compile to compute library and
> >  	# header paths.
> >  	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \
> > -		$(LIBSELINUX_MAKE_OPTS) DESTDIR=$(STAGING_DIR) all
> > +		$(LIBSELINUX_MAKE_OPTS) DESTDIR=$(STAGING_DIR) PREFIX=/usr all
> >  	$(LIBSELINUX_BUILD_PYTHON_BINDINGS)
> >  endef
> 
> How can this work without passing PREFIX=/usr also at install time ?
> (Same question for libsemanage and libsepol).

If not PREFIX is passed, it get the value of $(DESTDIR)/usr, which I
think is correct?

If PREFIX=/usr at install time, it tries to install on the host
system.

I guess autotools would have done a better job with DESTDIR and PREFIX.

What I can tell, the libselinux.pc is the only target that make use of
PREFIX during compile time.

But maybe I should set PREFIX explicitly to $(XXXXX_DIR)/usr during
installation just for clarity?

From libselinux/src/Makefile:
PREFIX ?= $(DESTDIR)/usr


> 
> Thanks,
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com


Best regards
Marcus Folkesson
Thomas Petazzoni Jan. 7, 2018, 4:34 p.m. | #4
Hello,

On Sun, 7 Jan 2018 17:24:34 +0100, Marcus Folkesson wrote:

> > How can this work without passing PREFIX=/usr also at install time ?
> > (Same question for libsemanage and libsepol).  
> 
> If not PREFIX is passed, it get the value of $(DESTDIR)/usr, which I
> think is correct?

No, in the standard semantic of DESTDIR and PREFIX:

 - PREFIX should be /usr, and DESTDIR $(TARGET_DIR) or $(STAGING_DIR)
   for target installation

 - PREFIX should be $(HOST_DIR) for host installation, DESTDIR is not
   used

PREFIX is where the software is going to be executed from. So the
software is going to be executed from /usr for target software, from
$(HOST_DIR) for host software. PREFIX potentially has an effect on the
stuff being built: PREFIX might end up being hardcoded into the
binaries being compiled.

DESTDIR is used to divert the installation. It is only meaningful
during installation. It has no effect on the stuff being built: DESTDIR
is never going to be hardcoded in a binary.

> If PREFIX=/usr at install time, it tries to install on the host
> system.

That's bogus.

> I guess autotools would have done a better job with DESTDIR and PREFIX.

Yes.

> What I can tell, the libselinux.pc is the only target that make use of
> PREFIX during compile time.
> 
> But maybe I should set PREFIX explicitly to $(XXXXX_DIR)/usr during
> installation just for clarity?

In the standard (autotools) semantic of PREFIX,
PREFIX=$(TARGET_DIR)/usr is wrong, see above.

> From libselinux/src/Makefile:
> PREFIX ?= $(DESTDIR)/usr

Yes that's just plain wrong.

So, two options:

 - We fix libselinux Makefile to have a more standard DESTDIR/PREFIX
   usage.

 - We keep libselinux Makefile as-is, but we had some comments in
   libselinux.mk that explain the weird PREFIX/DESTDIR usage.

Thanks!

Thomas
Marcus Folkesson Jan. 7, 2018, 4:51 p.m. | #5
Hi Thomas,

On Sun, Jan 07, 2018 at 05:34:27PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Sun, 7 Jan 2018 17:24:34 +0100, Marcus Folkesson wrote:
> 
> > > How can this work without passing PREFIX=/usr also at install time ?
> > > (Same question for libsemanage and libsepol).  
> > 
> > If not PREFIX is passed, it get the value of $(DESTDIR)/usr, which I
> > think is correct?
> 
> No, in the standard semantic of DESTDIR and PREFIX:
> 
>  - PREFIX should be /usr, and DESTDIR $(TARGET_DIR) or $(STAGING_DIR)
>    for target installation
> 
>  - PREFIX should be $(HOST_DIR) for host installation, DESTDIR is not
>    used
> 
> PREFIX is where the software is going to be executed from. So the
> software is going to be executed from /usr for target software, from
> $(HOST_DIR) for host software. PREFIX potentially has an effect on the
> stuff being built: PREFIX might end up being hardcoded into the
> binaries being compiled.
> 
> DESTDIR is used to divert the installation. It is only meaningful
> during installation. It has no effect on the stuff being built: DESTDIR
> is never going to be hardcoded in a binary.
> 

You are right.

> > If PREFIX=/usr at install time, it tries to install on the host
> > system.
> 
> That's bogus.

Just to make clear - *this* package will try to install on the host
system.

> 
> > I guess autotools would have done a better job with DESTDIR and PREFIX.
> 
> Yes.
> 
> > What I can tell, the libselinux.pc is the only target that make use of
> > PREFIX during compile time.
> > 
> > But maybe I should set PREFIX explicitly to $(XXXXX_DIR)/usr during
> > installation just for clarity?
> 
> In the standard (autotools) semantic of PREFIX,
> PREFIX=$(TARGET_DIR)/usr is wrong, see above.
> 
> > From libselinux/src/Makefile:
> > PREFIX ?= $(DESTDIR)/usr
> 
> Yes that's just plain wrong.
> 
> So, two options:
> 
>  - We fix libselinux Makefile to have a more standard DESTDIR/PREFIX
>    usage.

I think we should go for this one in the long run.
I will try to come up with something and get it upstreams for the
selinux project.

Should we keep libselinux Makefile as-is in meanwhile or add patches?

I can take responsibility for the packages being fixed when it is
upstreams.

> 
>  - We keep libselinux Makefile as-is, but we had some comments in
>    libselinux.mk that explain the weird PREFIX/DESTDIR usage.

> 
> Thanks!
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com


Best regards
Marcus Folkesson
Thomas Petazzoni Jan. 7, 2018, 7:40 p.m. | #6
Hello,

On Sun, 7 Jan 2018 17:51:32 +0100, Marcus Folkesson wrote:

> > > If PREFIX=/usr at install time, it tries to install on the host
> > > system.  
> > 
> > That's bogus.  
> 
> Just to make clear - *this* package will try to install on the host
> system.

I don't follow you here.

> >  - We fix libselinux Makefile to have a more standard DESTDIR/PREFIX
> >    usage.  
> 
> I think we should go for this one in the long run.
> I will try to come up with something and get it upstreams for the
> selinux project.
> 
> Should we keep libselinux Makefile as-is in meanwhile or add patches?

We can add patches, especially if you are pushing them upstream.

Thanks!

Thomas

Patch

diff --git a/package/libselinux/libselinux.mk b/package/libselinux/libselinux.mk
index 8ac8000de5..bf33da7f75 100644
--- a/package/libselinux/libselinux.mk
+++ b/package/libselinux/libselinux.mk
@@ -43,17 +43,19 @@  LIBSELINUX_MAKE_INSTALL_TARGETS += install-pywrap
 # dependencies are broken and result in file truncation errors at link
 # time if the Python bindings are built through the same make
 # invocation as the rest of the library.
+# DESTDIR is needed during the compile to compute library and header paths.
 define LIBSELINUX_BUILD_PYTHON_BINDINGS
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \
 		$(LIBSELINUX_MAKE_OPTS) DESTDIR=$(STAGING_DIR) swigify pywrap
 endef
 endif # python || python3
 
+# DESTDIR is needed during the compile to compute library and header paths.
 define LIBSELINUX_BUILD_CMDS
 	# DESTDIR is needed during the compile to compute library and
 	# header paths.
 	$(TARGET_MAKE_ENV) $(MAKE) -C $(@D) \
-		$(LIBSELINUX_MAKE_OPTS) DESTDIR=$(STAGING_DIR) all
+		$(LIBSELINUX_MAKE_OPTS) DESTDIR=$(STAGING_DIR) PREFIX=/usr all
 	$(LIBSELINUX_BUILD_PYTHON_BINDINGS)
 endef