devtools: fix ninja break under default DESTDIR path

  If DPDK_ABI_REF_DIR is not set, the default DESTDIR is a relative path.
This will break ninja in the ABI check test.

Fixes: 777014e56d07 ("devtools: add ABI checks")

Signed-off-by: Phil Yang <phil.yang@arm.com>
---
 devtools/test-meson-builds.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Phil Yang
> Sent: Thursday, July 9, 2020 8:53 AM
> To: david.marchand@redhat.com; dev@dpdk.org
> Cc: Honnappa.Nagarahalli@arm.com; Ruifeng.Wang@arm.com; nd@arm.com
> Subject: [dpdk-dev] [PATCH] devtools: fix ninja break under default DESTDIR
> path
> 
> If DPDK_ABI_REF_DIR is not set, the default DESTDIR is a relative path.
> This will break ninja in the ABI check test.
> 

The commit message is not very clear. The problem is we're specifying ninja's destination dir in variable DESTDIR when invoking ninja install. If the DESTDIR is not an absolute path, ninja complains (when invoking install). This isn't directly related to DPDK_ABI_REF_DIR, it's more a problem with how we call install_target.

> Fixes: 777014e56d07 ("devtools: add ABI checks")
> 
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> ---
>  devtools/test-meson-builds.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh index
> a87de63..2bfcaca 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -143,7 +143,7 @@ build () # <directory> <target compiler | cross file>
> <meson options>
>  	config $srcdir $builds_dir/$targetdir $cross --werror $*
>  	compile $builds_dir/$targetdir
>  	if [ -n "$DPDK_ABI_REF_VERSION" ]; then
> -		abirefdir=${DPDK_ABI_REF_DIR:-
> reference}/$DPDK_ABI_REF_VERSION
> +		abirefdir=${DPDK_ABI_REF_DIR:-
> $(pwd)/reference}/$DPDK_ABI_REF_VERSION
>  		if [ ! -d $abirefdir/$targetdir ]; then
>  			# clone current sources
>  			if [ ! -d $abirefdir/src ]; then
> --
> 2.7.4
> 

install_target on line 160 is called improperly. Let's fix it so that it matches line 169:
install_target $abirefdir/build $(readlink -f $abirefdir/$targetdir)

This way we don't need to add $(pwd).
  

Juraj Linkeš <juraj.linkes@pantheon.tech> writes:

> > Subject: [dpdk-dev] [PATCH] devtools: fix ninja break under default
> DESTDIR
> > path
> >
> > If DPDK_ABI_REF_DIR is not set, the default DESTDIR is a relative path.
> > This will break ninja in the ABI check test.
> >
> 
> The commit message is not very clear. The problem is we're specifying ninja's
> destination dir in variable DESTDIR when invoking ninja install. If the DESTDIR
> is not an absolute path, ninja complains (when invoking install). This isn't
Yes, this defect is due to 'ninja install' not happy with the relative path. 
Will reword the commit message in the next version.

> directly related to DPDK_ABI_REF_DIR, it's more a problem with how we call
> install_target.
If we have set 'DPDK_ABI_REF_DIR' before the test, 'abiref' should be an absolute path, but the default 'abiref' value is a relative path.
So should we keep them align?

> 
> > Fixes: 777014e56d07 ("devtools: add ABI checks")
> >
> > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > ---
> >  devtools/test-meson-builds.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-
> builds.sh index
> > a87de63..2bfcaca 100755
> > --- a/devtools/test-meson-builds.sh
> > +++ b/devtools/test-meson-builds.sh
> > @@ -143,7 +143,7 @@ build () # <directory> <target compiler | cross file>
> > <meson options>
> >  	config $srcdir $builds_dir/$targetdir $cross --werror $*
> >  	compile $builds_dir/$targetdir
> >  	if [ -n "$DPDK_ABI_REF_VERSION" ]; then
> > -		abirefdir=${DPDK_ABI_REF_DIR:-
> > reference}/$DPDK_ABI_REF_VERSION
> > +		abirefdir=${DPDK_ABI_REF_DIR:-
> > $(pwd)/reference}/$DPDK_ABI_REF_VERSION
> >  		if [ ! -d $abirefdir/$targetdir ]; then
> >  			# clone current sources
> >  			if [ ! -d $abirefdir/src ]; then
> > --
> > 2.7.4
> >
> 
> install_target on line 160 is called improperly. Let's fix it so that it matches line
> 169:
> install_target $abirefdir/build $(readlink -f $abirefdir/$targetdir)

