Message ID | 20210601015653.14499-3-feifei.wang2@arm.com (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Thomas Monjalon |
Headers | show |
Series | relative path support for ABI compatibility check | expand |
Context | Check | Description |
---|---|---|
ci/iol-testing | success | Testing PASS |
ci/github-robot | success | github build: passed |
ci/iol-abi-testing | success | Testing PASS |
ci/intel-Testing | success | Testing PASS |
ci/iol-intel-Functional | fail | Functional Testing issues |
ci/iol-intel-Performance | success | Performance Testing PASS |
ci/Intel-compilation | success | Compilation OK |
ci/checkpatch | warning | coding style issues |
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
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.
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. >
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)