DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] devtools: fix ninja break under default DESTDIR path
@ 2020-07-09  6:53 Phil Yang
  2020-09-17 11:08 ` Juraj Linkeš
  2020-09-23  9:40 ` [dpdk-dev] [PATCH v2 1/2] devtools: fix ninja install breakage under relative path Phil Yang
  0 siblings, 2 replies; 8+ messages in thread
From: Phil Yang @ 2020-07-09  6:53 UTC (permalink / raw)
  To: david.marchand, dev; +Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd

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(-)

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: fix ninja break under default DESTDIR path
  2020-07-09  6:53 [dpdk-dev] [PATCH] devtools: fix ninja break under default DESTDIR path Phil Yang
@ 2020-09-17 11:08 ` Juraj Linkeš
  2020-09-20  9:32   ` Phil Yang
  2020-09-23  9:40 ` [dpdk-dev] [PATCH v2 1/2] devtools: fix ninja install breakage under relative path Phil Yang
  1 sibling, 1 reply; 8+ messages in thread
From: Juraj Linkeš @ 2020-09-17 11:08 UTC (permalink / raw)
  To: Phil Yang, david.marchand, dev; +Cc: Honnappa.Nagarahalli, Ruifeng.Wang, nd



> -----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).


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: fix ninja break under default DESTDIR path
  2020-09-17 11:08 ` Juraj Linkeš
@ 2020-09-20  9:32   ` Phil Yang
  2020-09-21  6:19     ` Juraj Linkeš
  0 siblings, 1 reply; 8+ messages in thread
From: Phil Yang @ 2020-09-20  9:32 UTC (permalink / raw)
  To: Juraj Linkeš, david.marchand, dev
  Cc: Honnappa Nagarahalli, Ruifeng Wang, nd, nd

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] devtools: fix ninja break under default DESTDIR path
  2020-09-20  9:32   ` Phil Yang
@ 2020-09-21  6:19     ` Juraj Linkeš
  0 siblings, 0 replies; 8+ messages in thread
From: Juraj Linkeš @ 2020-09-21  6:19 UTC (permalink / raw)
  To: Phil Yang, david.marchand, dev; +Cc: Honnappa Nagarahalli, Ruifeng Wang, nd, nd



> -----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
> 
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] devtools: fix ninja install breakage under relative path
  2020-07-09  6:53 [dpdk-dev] [PATCH] devtools: fix ninja break under default DESTDIR path Phil Yang
  2020-09-17 11:08 ` Juraj Linkeš
@ 2020-09-23  9:40 ` Phil Yang
  2020-09-23  9:40   ` [dpdk-dev] [PATCH v2 2/2] devtools: use absolute path for the build directory Phil Yang
  2020-09-23 10:12   ` [dpdk-dev] [PATCH v2 1/2] devtools: fix ninja install breakage under relative path Juraj Linkeš
  1 sibling, 2 replies; 8+ messages in thread
From: Phil Yang @ 2020-09-23  9:40 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: juraj.linkes, Ruifeng.Wang, nd, stable, Bruce Richardson, Luca Boccassi

As the DESTDIR variable use as ninja's destination directory when invoking
ninja install. If the DESTDIR is not an absolute path, ninja complains.

Fixes: 777014e56d07 ("devtools: add ABI checks")
Cc: stable@dpdk.org

Signed-off-by: Phil Yang <phil.yang@arm.com>
---
v2:
Use readlink to ensure consistent coding style. (Juraj)

v1:
Initial version.

 devtools/test-meson-builds.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index a87de63..5236b9c 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -143,7 +143,8 @@ 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=$(readlink -f \
+			${DPDK_ABI_REF_DIR:-reference}/$DPDK_ABI_REF_VERSION)
 		if [ ! -d $abirefdir/$targetdir ]; then
 			# clone current sources
 			if [ ! -d $abirefdir/src ]; then
-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] devtools: use absolute path for the build directory
  2020-09-23  9:40 ` [dpdk-dev] [PATCH v2 1/2] devtools: fix ninja install breakage under relative path Phil Yang
@ 2020-09-23  9:40   ` Phil Yang
  2020-09-23 10:13     ` Juraj Linkeš
  2020-09-23 10:12   ` [dpdk-dev] [PATCH v2 1/2] devtools: fix ninja install breakage under relative path Juraj Linkeš
  1 sibling, 1 reply; 8+ messages in thread
From: Phil Yang @ 2020-09-23  9:40 UTC (permalink / raw)
  To: dev, david.marchand; +Cc: juraj.linkes, Ruifeng.Wang, nd, Bruce Richardson

The 'ninja install' needs absolute paths. Use the absolute path for the
default build_dir to avoid repeatedly calling of readlink.

Suggested-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Signed-off-by: Phil Yang <phil.yang@arm.com>
---
 devtools/test-meson-builds.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 5236b9c..f43f715 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -16,7 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
 
 MESON=${MESON:-meson}
 use_shared="--default-library=shared"
-builds_dir=${DPDK_BUILD_TEST_DIR:-.}
+builds_dir=$(readlink -f ${DPDK_BUILD_TEST_DIR:-.})
 
 if command -v gmake >/dev/null 2>&1 ; then
 	MAKE=gmake
@@ -168,11 +168,11 @@ build () # <directory> <target compiler | cross file> <meson options>
 		fi
 
 		install_target $builds_dir/$targetdir \
-			$(readlink -f $builds_dir/$targetdir/install)
+			$builds_dir/$targetdir/install
 		$srcdir/devtools/gen-abi.sh \
