All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] toolchain-external: handle case of dangling symlink
@ 2022-06-24 22:19 Markus Mayer via buildroot
  2023-01-11 14:47 ` Romain Naour
  2023-02-07  9:21 ` Thomas Petazzoni via buildroot
  0 siblings, 2 replies; 4+ messages in thread
From: Markus Mayer via buildroot @ 2022-06-24 22:19 UTC (permalink / raw
  To: Buildroot Mailing List; +Cc: Markus Mayer

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Buildroot] [PATCH] toolchain-external: handle case of dangling symlink
  2022-06-24 22:19 [Buildroot] [PATCH] toolchain-external: handle case of dangling symlink Markus Mayer via buildroot
@ 2023-01-11 14:47 ` Romain Naour
  2023-02-07  9:23   ` Thomas Petazzoni via buildroot
  2023-02-07  9:21 ` Thomas Petazzoni via buildroot
  1 sibling, 1 reply; 4+ messages in thread
From: Romain Naour @ 2023-01-11 14:47 UTC (permalink / raw
  To: Markus Mayer, Buildroot Mailing List

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Buildroot] [PATCH] toolchain-external: handle case of dangling symlink
  2022-06-24 22:19 [Buildroot] [PATCH] toolchain-external: handle case of dangling symlink Markus Mayer via buildroot
  2023-01-11 14:47 ` Romain Naour
@ 2023-02-07  9:21 ` Thomas Petazzoni via buildroot
  1 sibling, 0 replies; 4+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-02-07  9:21 UTC (permalink / raw
  To: Markus Mayer via buildroot; +Cc: Markus Mayer

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Buildroot] [PATCH] toolchain-external: handle case of dangling symlink
  2023-01-11 14:47 ` Romain Naour
@ 2023-02-07  9:23   ` Thomas Petazzoni via buildroot
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-02-07  9:23 UTC (permalink / raw
  To: Romain Naour; +Cc: Markus Mayer, Buildroot Mailing List

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-02-07  9:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-24 22:19 [Buildroot] [PATCH] toolchain-external: handle case of dangling symlink Markus Mayer via buildroot
2023-01-11 14:47 ` Romain Naour
2023-02-07  9:23   ` Thomas Petazzoni via buildroot
2023-02-07  9:21 ` Thomas Petazzoni via buildroot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.