Message ID | 20230628051920.2364466-1-srinuvasan_a@mentor.com |
---|---|
State | Superseded, archived |
Headers | show |
Series | classes/sdk.bbclass: remove additional "/" in new_target finding | expand |
On 28.06.23 07:19, Srinuvasan Arjunan wrote: > From: Srinuvasan A <srinuvasan.a@siemens.com> > > We observed that one additional / present in finding the new_target, > here target already start with /, hence no need to append the additional > /. > > In functionality wise there is no change. > > Signed-off-by: Srinuvasan A <srinuvasan.a@siemens.com> > --- > meta/classes/sdk.bbclass | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meta/classes/sdk.bbclass b/meta/classes/sdk.bbclass > index c6dc689..f14b447 100644 > --- a/meta/classes/sdk.bbclass > +++ b/meta/classes/sdk.bbclass > @@ -100,7 +100,7 @@ sdkchroot_finalize() { > > if [ "${target#/}" != "${target}" ]; then > basedir=$(dirname $link) > - new_target=$(realpath --no-symlinks -m --relative-to=$basedir ${ROOTFSDIR}/${target}) > + new_target=$(realpath --no-symlinks -m --relative-to=$basedir ${ROOTFSDIR}${target}) > Can you guarantee that ROOTFSDIR comes with a trailing / in ALL case? I'm skeptical about that. Jan
On Wednesday, June 28, 2023 at 12:42:52 PM UTC+5:30 Jan Kiszka wrote: On 28.06.23 07:19, Srinuvasan Arjunan wrote: > From: Srinuvasan A <srinuv...@siemens.com> > > We observed that one additional / present in finding the new_target, > here target already start with /, hence no need to append the additional > /. > > In functionality wise there is no change. > > Signed-off-by: Srinuvasan A <srinuv...@siemens.com> > --- > meta/classes/sdk.bbclass | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meta/classes/sdk.bbclass b/meta/classes/sdk.bbclass > index c6dc689..f14b447 100644 > --- a/meta/classes/sdk.bbclass > +++ b/meta/classes/sdk.bbclass > @@ -100,7 +100,7 @@ sdkchroot_finalize() { > > if [ "${target#/}" != "${target}" ]; then > basedir=$(dirname $link) > - new_target=$(realpath --no-symlinks -m --relative-to=$basedir ${ROOTFSDIR}/${target}) > + new_target=$(realpath --no-symlinks -m --relative-to=$basedir ${ROOTFSDIR}${target}) > Can you guarantee that ROOTFSDIR comes with a trailing / in ALL case? I'm skeptical about that. Actually ROOTFSDIR not contains / in all the cases, rather target always start with /, hence the additional / is not needed. f.e: Debug Logs: target=/etc/alternatives/lzma.1.gz [ etc/alternatives/lzma.1.gz != /etc/alternatives/lzma.1.gz ] dirname /home/srinu/work/TEST/ISAR_REL_PATH_CHECK_TEST/build/tmp/work/debian-bullseye-armhf/isar-image-base-sdk-qemuarm/1.0-r0/rootfs/usr/share/man/man1/lzma.1.gz basedir=/home/srinu/work/TEST/ISAR_REL_PATH_CHECK_TEST/build/tmp/work/debian-bullseye-armhf/isar-image-base-sdk-qemuarm/1.0-r0/rootfs/usr/share/man/man1 realpath --no-symlinks -m --relative-to=/home/srinu/work/TEST/ISAR_REL_PATH_CHECK_TEST/build/tmp/work/debian-bullseye-armhf/isar-image-base-sdk-qemuarm/1.0-r0/rootfs/usr/share/man/man1 /home/srinu/work/TEST/ISAR_REL_PATH_CHECK_TEST/build/tmp/work/debian-bullseye-armhf/isar-image-base-sdk-qemuarm/1.0-r0/rootfs//etc/alternatives/lzma.1.gz new_target=../../../../etc/alternatives/lzma.1.gz Jan
On 03.07.23 09:07, Srinuvasan Arjunan wrote: > > > On Wednesday, June 28, 2023 at 12:42:52 PM UTC+5:30 Jan Kiszka wrote: > > On 28.06.23 07:19, Srinuvasan Arjunan wrote: > > From: Srinuvasan A <srinuv...@siemens.com> > > > > We observed that one additional / present in finding the new_target, > > here target already start with /, hence no need to append the > additional > > /. > > > > In functionality wise there is no change. > > > > Signed-off-by: Srinuvasan A <srinuv...@siemens.com> > > --- > > meta/classes/sdk.bbclass | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/meta/classes/sdk.bbclass b/meta/classes/sdk.bbclass > > index c6dc689..f14b447 100644 > > --- a/meta/classes/sdk.bbclass > > +++ b/meta/classes/sdk.bbclass > > @@ -100,7 +100,7 @@ sdkchroot_finalize() { > > > > if [ "${target#/}" != "${target}" ]; then > > basedir=$(dirname $link) > > - new_target=$(realpath --no-symlinks -m --relative-to=$basedir > ${ROOTFSDIR}/${target}) > > + new_target=$(realpath --no-symlinks -m --relative-to=$basedir > ${ROOTFSDIR}${target}) > > > > Can you guarantee that ROOTFSDIR comes with a trailing / in ALL case? > I'm skeptical about that. > > > Actually ROOTFSDIR not contains / in all the cases, rather target > always start with /, hence the additional / is not needed. OK, we are under if [ "${target#/}" != "${target}" ]; then thus only enter this branch if target has a leading /. Maybe worth to leave a comment behind in the code nevertheless. Jan
diff --git a/meta/classes/sdk.bbclass b/meta/classes/sdk.bbclass index c6dc689..f14b447 100644 --- a/meta/classes/sdk.bbclass +++ b/meta/classes/sdk.bbclass @@ -100,7 +100,7 @@ sdkchroot_finalize() { if [ "${target#/}" != "${target}" ]; then basedir=$(dirname $link) - new_target=$(realpath --no-symlinks -m --relative-to=$basedir ${ROOTFSDIR}/${target}) + new_target=$(realpath --no-symlinks -m --relative-to=$basedir ${ROOTFSDIR}${target}) # remove first to allow rewriting directory links sudo rm $link