If so we need to call readlink at line #161 as well as it does at line #171.

> 
> This way we don't need to add $(pwd).

If you don't prefer $(pwd), we can do it like this:
abirefdir=$(readlink -f ${DPDK_ABI_REF_DIR:- reference}/$DPDK_ABI_REF_VERSION)

Thanks,
Phil
  

> -----Original Message-----
> From: Phil Yang <Phil.Yang@arm.com>
> Sent: Sunday, September 20, 2020 11:32 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; david.marchand@redhat.com;
> dev@dpdk.org
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: RE: [dpdk-dev] [PATCH] devtools: fix ninja break under default DESTDIR
> path
> 
> Juraj Linkeš <juraj.linkes@pantheon.tech> writes:
> 
> > > Subject: [dpdk-dev] [PATCH] devtools: fix ninja break under default
> > DESTDIR
> > > path
> > >
> > > If DPDK_ABI_REF_DIR is not set, the default DESTDIR is a relative path.
> > > This will break ninja in the ABI check test.
> > >
> >
> > The commit message is not very clear. The problem is we're specifying
> > ninja's destination dir in variable DESTDIR when invoking ninja
> > install. If the DESTDIR is not an absolute path, ninja complains (when
> > invoking install). This isn't
> Yes, this defect is due to 'ninja install' not happy with the relative path.
> Will reword the commit message in the next version.
> 
> > directly related to DPDK_ABI_REF_DIR, it's more a problem with how we
> > call install_target.
> If we have set 'DPDK_ABI_REF_DIR' before the test, 'abiref' should be an
> absolute path, but the default 'abiref' value is a relative path.
> So should we keep them align?
> 
> >
> > > Fixes: 777014e56d07 ("devtools: add ABI checks")
> > >
> > > Signed-off-by: Phil Yang <phil.yang@arm.com>
> > > ---
> > >  devtools/test-meson-builds.sh | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-
> > builds.sh index
> > > a87de63..2bfcaca 100755
> > > --- a/devtools/test-meson-builds.sh
> > > +++ b/devtools/test-meson-builds.sh
> > > @@ -143,7 +143,7 @@ build () # <directory> <target compiler | cross
> > > file> <meson options>
> > >  	config $srcdir $builds_dir/$targetdir $cross --werror $*
> > >  	compile $builds_dir/$targetdir
> > >  	if [ -n "$DPDK_ABI_REF_VERSION" ]; then
> > > -		abirefdir=${DPDK_ABI_REF_DIR:-
> > > reference}/$DPDK_ABI_REF_VERSION
> > > +		abirefdir=${DPDK_ABI_REF_DIR:-
> > > $(pwd)/reference}/$DPDK_ABI_REF_VERSION
> > >  		if [ ! -d $abirefdir/$targetdir ]; then
> > >  			# clone current sources
> > >  			if [ ! -d $abirefdir/src ]; then
> > > --
> > > 2.7.4
> > >
> >
> > install_target on line 160 is called improperly. Let's fix it so that
> > it matches line
> > 169:
> > install_target $abirefdir/build $(readlink -f $abirefdir/$targetdir)
> 
> If so we need to call readlink at line #161 as well as it does at line #171.
> 
> >
> > This way we don't need to add $(pwd).
> 
> If you don't prefer $(pwd), we can do it like this:
> abirefdir=$(readlink -f ${DPDK_ABI_REF_DIR:-
> reference}/$DPDK_ABI_REF_VERSION)
> 

I mostly wanted the code to be consistent and since readlink was already in the code I pointed that out. I think your last proposal is the best of the two worlds, but let's change the code everywhere (i.e. lines 170-174 as well).

> Thanks,
> Phil
> 
>