[Buildroot] [PATCH] toolchain-external: handle case of dangling symlink

[Buildroot] [PATCH] toolchain-external: handle case of dangling symlink

[Markus Mayer via buildroot]

copy_toolchain_lib_root was not handling the case of "readlink"
returning nothing, which will happen if the symlink it is trying to
resolve does not point to a valid file on the build host. This
shouldn't happen, but it can.

The end result of this situation would be an endless loop of error
messages that would only end if aborted manually.

    [...]
    cp: missing destination file operand after
    '/local/users/mmayer/buildroot/output/arm64/target//'
    Try 'cp --help' for more information.
    readlink: missing operand
    Try 'readlink --help' for more information.
    basename: missing operand
    Try 'basename --help' for more information.
    dirname: missing operand
    Try 'dirname --help' for more information.
    ^C
    make[1]: *** [package/pkg-generic.mk:384:

Instead of looping endlessly without explanation, let's abort and
inform the user that something seems amiss with their setup.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 toolchain/helpers.mk | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
index 05aa47a0dd8b..788f5ef671ce 100644
--- a/toolchain/helpers.mk
+++ b/toolchain/helpers.mk
@@ -19,7 +19,12 @@ copy_toolchain_lib_root = \
 			rm -fr $(TARGET_DIR)/$${DESTDIR}/$${LIBNAME}; \
 			if test -h $${LIBPATH} ; then \
 				cp -d $${LIBPATH} $(TARGET_DIR)/$${DESTDIR}/$${LIBNAME}; \
+				OLD_LIBPATH="$${LIBPATH}"; \
 				LIBPATH="`readlink -f $${LIBPATH}`"; \
+				if [ "$${LIBPATH}" = "" ]; then \
+					echo "LIBPATH empty after trying to resolve symlink $${OLD_LIBPATH}" 1>&2; \
+					exit 1; \
+				fi; \
 			elif test -f $${LIBPATH}; then \
 				$(INSTALL) -D -m0755 $${LIBPATH} $(TARGET_DIR)/$${DESTDIR}/$${LIBNAME}; \
 				break ; \
-- 
2.25.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] toolchain-external: handle case of dangling symlink

[Romain Naour]

Hello Markus,

Le 25/06/2022 à 00:19, Markus Mayer via buildroot a écrit :
> copy_toolchain_lib_root was not handling the case of "readlink"
> returning nothing, which will happen if the symlink it is trying to
> resolve does not point to a valid file on the build host. This
> shouldn't happen, but it can.

Indeed.

It could happen if the user provide such symlink in BR2_TOOLCHAIN_EXTRA_LIBS.

> 
> The end result of this situation would be an endless loop of error
> messages that would only end if aborted manually.
> 
>     [...]
>     cp: missing destination file operand after
>     '/local/users/mmayer/buildroot/output/arm64/target//'
>     Try 'cp --help' for more information.
>     readlink: missing operand
>     Try 'readlink --help' for more information.
>     basename: missing operand
>     Try 'basename --help' for more information.
>     dirname: missing operand
>     Try 'dirname --help' for more information.
>     ^C
>     make[1]: *** [package/pkg-generic.mk:384:
> 
> Instead of looping endlessly without explanation, let's abort and
> inform the user that something seems amiss with their setup.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  toolchain/helpers.mk | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> index 05aa47a0dd8b..788f5ef671ce 100644
> --- a/toolchain/helpers.mk
> +++ b/toolchain/helpers.mk
> @@ -19,7 +19,12 @@ copy_toolchain_lib_root = \
>  			rm -fr $(TARGET_DIR)/$${DESTDIR}/$${LIBNAME}; \
>  			if test -h $${LIBPATH} ; then \
>  				cp -d $${LIBPATH} $(TARGET_DIR)/$${DESTDIR}/$${LIBNAME}; \
> +				OLD_LIBPATH="$${LIBPATH}"; \
>  				LIBPATH="`readlink -f $${LIBPATH}`"; \
> +				if [ "$${LIBPATH}" = "" ]; then \
> +					echo "LIBPATH empty after trying to resolve symlink $${OLD_LIBPATH}" 1>&2; \
> +					exit 1; \
> +				fi; \

I'm not sure it's ok to fail here on a dangling symlink.
Maybe add a check and ignore such dangling symlink with a warning for the user.

It's unlikely that such dangling symlink provided by the toolchain will be useful.

Best regards,
Romain


>  			elif test -f $${LIBPATH}; then \
>  				$(INSTALL) -D -m0755 $${LIBPATH} $(TARGET_DIR)/$${DESTDIR}/$${LIBNAME}; \
>  				break ; \

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] toolchain-external: handle case of dangling symlink

[Thomas Petazzoni via buildroot]

Hello Romain,

On Wed, 11 Jan 2023 15:47:50 +0100
Romain Naour <romain.naour@smile.fr> wrote:

> > +				if [ "$${LIBPATH}" = "" ]; then \
> > +					echo "LIBPATH empty after trying to resolve symlink $${OLD_LIBPATH}" 1>&2; \
> > +					exit 1; \
> > +				fi; \  
> 
> I'm not sure it's ok to fail here on a dangling symlink.
> Maybe add a check and ignore such dangling symlink with a warning for the user.
> 
> It's unlikely that such dangling symlink provided by the toolchain will be useful.

I hesitated a bit on this. But for now, I believe a toolchain with a
dangling symlink is badly broken, and we should not work with it. A
warning message with a build continuing will never be noticed by the
user, so if we don't abort with hard error, nobody will notice. So I
applied the patch of Markus as is for now.

We will see if people complain that this breaks with their toolchain,
and if it does, we will change the exit to a break to simply move on to
handle the next library.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] toolchain-external: handle case of dangling symlink

[Thomas Petazzoni via buildroot]

On Fri, 24 Jun 2022 15:19:01 -0700
Markus Mayer via buildroot <buildroot@buildroot.org> wrote:

> copy_toolchain_lib_root was not handling the case of "readlink"
> returning nothing, which will happen if the symlink it is trying to
> resolve does not point to a valid file on the build host. This
> shouldn't happen, but it can.
> 
> The end result of this situation would be an endless loop of error
> messages that would only end if aborted manually.
> 
>     [...]
>     cp: missing destination file operand after
>     '/local/users/mmayer/buildroot/output/arm64/target//'
>     Try 'cp --help' for more information.
>     readlink: missing operand
>     Try 'readlink --help' for more information.
>     basename: missing operand
>     Try 'basename --help' for more information.
>     dirname: missing operand
>     Try 'dirname --help' for more information.
>     ^C
>     make[1]: *** [package/pkg-generic.mk:384:
> 
> Instead of looping endlessly without explanation, let's abort and
> inform the user that something seems amiss with their setup.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  toolchain/helpers.mk | 5 +++++
>  1 file changed, 5 insertions(+)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot