[v1,2/2] devtools: use absolute path for the build directory

Message ID 20210601015653.14499-3-feifei.wang2@arm.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series relative path support for ABI compatibility check |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional fail Functional Testing issues
ci/intel-Testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/github-robot success github build: passed
ci/iol-testing success Testing PASS

Commit Message

Feifei Wang June 1, 2021, 1:56 a.m. UTC
  From: Phil Yang <phil.yang@arm.com>

To make the code easier to maintain, 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>
Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 devtools/test-meson-builds.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
  

Comments

Feifei Wang July 28, 2021, 7:20 a.m. UTC | #1
Hi, Bruce

Sorry to disturb you again. Would you please help review the second patch
of this series? Thanks very much.

Best Regards
Feifei

> -----邮件原件-----
> 发件人: Feifei Wang <feifei.wang2@arm.com>
> 发送时间: Tuesday, June 1, 2021 9:57 AM
> 收件人: Bruce Richardson <bruce.richardson@intel.com>
> 抄送: dev@dpdk.org; nd <nd@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> Juraj Linkeš <juraj.linkes@pantheon.tech>; Feifei Wang
> <Feifei.Wang2@arm.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>
> 主题: [PATCH v1 2/2] devtools: use absolute path for the build directory
> 
> From: Phil Yang <phil.yang@arm.com>
> 
> To make the code easier to maintain, 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>
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>  devtools/test-meson-builds.sh | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> index 43b906598d..d6b0e7e059 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
> @@ -193,16 +193,16 @@ build () # <directory> <target cc | cross file> <ABI
> check> [meson options]
>  		fi
> 
>  		install_target $builds_dir/$targetdir \
> -			$(readlink -f $builds_dir/$targetdir/install)
> +			$builds_dir/$targetdir/install
>  		echo "Checking ABI compatibility of $targetdir" >&$verbose
>  		echo $srcdir/devtools/gen-abi.sh \
> -			$(readlink -f
> $builds_dir/$targetdir/install) >&$veryverbose
> +			$builds_dir/$targetdir/install >&$veryverbose
>  		$srcdir/devtools/gen-abi.sh \
> -			$(readlink -f
> $builds_dir/$targetdir/install) >&$veryverbose
> +			$builds_dir/$targetdir/install >&$veryverbose
>  		echo $srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
> -			$(readlink -f
> $builds_dir/$targetdir/install) >&$veryverbose
> +			$builds_dir/$targetdir/install >&$veryverbose
>  		$srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
> -			$(readlink -f
> $builds_dir/$targetdir/install) >&$verbose
> +			$builds_dir/$targetdir/install >&$verbose
>  	fi
>  }
> 
> @@ -275,7 +275,7 @@ done
>  # Test installation of the x86-generic target, to be used for checking  # the
> sample apps build using the pkg-config file for cflags and libs  load_env cc -
> build_path=$(readlink -f $builds_dir/build-x86-generic)
> +build_path=$builds_dir/build-x86-generic
>  export DESTDIR=$build_path/install
>  install_target $build_path $DESTDIR
>  pc_file=$(find $DESTDIR -name libdpdk.pc)
> --
> 2.25.1
  
Thomas Monjalon Aug. 6, 2021, 3:43 p.m. UTC | #2
01/06/2021 03:56, Feifei Wang:
> From: Phil Yang <phil.yang@arm.com>
> 
> To make the code easier to maintain, use the absolute path for the
> default build_dir to avoid repeatedly calling of readlink.
[...]
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> -builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> +builds_dir=$(readlink -f ${DPDK_BUILD_TEST_DIR:-.})

It means that all uses of builds_dir will get the absolute path.
It may have consequences on meson configuration,
and will make outputs and logs longer.

I'm not sure this change is desirable.
  
Feifei Wang Aug. 11, 2021, 3:14 a.m. UTC | #3
Hi, Thomas

Thanks for your reviewing.

I agree with your comment. 
As your concern, this patch is simple but may have some negative effects.
Thus I will drop it from series in the next version.

Best Regards
Feifei

> -----邮件原件-----
> 发件人: Thomas Monjalon <thomas@monjalon.net>
> 发送时间: Friday, August 6, 2021 11:43 PM
> 收件人: Phil Yang <Phil.Yang@arm.com>; Feifei Wang
> <Feifei.Wang2@arm.com>
> 抄送: Bruce Richardson <bruce.richardson@intel.com>; dev@dpdk.org; nd
> <nd@arm.com>; Juraj Linkeš <juraj.linkes@pantheon.tech>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; david.marchand@redhat.com
> 主题: Re: [dpdk-dev] [PATCH v1 2/2] devtools: use absolute path for the
> build directory
> 
> 01/06/2021 03:56, Feifei Wang:
> > From: Phil Yang <phil.yang@arm.com>
> >
> > To make the code easier to maintain, use the absolute path for the
> > default build_dir to avoid repeatedly calling of readlink.
> [...]
> > --- a/devtools/test-meson-builds.sh
> > +++ b/devtools/test-meson-builds.sh
> > -builds_dir=${DPDK_BUILD_TEST_DIR:-.}
> > +builds_dir=$(readlink -f ${DPDK_BUILD_TEST_DIR:-.})
> 
> It means that all uses of builds_dir will get the absolute path.
> It may have consequences on meson configuration, and will make outputs
> and logs longer.
> 
> I'm not sure this change is desirable.
>
  

Patch

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 43b906598d..d6b0e7e059 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
@@ -193,16 +193,16 @@  build () # <directory> <target cc | cross file> <ABI check> [meson options]
 		fi
 
 		install_target $builds_dir/$targetdir \
-			$(readlink -f $builds_dir/$targetdir/install)
+			$builds_dir/$targetdir/install
 		echo "Checking ABI compatibility of $targetdir" >&$verbose
 		echo $srcdir/devtools/gen-abi.sh \
-			$(readlink -f $builds_dir/$targetdir/install) >&$veryverbose
+			$builds_dir/$targetdir/install >&$veryverbose
 		$srcdir/devtools/gen-abi.sh \
-			$(readlink -f $builds_dir/$targetdir/install) >&$veryverbose
+			$builds_dir/$targetdir/install >&$veryverbose
 		echo $srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
-			$(readlink -f $builds_dir/$targetdir/install) >&$veryverbose
+			$builds_dir/$targetdir/install >&$veryverbose
 		$srcdir/devtools/check-abi.sh $abirefdir/$targetdir \
-			$(readlink -f $builds_dir/$targetdir/install) >&$verbose
+			$builds_dir/$targetdir/install >&$verbose
 	fi
 }
 
@@ -275,7 +275,7 @@  done
 # Test installation of the x86-generic target, to be used for checking
 # the sample apps build using the pkg-config file for cflags and libs
 load_env cc
-build_path=$(readlink -f $builds_dir/build-x86-generic)
+build_path=$builds_dir/build-x86-generic
 export DESTDIR=$build_path/install
 install_target $build_path $DESTDIR
 pc_file=$(find $DESTDIR -name libdpdk.pc)