-			$(readlink -f $builds_dir/$targetdir/install)
+			$builds_dir/$targetdir/install
 		$srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
-			$(readlink -f $builds_dir/$targetdir/install)
+			$builds_dir/$targetdir/install
 	fi
 }
 
-- 
2.7.4


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] devtools: fix ninja install breakage under relative path
  2020-09-23  9:40 ` [dpdk-dev] [PATCH v2 1/2] devtools: fix ninja install breakage under relative path Phil Yang
  2020-09-23  9:40   ` [dpdk-dev] [PATCH v2 2/2] devtools: use absolute path for the build directory Phil Yang
@ 2020-09-23 10:12   ` Juraj Linkeš
  1 sibling, 0 replies; 8+ messages in thread
From: Juraj Linkeš @ 2020-09-23 10:12 UTC (permalink / raw)
  To: Phil Yang, dev, david.marchand
  Cc: Ruifeng.Wang, nd, stable, Bruce Richardson, Luca Boccassi



> -----Original Message-----
> From: Phil Yang <phil.yang@arm.com>
> Sent: Wednesday, September 23, 2020 11:40 AM
> To: dev@dpdk.org; david.marchand@redhat.com
> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Ruifeng.Wang@arm.com;
> nd@arm.com; stable@dpdk.org; Bruce Richardson
> <bruce.richardson@intel.com>; Luca Boccassi <bluca@debian.org>
> Subject: [PATCH v2 1/2] devtools: fix ninja install breakage under relative path
> 
> As the DESTDIR variable use as ninja's destination directory when invoking ninja
> install. If the DESTDIR is not an absolute path, ninja complains.
> 
> Fixes: 777014e56d07 ("devtools: add ABI checks")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> ---
> v2:
> Use readlink to ensure consistent coding style. (Juraj)
> 
> v1:
> Initial version.
> 
>  devtools/test-meson-builds.sh | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh index
> a87de63..5236b9c 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -143,7 +143,8 @@ 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=$(readlink -f \
> +			${DPDK_ABI_REF_DIR:-
> reference}/$DPDK_ABI_REF_VERSION)
>  		if [ ! -d $abirefdir/$targetdir ]; then
>  			# clone current sources
>  			if [ ! -d $abirefdir/src ]; then
> --
> 2.7.4
> 

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] devtools: use absolute path for the build directory
  2020-09-23  9:40   ` [dpdk-dev] [PATCH v2 2/2] devtools: use absolute path for the build directory Phil Yang
@ 2020-09-23 10:13     ` Juraj Linkeš
  0 siblings, 0 replies; 8+ messages in thread
From: Juraj Linkeš @ 2020-09-23 10:13 UTC (permalink / raw)
  To: Phil Yang, dev, david.marchand; +Cc: Ruifeng.Wang, nd, Bruce Richardson



> -----Original Message-----
> From: Phil Yang <phil.yang@arm.com>
> Sent: Wednesday, September 23, 2020 11:40 AM
> To: dev@dpdk.org; david.marchand@redhat.com
> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Ruifeng.Wang@arm.com;
> nd@arm.com; Bruce Richardson <bruce.richardson@intel.com>
> Subject: [PATCH v2 2/2] devtools: use absolute path for the build directory
> 
> The 'ninja install' needs absolute paths. Use the absolute path for the default
> build_dir to avoid repeatedly calling of readlink.
> 
> Suggested-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Signed-off-by: Phil Yang <phil.yang@arm.com>
> ---
>  devtools/test-meson-builds.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh index
> 5236b9c..f43f715 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -16,7 +16,7 @@ srcdir=$(dirname $(readlink -f $0))/..
> 
>  MESON=${MESON:-meson}
>  use_shared="--default-library=shared"
> -builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> +builds_dir=$(readlink -f ${DPDK_BUILD_TEST_DIR:-.})
> 
>  if command -v gmake >/dev/null 2>&1 ; then
>  	MAKE=gmake
> @@ -168,11 +168,11 @@ build () # <directory> <target compiler | cross file>
> <meson options>
>  		fi
> 
>  		install_target $builds_dir/$targetdir \
> -			$(readlink -f $builds_dir/$targetdir/install)
> +			$builds_dir/$targetdir/install
>  		$srcdir/devtools/gen-abi.sh \
> -			$(readlink -f $builds_dir/$targetdir/install)
> +			$builds_dir/$targetdir/install
>  		$srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
> -			$(readlink -f $builds_dir/$targetdir/install)
> +			$builds_dir/$targetdir/install
>  	fi
>  }
> 
> --
> 2.7.4
> 

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-09-23 10:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09  6:53 [dpdk-dev] [PATCH] devtools: fix ninja break under default DESTDIR path Phil Yang
2020-09-17 11:08 ` Juraj Linkeš
2020-09-20  9:32   ` Phil Yang
2020-09-21  6:19     ` Juraj Linkeš
2020-09-23  9:40 ` [dpdk-dev] [PATCH v2 1/2] devtools: fix ninja install breakage under relative path Phil Yang
2020-09-23  9:40   ` [dpdk-dev] [PATCH v2 2/2] devtools: use absolute path for the build directory Phil Yang
2020-09-23 10:13     ` Juraj Linkeš
2020-09-23 10:12   ` [dpdk-dev] [PATCH v2 1/2] devtools: fix ninja install breakage under relative path Juraj Linkeš

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).