[PATCH] Makefile: always include and link with DESTDIR

[PATCH] Makefile: always include and link with DESTDIR

[Christian Göttsche]

The top level Makefile adds, if the environment variable DESTDIR is
defined, the according include and link directory to CFLAGS and LDFLAGS
to build all userspace tools against dependencies from this repository
and not the system.
If CFLAGS or LDFLAGS are specified by the user, e.g.

    DESTDIR=~/destdir CFLAGS=-Dfoo LDFLAGS=-Lbar make install

use the override directive to force adding DESTDIR paths to the user
specified CFLAGS or LDFLAGS.

Note that

    DESTDIR=~/destdir make CFLAGS=-Dfoo LDFLAGS=-Lbar install

does not work, since in sub-directories the internal make options take
precedence over the overridden environment variables in the top
Makefile.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 2ffba8e9..e05e924b 100644
--- a/Makefile
+++ b/Makefile
@@ -29,8 +29,8 @@ ifneq ($(DESTDIR),)
 	LIBDIR ?= $(DESTDIR)$(PREFIX)/lib
 	LIBSEPOLA ?= $(LIBDIR)/libsepol.a
 
-	CFLAGS += -I$(DESTDIR)$(PREFIX)/include
-	LDFLAGS += -L$(DESTDIR)$(PREFIX)/lib -L$(LIBDIR)
+	override CFLAGS += -I$(DESTDIR)$(PREFIX)/include
+	override LDFLAGS += -L$(DESTDIR)$(PREFIX)/lib -L$(LIBDIR)
 	export CFLAGS
 	export LDFLAGS
 	export LIBSEPOLA
-- 
2.36.1

Re: [PATCH] Makefile: always include and link with DESTDIR

[Nicolas Iooss]

On Fri, May 20, 2022 at 3:00 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> The top level Makefile adds, if the environment variable DESTDIR is
> defined, the according include and link directory to CFLAGS and LDFLAGS
> to build all userspace tools against dependencies from this repository
> and not the system.
> If CFLAGS or LDFLAGS are specified by the user, e.g.
>
>     DESTDIR=~/destdir CFLAGS=-Dfoo LDFLAGS=-Lbar make install
>
> use the override directive to force adding DESTDIR paths to the user
> specified CFLAGS or LDFLAGS.
>
> Note that
>
>     DESTDIR=~/destdir make CFLAGS=-Dfoo LDFLAGS=-Lbar install
>
> does not work, since in sub-directories the internal make options take
> precedence over the overridden environment variables in the top
> Makefile.

Hello,

