[CRIU] Subject: [PATCH 2/2] Makefile variables BINDIR, SBINDIR, MANDIR, LIBDIR shall also be externally configurable.

[Dmitry Safonov]

2017-06-27 23:22 GMT+03:00 Francesco Giancane <francescogiancane8 at gmail.com>:
> Francesco Giancane
>
> Via Paolo Braccini 46, 10141 Torino (TO)
> +393933920466
> https://it.linkedin.com/in/francesco-giancane-aa7213117
>
>
>
> 2017-06-27 22:08 GMT+02:00 Dmitry Safonov <0x7f454c46 at gmail.com>:
>> Nit: using git with `git send-email --to criu at openvz.org` will make
>> sending patches much easier for you. You only need to fill
>> your post server settings in git config.
>
> Thank you for the suggestion. Will check it out for the next time. :)
>>
>> 2017-06-27 22:51 GMT+03:00 Francesco Giancane <francescogiancane8 at gmail.com>:
>>> From 551e9d2f88258f42c4f2594d7c6805c486b02f99 Mon Sep 17 00:00:00 2001
>>> From: Francesco Giancane <francescogiancane8 at gmail.com>
>>> Date: Mon, 26 Jun 2017 00:50:44 +0200
>>> Subject: [PATCH 2/2] Makefile variables BINDIR, SBINDIR, MANDIR, LIBDIR shall
>>>  also be externally configurable.
>>>
>>>  As stated in the official INSTALL.md document, BINDIR, SBINDIR, MANDIR, LIBDIR
>>>  variables shall be configurable using environment variables.
>>>
>>> i.e.
>>> export BINDIR='...'
>>> export SBINDIR='...'
>>> export MANDIR='...'
>>> export LIBDIR='...'
>>>
>>> make install
>>>
>>>  Before this patch, they were set at fixed values. Now it is possible to change
>>>  them at ease.
>>>
>>>  I preserved the possibility to add the '64' suffix or the DEB_HOST_MULTIARCH
>>>  variable as before, so that the LIBDIR is correctly computed.
>>
>> I would expect that setting LIBDIR from the environment wouldn't get
>> overwritten in
>> the makefile. What if we make all LIBDIR assigns conditional (?=) and
>> move the first
>> one LIBDIR ?= $(PREFIX)/lib under endif for DEB_HOST_MULTIARCH.
>> Also, please, don't ruin space-indention.
>
> Sorry for the indentation. My vi environment is probably not
> configured with the same indentation rules.

That's Ok, just keep the same formatting as in the other part of a file.

> To be honest, the fix I propose is the first thing that came into my
> mind: the suffix is just in case of multi arch systems,
> so I thought that this should work well. Maybe your solution is even
> more elegant than mine. No problem by my side
> if you are willing to change it the way you proposed.

Yep, so if that works for you, could you rework your patch this way?
Because that seems to be expected that if you specify LIBDIR,
the files will be installed in the directory you've specified, not in some
other directory that's formed based on your LIBDIR variable.

>
>>
>>>
>>> Signed-off-by: Francesco Giancane <francescogiancane8 at gmail.com>
>>> ---
>>>  Makefile.install | 12 ++++++------
>>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Makefile.install b/Makefile.install
>>> index 3606e1e..bd8bbd8 100644
>>> --- a/Makefile.install
>>> +++ b/Makefile.install
>>> @@ -1,10 +1,10 @@
>>>  #
>>>  # Installation paths.
>>>  PREFIX ?= /usr/local
>>> -BINDIR := $(PREFIX)/bin
>>> -SBINDIR := $(PREFIX)/sbin
>>> -MANDIR := $(PREFIX)/share/man
>>> -LIBDIR := $(PREFIX)/lib
>>> +BINDIR ?= $(PREFIX)/bin
>>> +SBINDIR ?= $(PREFIX)/sbin
>>> +MANDIR ?= $(PREFIX)/share/man
>>> +LIBDIR ?= $(PREFIX)/lib
>>>  INCLUDEDIR := $(PREFIX)/include
>>>  LIBEXECDIR := $(PREFIX)/libexec
>>>  RUNDIR ?= /run
>>> @@ -13,12 +13,12 @@ RUNDIR ?= /run
>>>  # For recent Debian/Ubuntu with multiarch support.
>>>  DEB_HOST_MULTIARCH := $(shell dpkg-architecture -qDEB_HOST_MULTIARCH
>>> 2>/dev/null)
>>>  ifneq "$(DEB_HOST_MULTIARCH)" ""
>>> -        LIBDIR := $(PREFIX)/lib/$(DEB_HOST_MULTIARCH)
>>> +LIBDIR := $(LIBDIR)/$(DEB_HOST_MULTIARCH)
>>>  else
>>>          #
>>>          # For most other systems
>>>          ifeq "$(shell uname -m)" "x86_64"
>>> -                LIBDIR := $(PREFIX)/lib64
>>> + LIBDIR := $(LIBDIR)64
>>>          endif
>>>  endif
>>>
>>> --
>>> 2.9.4
>>>
>>> Francesco Giancane
>>
>>
>>
>> --
>>              Dmitry

-- 
             Dmitry