| Message ID | 20220624221901.514921-1-mmayer@broadcom.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | toolchain-external: handle case of dangling symlink | expand |
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 ; \
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
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
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 ; \
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(+)