>From my understanding of the documentation of "override"
(https://www.gnu.org/software/make/manual/html_node/Override-Directive.html)
it only matters when setting variables which come from the command
line, not from the environment. On my system (Arch Linux with "GNU
Make 4.3"), your first command works fine. To really be sure I
understood things correctly, I added a target into the main Makefile:

testenv:
    @echo Root Makefile: CFLAGS=$(CFLAGS)
    (cd libsepol && $(MAKE) $@)

... and added similar commands to libsepol/Makefile and
libsepol/src/Makefile. Without override, "DESTDIR=/tmp/destdir
CFLAGS=-Dfoo make testenv" displays:

Root Makefile: CFLAGS=-Dfoo -I/tmp/destdir/usr/include
libsepol Makefile: CFLAGS=-Dfoo -I/tmp/destdir/usr/include
libsepol/src Makefile: CFLAGS=-Dfoo -I/tmp/destdir/usr/include -I.
-I../include -D_GNU_SOURCE -I../cil/include -DHAVE_REALLOCARRAY

... which shows that the Makefile works as expected. Adding "override"
does not change this output. It only changes it with
"DESTDIR=/tmp/destdir make CFLAGS=-Dfoo testenv":

Root Makefile: CFLAGS=-Dfoo -I/tmp/destdir/usr/include
libsepol Makefile: CFLAGS=-Dfoo
libsepol/src Makefile: CFLAGS=-Dfoo -I. -I../include -D_GNU_SOURCE
-I../cil/include -DHAVE_REALLOCARRAY

Your patch makes the first output have " -I/tmp/destdir/usr/include"
but not the other lines, because $(MAKEFLAGS) contains "CFLAGS=-Dfoo"
(as documented on
https://www.gnu.org/software/make/manual/html_node/Variables_002fRecursion.html
). So using CFLAGS in command-line argument does not work and making
it work would require removing CFLAGS and LDFLAGS from MAKEFLAGS,
which seems fragile.

Therefore, I did not manage to reproduce the issue that your patch was
fixing and I did not understand why using "override" helped. You could
be using a specific kind of make which behaves differently as mine.
Could you please provide some way to reproduce the issue you were
experiencing (that "DESTDIR=~/destdir CFLAGS=-Dfoo LDFLAGS=-Lbar make
install" did not work on your system)?

Thanks,
Nicolas

> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 2ffba8e9..e05e924b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -29,8 +29,8 @@ ifneq ($(DESTDIR),)
>         LIBDIR ?= $(DESTDIR)$(PREFIX)/lib
>         LIBSEPOLA ?= $(LIBDIR)/libsepol.a
>
> -       CFLAGS += -I$(DESTDIR)$(PREFIX)/include
> -       LDFLAGS += -L$(DESTDIR)$(PREFIX)/lib -L$(LIBDIR)
> +       override CFLAGS += -I$(DESTDIR)$(PREFIX)/include
> +       override LDFLAGS += -L$(DESTDIR)$(PREFIX)/lib -L$(LIBDIR)
>         export CFLAGS
>         export LDFLAGS
>         export LIBSEPOLA
> --
> 2.36.1
>

Re: [PATCH] Makefile: always include and link with DESTDIR

[Christian Göttsche]

On Mon, 30 May 2022 at 00:49, Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Fri, May 20, 2022 at 3:00 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > The top level Makefile adds, if the environment variable DESTDIR is
> > defined, the according include and link directory to CFLAGS and LDFLAGS
> > to build all userspace tools against dependencies from this repository
> > and not the system.
> > If CFLAGS or LDFLAGS are specified by the user, e.g.
> >
> >     DESTDIR=~/destdir CFLAGS=-Dfoo LDFLAGS=-Lbar make install
> >
> > use the override directive to force adding DESTDIR paths to the user
> > specified CFLAGS or LDFLAGS.
> >
> > Note that
> >
> >     DESTDIR=~/destdir make CFLAGS=-Dfoo LDFLAGS=-Lbar install
> >
> > does not work, since in sub-directories the internal make options take
> > precedence over the overridden environment variables in the top
> > Makefile.
>
> Hello,
>
> >From my understanding of the documentation of "override"
> (https://www.gnu.org/software/make/manual/html_node/Override-Directive.html)
> it only matters when setting variables which come from the command
> line, not from the environment. On my system (Arch Linux with "GNU
> Make 4.3"), your first command works fine. To really be sure I
> understood things correctly, I added a target into the main Makefile:
>
> testenv:
>     @echo Root Makefile: CFLAGS=$(CFLAGS)
>     (cd libsepol && $(MAKE) $@)
>
> ... and added similar commands to libsepol/Makefile and
> libsepol/src/Makefile. Without override, "DESTDIR=/tmp/destdir
> CFLAGS=-Dfoo make testenv" displays:
>
> Root Makefile: CFLAGS=-Dfoo -I/tmp/destdir/usr/include
> libsepol Makefile: CFLAGS=-Dfoo -I/tmp/destdir/usr/include
> libsepol/src Makefile: CFLAGS=-Dfoo -I/tmp/destdir/usr/include -I.
> -I../include -D_GNU_SOURCE -I../cil/include -DHAVE_REALLOCARRAY
>
> ... which shows that the Makefile works as expected. Adding "override"
> does not change this output. It only changes it with
> "DESTDIR=/tmp/destdir make CFLAGS=-Dfoo testenv":
>
> Root Makefile: CFLAGS=-Dfoo -I/tmp/destdir/usr/include
> libsepol Makefile: CFLAGS=-Dfoo
> libsepol/src Makefile: CFLAGS=-Dfoo -I. -I../include -D_GNU_SOURCE
> -I../cil/include -DHAVE_REALLOCARRAY
>
> Your patch makes the first output have " -I/tmp/destdir/usr/include"
> but not the other lines, because $(MAKEFLAGS) contains "CFLAGS=-Dfoo"
> (as documented on
> https://www.gnu.org/software/make/manual/html_node/Variables_002fRecursion.html
> ). So using CFLAGS in command-line argument does not work and making
> it work would require removing CFLAGS and LDFLAGS from MAKEFLAGS,
> which seems fragile.
>
> Therefore, I did not manage to reproduce the issue that your patch was
> fixing and I did not understand why using "override" helped. You could
> be using a specific kind of make which behaves differently as mine.
> Could you please provide some way to reproduce the issue you were
> experiencing (that "DESTDIR=~/destdir CFLAGS=-Dfoo LDFLAGS=-Lbar make
> install" did not work on your system)?

Thanks for reviewing.
I must have mixed up something, cause

    DESTDIR=~/destdir CFLAGS=-Dfoo LDFLAGS=-Lbar make install

works indeed.
Please disregard this patch.

>
> Thanks,
> Nicolas
>
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >  Makefile | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 2ffba8e9..e05e924b 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -29,8 +29,8 @@ ifneq ($(DESTDIR),)
> >         LIBDIR ?= $(DESTDIR)$(PREFIX)/lib
> >         LIBSEPOLA ?= $(LIBDIR)/libsepol.a
> >
> > -       CFLAGS += -I$(DESTDIR)$(PREFIX)/include
> > -       LDFLAGS += -L$(DESTDIR)$(PREFIX)/lib -L$(LIBDIR)
> > +       override CFLAGS += -I$(DESTDIR)$(PREFIX)/include
> > +       override LDFLAGS += -L$(DESTDIR)$(PREFIX)/lib -L$(LIBDIR)
> >         export CFLAGS
> >         export LDFLAGS
> >         export LIBSEPOLA
> > --
> > 2.36.1
> >
>