diff mbox series

busybox: don't clobber dangling symlinks

Message ID 412a4a2df4b4bd31b183dba362dca4a326535624.1531825303.git.baruch@tkos.co.il
State Accepted
Headers show
Series busybox: don't clobber dangling symlinks | expand

Commit Message

Baruch Siach July 17, 2018, 11:01 a.m. UTC
We sometimes create dangling symlinks in the target directory. That is
because we need canonical targets, as relative targets don't work well
with BR2_ROOTFS_MERGED_USR. For example, the vim package installs the
/bin/vi symlink to /usr/bin/vim. This symlink might be dangling when the
build host has no vim installed there.

Patch the busybox install.sh script to avoid clobber of dangling
symlinks.

Fixes:
http://autobuild.buildroot.net/results/796/796107430db6545401d9926e84f19eaf2040b756/

Cc: Adam Duskett <aduskett@gmail.com>
Cc: Carlos Santos <casantos@datacom.com.br>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 ...l.sh-don-t-clobber-dangling-symlinks.patch | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)
 create mode 100644 package/busybox/0003-install.sh-don-t-clobber-dangling-symlinks.patch

Comments

Carlos Santos July 18, 2018, 12:36 p.m. UTC | #1
> From: "Baruch Siach" <baruch@tkos.co.il>
> To: "buildroot" <buildroot@busybox.net>
> Cc: "Baruch Siach" <baruch@tkos.co.il>, "ratbert90" <aduskett@gmail.com>, "DATACOM" <casantos@datacom.com.br>, "Yann
> Morin" <yann.morin.1998@free.fr>
> Sent: Tuesday, July 17, 2018 8:01:43 AM
> Subject: [PATCH] busybox: don't clobber dangling symlinks

> We sometimes create dangling symlinks in the target directory. That is
> because we need canonical targets, as relative targets don't work well
> with BR2_ROOTFS_MERGED_USR. For example, the vim package installs the
> /bin/vi symlink to /usr/bin/vim. This symlink might be dangling when the
> build host has no vim installed there.
> 
> Patch the busybox install.sh script to avoid clobber of dangling
> symlinks.
> 
> Fixes:
> http://autobuild.buildroot.net/results/796/796107430db6545401d9926e84f19eaf2040b756/
> 
> Cc: Adam Duskett <aduskett@gmail.com>
> Cc: Carlos Santos <casantos@datacom.com.br>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> ...l.sh-don-t-clobber-dangling-symlinks.patch | 38 +++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644
> package/busybox/0003-install.sh-don-t-clobber-dangling-symlinks.patch
> 
> diff --git
> a/package/busybox/0003-install.sh-don-t-clobber-dangling-symlinks.patch
> b/package/busybox/0003-install.sh-don-t-clobber-dangling-symlinks.patch
> new file mode 100644
> index 000000000000..b6fb5b92368f
> --- /dev/null
> +++ b/package/busybox/0003-install.sh-don-t-clobber-dangling-symlinks.patch
> @@ -0,0 +1,38 @@
> +From c9f1a877f1b9e2602913600d769edb17ee41d15d Mon Sep 17 00:00:00 2001
> +From: Baruch Siach <baruch@tkos.co.il>
> +Date: Tue, 17 Jul 2018 13:18:09 +0300
> +Subject: [PATCH] install.sh: don't clobber dangling symlinks
> +
> +Symlinks in a subdirectory that is to become target rootfs are sometimes
> +dangling because they link to canonical file names that are not present
> +on the host, but are present relative to the target rootfs root. Don't
> +copy over dangling symlinks when noclobber is enabled
> +
> +The -e test treats dangling symlinks as non-existent files. Add -h test
> +that returns true for all symlinks.
> +
> +Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> +Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> +---
> +Upstream status:
> +http://lists.busybox.net/pipermail/busybox/2018-July/086555.html
> +
> + applets/install.sh | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/applets/install.sh b/applets/install.sh
> +index 9aede0f530e2..415896893e86 100755
> +--- a/applets/install.sh
> ++++ b/applets/install.sh
> +@@ -83,7 +83,7 @@ install -m 755 busybox "$prefix/bin/busybox" || exit 1
> + for i in $h; do
> + 	appdir=`dirname "$i"`
> + 	app=`basename "$i"`
> +-	if [ x"$noclobber" = x"1" ] && [ -e "$prefix/$i" ]; then
> ++	if [ x"$noclobber" = x"1" ] && ([ -e "$prefix/$i" ] || [ -h "$prefix/$i" ]);
> then
> + 		echo "  $prefix/$i already exists"
> + 		continue
> + 	fi
> +--
> +2.18.0
> +
> --
> 2.18.0

I'm not sure if this is a good approach. Perhaps we should look for
dangling symlinks in target-finalize and stop if one is found.

For the specific case of vim, I propose a different solution:

    https://patchwork.ozlabs.org/patch/945700/
Thomas Petazzoni Aug. 21, 2018, 12:20 p.m. UTC | #2
Hello,

On Tue, 17 Jul 2018 14:01:43 +0300, Baruch Siach wrote:
> We sometimes create dangling symlinks in the target directory. That is
> because we need canonical targets, as relative targets don't work well
> with BR2_ROOTFS_MERGED_USR. For example, the vim package installs the
> /bin/vi symlink to /usr/bin/vim. This symlink might be dangling when the
> build host has no vim installed there.
> 
> Patch the busybox install.sh script to avoid clobber of dangling
> symlinks.
> 
> Fixes:
> http://autobuild.buildroot.net/results/796/796107430db6545401d9926e84f19eaf2040b756/
> 
> Cc: Adam Duskett <aduskett@gmail.com>
> Cc: Carlos Santos <casantos@datacom.com.br>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  ...l.sh-don-t-clobber-dangling-symlinks.patch | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 package/busybox/0003-install.sh-don-t-clobber-dangling-symlinks.patch

Applied to master, thanks.

Carlos: I know you disagreed with this patch, but since it has been
merged by the upstream Busybox developers, I don't see the point of not
merging it in Buildroot as well, since we will anyway get this behavior
at the next Busybox version bump.

Best regards,

Thomas
diff mbox series

Patch

diff --git a/package/busybox/0003-install.sh-don-t-clobber-dangling-symlinks.patch b/package/busybox/0003-install.sh-don-t-clobber-dangling-symlinks.patch
new file mode 100644
index 000000000000..b6fb5b92368f
--- /dev/null
+++ b/package/busybox/0003-install.sh-don-t-clobber-dangling-symlinks.patch
@@ -0,0 +1,38 @@ 
+From c9f1a877f1b9e2602913600d769edb17ee41d15d Mon Sep 17 00:00:00 2001
+From: Baruch Siach <baruch@tkos.co.il>
+Date: Tue, 17 Jul 2018 13:18:09 +0300
+Subject: [PATCH] install.sh: don't clobber dangling symlinks
+
+Symlinks in a subdirectory that is to become target rootfs are sometimes
+dangling because they link to canonical file names that are not present
+on the host, but are present relative to the target rootfs root. Don't
+copy over dangling symlinks when noclobber is enabled
+
+The -e test treats dangling symlinks as non-existent files. Add -h test
+that returns true for all symlinks.
+
+Cc: Yann E. MORIN <yann.morin.1998@free.fr>
+Signed-off-by: Baruch Siach <baruch@tkos.co.il>
+---
+Upstream status: 
+http://lists.busybox.net/pipermail/busybox/2018-July/086555.html
+
+ applets/install.sh | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/applets/install.sh b/applets/install.sh
+index 9aede0f530e2..415896893e86 100755
+--- a/applets/install.sh
++++ b/applets/install.sh
+@@ -83,7 +83,7 @@ install -m 755 busybox "$prefix/bin/busybox" || exit 1
+ for i in $h; do
+ 	appdir=`dirname "$i"`
+ 	app=`basename "$i"`
+-	if [ x"$noclobber" = x"1" ] && [ -e "$prefix/$i" ]; then
++	if [ x"$noclobber" = x"1" ] && ([ -e "$prefix/$i" ] || [ -h "$prefix/$i" ]); then
+ 		echo "  $prefix/$i already exists"
+ 		continue
+ 	fi
+-- 
+2.18.0
+