criu: add a shell wrapper

Submitted by Dmitry Safonov on April 20, 2016, 2:49 p.m.

Details

Message ID 571796DE.9040207@virtuozzo.com
State Rejected
Series "criu: add a shell wrapper"
Headers show

Patch hide | download patch | download mbox

diff --git a/criu/Makefile b/criu/Makefile
index 1836bde5e1d3..14bf6d3c08f6 100644
--- a/criu/Makefile
+++ b/criu/Makefile
@@ -1,6 +1,7 @@ 
  # here is a workaround for a bug in libnl-3:
  # 6a8d90f5fec4 "attr: Allow attribute type 0"
  LDFLAGS                        += -Wl,--wrap=nla_parse,--wrap=nlmsg_parse
+LDFLAGS                        += -Wl,-rpath,'$$ORIGIN/../soccr'

  ARCH_DIR               := arch/$(SRCARCH)
  export ARCH_DIR

Comments

Adrian Reber April 21, 2016, 7:13 a.m.
On Wed, Apr 20, 2016 at 05:49:02PM +0300, Dmitry Safonov wrote:
> On 04/20/2016 05:28 PM, Andrew Vagin wrote:
> >On Wed, Apr 20, 2016 at 08:06:12AM +0300, Pavel Emelyanov wrote:
> >>Yes, the problem exists, I agree. But this solution is awful :(
> >>Can we better export the LD_LIBRARY_PATH in the zdtm.py script?
> >As far as I know Kir took the idea from autotools, so it's a standard
> >solution.
> >
> >We can set LD_LIBRARY_PATH, but eachone who need to execute from
> >from source, will have to do this.
> >
> >I don't know why you think it's awful:). It just works.
> 
> I may suggest - can we use ld's -rpath option?
> That will add directory to runtime library search path.
> I mean, something like that (completely untested):
> 
> --->8---
> diff --git a/criu/Makefile b/criu/Makefile
> index 1836bde5e1d3..14bf6d3c08f6 100644
> --- a/criu/Makefile
> +++ b/criu/Makefile
> @@ -1,6 +1,7 @@
>  # here is a workaround for a bug in libnl-3:
>  # 6a8d90f5fec4 "attr: Allow attribute type 0"
>  LDFLAGS                        += -Wl,--wrap=nla_parse,--wrap=nlmsg_parse
> +LDFLAGS                        += -Wl,-rpath,'$$ORIGIN/../soccr'

This sounds like a bad idea from the packaging stand point of view.
For the criu binary installed under /usr this will be the wrong path.

		Adrian
Dmitry Safonov April 21, 2016, 9:29 a.m.
On 04/21/2016 10:13 AM, Adrian Reber wrote:
> On Wed, Apr 20, 2016 at 05:49:02PM +0300, Dmitry Safonov wrote:
>> I may suggest - can we use ld's -rpath option?
>> That will add directory to runtime library search path.
>> I mean, something like that (completely untested):
>>
>> --->8---
>> diff --git a/criu/Makefile b/criu/Makefile
>> index 1836bde5e1d3..14bf6d3c08f6 100644
>> --- a/criu/Makefile
>> +++ b/criu/Makefile
>> @@ -1,6 +1,7 @@
>>   # here is a workaround for a bug in libnl-3:
>>   # 6a8d90f5fec4 "attr: Allow attribute type 0"
>>   LDFLAGS                        += -Wl,--wrap=nla_parse,--wrap=nlmsg_parse
>> +LDFLAGS                        += -Wl,-rpath,'$$ORIGIN/../soccr'
> This sounds like a bad idea from the packaging stand point of view.
> For the criu binary installed under /usr this will be the wrong path.

Hmm, so what's the point?
It can contain multiple paths:
LDFLAGS                        += 
-Wl,-rpath,'/lib:/usr/lib:$$ORIGIN/../soccr'

Or anything else.
Or I didn't get your message right.
Adrian Reber April 22, 2016, 6:31 a.m.
On Thu, Apr 21, 2016 at 12:29:11PM +0300, Dmitry Safonov wrote:
> On 04/21/2016 10:13 AM, Adrian Reber wrote:
> >On Wed, Apr 20, 2016 at 05:49:02PM +0300, Dmitry Safonov wrote:
> >>I may suggest - can we use ld's -rpath option?
> >>That will add directory to runtime library search path.
> >>I mean, something like that (completely untested):
> >>
> >>--->8---
> >>diff --git a/criu/Makefile b/criu/Makefile
> >>index 1836bde5e1d3..14bf6d3c08f6 100644
> >>--- a/criu/Makefile
> >>+++ b/criu/Makefile
> >>@@ -1,6 +1,7 @@
> >>  # here is a workaround for a bug in libnl-3:
> >>  # 6a8d90f5fec4 "attr: Allow attribute type 0"
> >>  LDFLAGS                        += -Wl,--wrap=nla_parse,--wrap=nlmsg_parse
> >>+LDFLAGS                        += -Wl,-rpath,'$$ORIGIN/../soccr'
> >This sounds like a bad idea from the packaging stand point of view.
> >For the criu binary installed under /usr this will be the wrong path.
> 
> Hmm, so what's the point?
> It can contain multiple paths:
> LDFLAGS                        +=
> -Wl,-rpath,'/lib:/usr/lib:$$ORIGIN/../soccr'
> 
> Or anything else.
> Or I didn't get your message right.

This adds some random path to the resulting binary which is only valid
during build-time and has no value on the system running the binary.

Various distributions have rules to not use RPATH at all in their
packages:

https://wiki.debian.org/RpathIssue
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Beware_of_Rpath

So RPATH might be useful during testing but not for the released binary.
There are tools to remove it after the build, but that would add an
additional step in the package building process.

		Adrian
Dmitry Safonov April 22, 2016, 10:11 a.m.
On 04/22/2016 09:31 AM, Adrian Reber wrote:
> On Thu, Apr 21, 2016 at 12:29:11PM +0300, Dmitry Safonov wrote:
>> Hmm, so what's the point?
>> It can contain multiple paths:
>> LDFLAGS                        +=
>> -Wl,-rpath,'/lib:/usr/lib:$$ORIGIN/../soccr'
>>
>> Or anything else.
>> Or I didn't get your message right.
> This adds some random path to the resulting binary which is only valid
> during build-time and has no value on the system running the binary.
>
> Various distributions have rules to not use RPATH at all in their
> packages:
>
> https://wiki.debian.org/RpathIssue
> https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Beware_of_Rpath
>
> So RPATH might be useful during testing but not for the released binary.
> There are tools to remove it after the build, but that would add an
> additional step in the package building process.

Ok, so on `make install` target it can be than removed.