[34/36] busybox: drop 0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch

Message ID 20211117153525.257146-34-alex@linutronix.de
State Accepted, archived
Commit e4831586ee03f189f7cf90aa444f7dc71544d3ec
Headers show
Series [01/36] ovmf: submit patch upstream | expand

Commit Message

Alexander Kanavin Nov. 17, 2021, 3:35 p.m. UTC
This was added 10 years ago, is almost certainly non-upstreamable
and it isn't clear what the issues it aims to fix are:
the AB revealed no problems when the patch is removed.

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 ...ing-instead-of-LD-and-use-CFLAGS-and.patch | 114 ------------------
 meta/recipes-core/busybox/busybox_1.34.1.bb   |   1 -
 2 files changed, 115 deletions(-)
 delete mode 100644 meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch

Comments

Andre McCurdy Nov. 17, 2021, 8:43 p.m. UTC | #1
On Wed, Nov 17, 2021 at 7:36 AM Alexander Kanavin
<alex.kanavin@gmail.com> wrote:
>
> This was added 10 years ago, is almost certainly non-upstreamable
> and it isn't clear what the issues it aims to fix are:
> the AB revealed no problems when the patch is removed.

The comment in the patch was: This fixes the issue where LDFLAGS
escaped with -Wl are ignored during compilation. Seems fairly clear!

Checking that the AB run didn't fail is probably not enough to justify
removing this patch. You should also confirm that LDFLAGS escaped with
-Wl (e.g. -Wl,-z,relro,-z,now when security flags are enabled, etc)
are still passed to the linker.

> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> ---
>  ...ing-instead-of-LD-and-use-CFLAGS-and.patch | 114 ------------------
>  meta/recipes-core/busybox/busybox_1.34.1.bb   |   1 -
>  2 files changed, 115 deletions(-)
>  delete mode 100644 meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
>
> diff --git a/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch b/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
> deleted file mode 100644
> index 2bf2b91c7e..0000000000
> --- a/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
> +++ /dev/null
> @@ -1,114 +0,0 @@
> -From a9333eb6a7b8dbda735947cd5bc981ff9352a2c9 Mon Sep 17 00:00:00 2001
> -From: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
> -Date: Thu, 10 Mar 2011 00:27:08 -0500
> -Subject: [PATCH 1/2] Use $(CC) when linking instead of $(LD) and use $(CFLAGS)
> - and $(EXTRA_CFLAGS) when linking.
> -
> -This fixes the issue where LDFLAGS escaped with -Wl are ignored during
> -compilation. It also simplifies using CFLAGS or EXTRA_CFLAGS (such as
> --m32 on x86_64 or -flto) which apply to both compilation and linking
> -situations.
> -
> -Signed-off-by: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
> ----
> -Upstream-Status: Pending
> -
> - Makefile               |  7 ++++---
> - scripts/Makefile.build |  8 ++++----
> - scripts/Makefile.lib   | 13 +++----------
> - 3 files changed, 11 insertions(+), 17 deletions(-)
> -
> -Index: busybox-1.23.2/Makefile
> -===================================================================
> ---- busybox-1.23.2.orig/Makefile
> -+++ busybox-1.23.2/Makefile
> -@@ -309,7 +309,8 @@ CHECKFLAGS     := -D__linux__ -Dlinux -D
> - MODFLAGS      = -DMODULE
> - CFLAGS_MODULE   = $(MODFLAGS)
> - AFLAGS_MODULE   = $(MODFLAGS)
> --LDFLAGS_MODULE  = -r
> -+LDFLAGS_RELOCATABLE = -r -nostdlib
> -+LDFLAGS_MODULE  = $(LDFLAGS_RELOCATABLE)
> - CFLAGS_KERNEL =
> - AFLAGS_KERNEL =
> -
> -@@ -331,7 +332,7 @@ KERNELVERSION = $(VERSION).$(PATCHLEVEL)
> - export        VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION \
> -       ARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC \
> -       CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE \
> --      HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
> -+      HOSTCXX HOSTCXXFLAGS LDFLAGS_RELOCATABLE LDFLAGS_MODULE CHECK CHECKFLAGS
> -
> - export CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
> - export CFLAGS CFLAGS_KERNEL CFLAGS_MODULE
> -@@ -610,7 +611,7 @@ quiet_cmd_busybox__ ?= LINK    $@
> -       cmd_busybox__ ?= $(srctree)/scripts/trylink \
> -       "$@" \
> -       "$(CC)" \
> --      "$(CFLAGS) $(CFLAGS_busybox)" \
> -+      "$(CFLAGS) $(CFLAGS_busybox) $(EXTRA_CFLAGS)" \
> -       "$(LDFLAGS) $(EXTRA_LDFLAGS)" \
> -       "$(core-y)" \
> -       "$(libs-y)" \
> -Index: busybox-1.23.2/scripts/Makefile.build
> -===================================================================
> ---- busybox-1.23.2.orig/scripts/Makefile.build
> -+++ busybox-1.23.2/scripts/Makefile.build
> -@@ -174,7 +174,7 @@ cmd_modversions =                                                  \
> -               | $(GENKSYMS) -a $(ARCH)                                \
> -               > $(@D)/.tmp_$(@F:.o=.ver);                             \
> -                                                                       \
> --              $(LD) $(LDFLAGS) -r -o $@ $(@D)/.tmp_$(@F)              \
> -+               $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@ $(@D)/.tmp_$(@F)        \
> -                       -T $(@D)/.tmp_$(@F:.o=.ver);                    \
> -               rm -f $(@D)/.tmp_$(@F) $(@D)/.tmp_$(@F:.o=.ver);        \
> -       else                                                            \
> -@@ -257,7 +257,7 @@ quiet_cmd_link_o_target = LD      $@
> - # If the list of objects to link is empty, just create an empty built-in.o
> - # -nostdlib is added to make "make LD=gcc ..." work (some people use that)
> - cmd_link_o_target = $(if $(strip $(obj-y)),\
> --              $(LD) -nostdlib $(ld_flags) -r -o $@ $(filter $(obj-y), $^),\
> -+              $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@ $(filter $(obj-y), $^),\
> -               rm -f $@; $(AR) rcs $@)
> -
> - $(builtin-target): $(obj-y) FORCE
> -@@ -292,10 +292,10 @@ $($(subst $(obj)/,,$(@:.o=-objs)))    \
> - $($(subst $(obj)/,,$(@:.o=-y)))), $^)
> -
> - quiet_cmd_link_multi-y = LD      $@
> --cmd_link_multi-y = $(LD) $(ld_flags) -r -o $@ $(link_multi_deps)
> -+cmd_link_multi-y = $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@ $(link_multi_deps)
> -
> - quiet_cmd_link_multi-m = LD [M]  $@
> --cmd_link_multi-m = $(LD) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
> -+cmd_link_multi-m = $(CC) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
> -
> - # We would rather have a list of rules like
> - #     foo.o: $(foo-objs)
> -Index: busybox-1.23.2/scripts/Makefile.lib
> -===================================================================
> ---- busybox-1.23.2.orig/scripts/Makefile.lib
> -+++ busybox-1.23.2/scripts/Makefile.lib
> -@@ -121,7 +121,8 @@ cpp_flags      = -Wp,-MD,$(depfile) $(NO
> - # yet ld_flags is fed to ld.
> - #ld_flags       = $(LDFLAGS) $(EXTRA_LDFLAGS)
> - # Remove the -Wl, prefix from linker options normally passed through gcc
> --ld_flags       = $(filter-out -Wl$(comma)%,$(LDFLAGS) $(EXTRA_LDFLAGS))
> -+ld_flags       = $(filter-out -Wl$(comma)%,$(LDFLAGS) $(EXTRA_LDFLAGS) $(CFLAGS) $(EXTRA_CFLAGS))
> -+ld_flags_partial = $($(filter-out -shared%, $(filter-out -pie%,$(ld_flags))))
> -
> -
> - # Finds the multi-part object the current object will be linked into
> -@@ -151,10 +152,8 @@ $(obj)/%:: $(src)/%_shipped
> - # Linking
> - # ---------------------------------------------------------------------------
> -
> --# TODO: LDFLAGS usually is supposed to contain gcc's flags, not ld's.
> --# but here we feed them to ld!
> --quiet_cmd_ld = LD      $@
> --cmd_ld = $(LD) $(LDFLAGS) $(EXTRA_LDFLAGS) $(LDFLAGS_$(@F)) \
> -+quiet_cmd_ld = CC    $@
> -+cmd_ld = $(CC) $(ld_flags) $(LDFLAGS_$(@F)) \
> -              $(filter-out FORCE,$^) -o $@
> -
> - # Objcopy
> diff --git a/meta/recipes-core/busybox/busybox_1.34.1.bb b/meta/recipes-core/busybox/busybox_1.34.1.bb
> index 6aed0f0476..3651c06126 100644
> --- a/meta/recipes-core/busybox/busybox_1.34.1.bb
> +++ b/meta/recipes-core/busybox/busybox_1.34.1.bb
> @@ -26,7 +26,6 @@ SRC_URI = "https://busybox.net/downloads/busybox-${PV}.tar.bz2;name=tarball \
>             file://login-utilities.cfg \
>             file://recognize_connmand.patch \
>             file://busybox-cross-menuconfig.patch \
> -           file://0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch \
>             file://mount-via-label.cfg \
>             file://sha1sum.cfg \
>             file://sha256sum.cfg \
> --
> 2.20.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#158429): https://lists.openembedded.org/g/openembedded-core/message/158429
> Mute This Topic: https://lists.openembedded.org/mt/87122011/3619030
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [armccurdy@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Alexander Kanavin Nov. 17, 2021, 10:05 p.m. UTC | #2
On Wed, 17 Nov 2021 at 21:43, Andre McCurdy <armccurdy@gmail.com> wrote:

> On Wed, Nov 17, 2021 at 7:36 AM Alexander Kanavin
> <alex.kanavin@gmail.com> wrote:
> >
> > This was added 10 years ago, is almost certainly non-upstreamable
> > and it isn't clear what the issues it aims to fix are:
> > the AB revealed no problems when the patch is removed.
>
> The comment in the patch was: This fixes the issue where LDFLAGS
> escaped with -Wl are ignored during compilation. Seems fairly clear!
>
> Checking that the AB run didn't fail is probably not enough to justify
> removing this patch. You should also confirm that LDFLAGS escaped with
> -Wl (e.g. -Wl,-z,relro,-z,now when security flags are enabled, etc)
> are still passed to the linker.
>

Thanks, I will double check.

Alex


>
> > Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> > ---
> >  ...ing-instead-of-LD-and-use-CFLAGS-and.patch | 114 ------------------
> >  meta/recipes-core/busybox/busybox_1.34.1.bb   |   1 -
> >  2 files changed, 115 deletions(-)
> >  delete mode 100644
> meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
> >
> > diff --git
> a/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
> b/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
> > deleted file mode 100644
> > index 2bf2b91c7e..0000000000
> > ---
> a/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
> > +++ /dev/null
> > @@ -1,114 +0,0 @@
> > -From a9333eb6a7b8dbda735947cd5bc981ff9352a2c9 Mon Sep 17 00:00:00 2001
> > -From: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
> > -Date: Thu, 10 Mar 2011 00:27:08 -0500
> > -Subject: [PATCH 1/2] Use $(CC) when linking instead of $(LD) and use
> $(CFLAGS)
> > - and $(EXTRA_CFLAGS) when linking.
> > -
> > -This fixes the issue where LDFLAGS escaped with -Wl are ignored during
> > -compilation. It also simplifies using CFLAGS or EXTRA_CFLAGS (such as
> > --m32 on x86_64 or -flto) which apply to both compilation and linking
> > -situations.
> > -
> > -Signed-off-by: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
> > ----
> > -Upstream-Status: Pending
> > -
> > - Makefile               |  7 ++++---
> > - scripts/Makefile.build |  8 ++++----
> > - scripts/Makefile.lib   | 13 +++----------
> > - 3 files changed, 11 insertions(+), 17 deletions(-)
> > -
> > -Index: busybox-1.23.2/Makefile
> > -===================================================================
> > ---- busybox-1.23.2.orig/Makefile
> > -+++ busybox-1.23.2/Makefile
> > -@@ -309,7 +309,8 @@ CHECKFLAGS     := -D__linux__ -Dlinux -D
> > - MODFLAGS      = -DMODULE
> > - CFLAGS_MODULE   = $(MODFLAGS)
> > - AFLAGS_MODULE   = $(MODFLAGS)
> > --LDFLAGS_MODULE  = -r
> > -+LDFLAGS_RELOCATABLE = -r -nostdlib
> > -+LDFLAGS_MODULE  = $(LDFLAGS_RELOCATABLE)
> > - CFLAGS_KERNEL =
> > - AFLAGS_KERNEL =
> > -
> > -@@ -331,7 +332,7 @@ KERNELVERSION = $(VERSION).$(PATCHLEVEL)
> > - export        VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION \
> > -       ARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC \
> > -       CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL
> UTS_MACHINE \
> > --      HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
> > -+      HOSTCXX HOSTCXXFLAGS LDFLAGS_RELOCATABLE LDFLAGS_MODULE CHECK
> CHECKFLAGS
> > -
> > - export CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
> > - export CFLAGS CFLAGS_KERNEL CFLAGS_MODULE
> > -@@ -610,7 +611,7 @@ quiet_cmd_busybox__ ?= LINK    $@
> > -       cmd_busybox__ ?= $(srctree)/scripts/trylink \
> > -       "$@" \
> > -       "$(CC)" \
> > --      "$(CFLAGS) $(CFLAGS_busybox)" \
> > -+      "$(CFLAGS) $(CFLAGS_busybox) $(EXTRA_CFLAGS)" \
> > -       "$(LDFLAGS) $(EXTRA_LDFLAGS)" \
> > -       "$(core-y)" \
> > -       "$(libs-y)" \
> > -Index: busybox-1.23.2/scripts/Makefile.build
> > -===================================================================
> > ---- busybox-1.23.2.orig/scripts/Makefile.build
> > -+++ busybox-1.23.2/scripts/Makefile.build
> > -@@ -174,7 +174,7 @@ cmd_modversions =
>                 \
> > -               | $(GENKSYMS) -a $(ARCH)                                \
> > -               > $(@D)/.tmp_$(@F:.o=.ver);                             \
> > -                                                                       \
> > --              $(LD) $(LDFLAGS) -r -o $@ $(@D)/.tmp_$(@F)              \
> > -+               $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@
> $(@D)/.tmp_$(@F)        \
> > -                       -T $(@D)/.tmp_$(@F:.o=.ver);                    \
> > -               rm -f $(@D)/.tmp_$(@F) $(@D)/.tmp_$(@F:.o=.ver);        \
> > -       else                                                            \
> > -@@ -257,7 +257,7 @@ quiet_cmd_link_o_target = LD      $@
> > - # If the list of objects to link is empty, just create an empty
> built-in.o
> > - # -nostdlib is added to make "make LD=gcc ..." work (some people use
> that)
> > - cmd_link_o_target = $(if $(strip $(obj-y)),\
> > --              $(LD) -nostdlib $(ld_flags) -r -o $@ $(filter $(obj-y),
> $^),\
> > -+              $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@
> $(filter $(obj-y), $^),\
> > -               rm -f $@; $(AR) rcs $@)
> > -
> > - $(builtin-target): $(obj-y) FORCE
> > -@@ -292,10 +292,10 @@ $($(subst $(obj)/,,$(@:.o=-objs)))    \
> > - $($(subst $(obj)/,,$(@:.o=-y)))), $^)
> > -
> > - quiet_cmd_link_multi-y = LD      $@
> > --cmd_link_multi-y = $(LD) $(ld_flags) -r -o $@ $(link_multi_deps)
> > -+cmd_link_multi-y = $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o
> $@ $(link_multi_deps)
> > -
> > - quiet_cmd_link_multi-m = LD [M]  $@
> > --cmd_link_multi-m = $(LD) $(ld_flags) $(LDFLAGS_MODULE) -o $@
> $(link_multi_deps)
> > -+cmd_link_multi-m = $(CC) $(ld_flags) $(LDFLAGS_MODULE) -o $@
> $(link_multi_deps)
> > -
> > - # We would rather have a list of rules like
> > - #     foo.o: $(foo-objs)
> > -Index: busybox-1.23.2/scripts/Makefile.lib
> > -===================================================================
> > ---- busybox-1.23.2.orig/scripts/Makefile.lib
> > -+++ busybox-1.23.2/scripts/Makefile.lib
> > -@@ -121,7 +121,8 @@ cpp_flags      = -Wp,-MD,$(depfile) $(NO
> > - # yet ld_flags is fed to ld.
> > - #ld_flags       = $(LDFLAGS) $(EXTRA_LDFLAGS)
> > - # Remove the -Wl, prefix from linker options normally passed through
> gcc
> > --ld_flags       = $(filter-out -Wl$(comma)%,$(LDFLAGS) $(EXTRA_LDFLAGS))
> > -+ld_flags       = $(filter-out -Wl$(comma)%,$(LDFLAGS) $(EXTRA_LDFLAGS)
> $(CFLAGS) $(EXTRA_CFLAGS))
> > -+ld_flags_partial = $($(filter-out -shared%, $(filter-out
> -pie%,$(ld_flags))))
> > -
> > -
> > - # Finds the multi-part object the current object will be linked into
> > -@@ -151,10 +152,8 @@ $(obj)/%:: $(src)/%_shipped
> > - # Linking
> > - #
> ---------------------------------------------------------------------------
> > -
> > --# TODO: LDFLAGS usually is supposed to contain gcc's flags, not ld's.
> > --# but here we feed them to ld!
> > --quiet_cmd_ld = LD      $@
> > --cmd_ld = $(LD) $(LDFLAGS) $(EXTRA_LDFLAGS) $(LDFLAGS_$(@F)) \
> > -+quiet_cmd_ld = CC    $@
> > -+cmd_ld = $(CC) $(ld_flags) $(LDFLAGS_$(@F)) \
> > -              $(filter-out FORCE,$^) -o $@
> > -
> > - # Objcopy
> > diff --git a/meta/recipes-core/busybox/busybox_1.34.1.bb
> b/meta/recipes-core/busybox/busybox_1.34.1.bb
> > index 6aed0f0476..3651c06126 100644
> > --- a/meta/recipes-core/busybox/busybox_1.34.1.bb
> > +++ b/meta/recipes-core/busybox/busybox_1.34.1.bb
> > @@ -26,7 +26,6 @@ SRC_URI = "
> https://busybox.net/downloads/busybox-${PV}.tar.bz2;name=tarball \
> >             file://login-utilities.cfg \
> >             file://recognize_connmand.patch \
> >             file://busybox-cross-menuconfig.patch \
> > -
>  file://0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch \
> >             file://mount-via-label.cfg \
> >             file://sha1sum.cfg \
> >             file://sha256sum.cfg \
> > --
> > 2.20.1
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#158429):
> https://lists.openembedded.org/g/openembedded-core/message/158429
> > Mute This Topic: https://lists.openembedded.org/mt/87122011/3619030
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> armccurdy@gmail.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
>
Alexander Kanavin Nov. 18, 2021, 10:32 a.m. UTC | #3
I double checked, with strace (can't trust the logs as they don't show the
actual linker invocation :)

execve("/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0/recipe-sysroot-native/usr/bin/x86_64-poky-linux/x86_64-poky-linux-gcc",
["x86_64-poky-linux-gcc", "-m64", "-march=core2", "-mtune=core2", "-msse3",
"-mfpmath=sse", "-fstack-protector-strong", "-O2", "-D_FORTIFY_SOURCE=2",
"--sysroot=/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0/recipe-sysroot",
"-malign-data=abi", "-Wall", "-Wshadow", "-Wwrite-strings", "-Wundef",
"-Wstrict-prototypes", "-Wunused", "-Wunused-parameter",
"-Wunused-function", "-Wunused-value", "-Wmissing-prototypes",
"-Wmissing-declarations", "-Wno-format-security",
"-Wdeclaration-after-statement", "-Wold-style-definition",
"-finline-limit=0", "-fno-builtin-strlen", "-fomit-frame-pointer",
"-ffunction-sections", "-fdata-sections", "-fno-guess-branch-probability",
"-funsigned-char", "-static-libgcc", "-falign-functions=1",
"-falign-jumps=1", "-falign-labels=1", "-falign-loops=1",
"-fno-unwind-tables", "-fno-asynchronous-unwind-tables",
"-fno-builtin-printf", "-Os", "-O2", "-pipe", "-g",
"-feliminate-unused-debug-types", "-m64", "-march=core2", "-mtune=core2",
"-msse3", "-mfpmath=sse", "-fstack-protector-strong", "-O2",
"-D_FORTIFY_SOURCE=2", "-Wl,-O1", "-Wl,--hash-style=gnu",
"-Wl,--as-needed",
"-fmacro-prefix-map=/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0=/usr/src/debug/busybox/1.34.1-r0",
"-fdebug-prefix-map=/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0=/usr/src/debug/busybox/1.34.1-r0",
"-fdebug-prefix-map=/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0/recipe-sysroot=",
"-fdebug-prefix-map=/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0/recipe-sysroot-native=",
"-Wl,-z,relro,-z,now", "-o", "busybox_unstripped", "-Wl,--sort-common",
"-Wl,--sort-section,alignment", "-Wl,--gc-sections", "-Wl,--start-group",
"applets/built-in.o", "archival/lib.a", "archival/libarchive/lib.a",
"console-tools/lib.a", "coreutils/lib.a", "coreutils/libcoreutils/lib.a",
"debianutils/lib.a", "klibc-utils/lib.a", "e2fsprogs/lib.a",
"editors/lib.a", "findutils/lib.a", "init/lib.a", "libbb/lib.a",
"libpwdgrp/lib.a", "loginutils/lib.a", "mailutils/lib.a",
"miscutils/lib.a", "modutils/lib.a", "networking/lib.a",
"networking/libiproute/lib.a", "networking/udhcp/lib.a",
"printutils/lib.a", "procps/lib.a", "runit/lib.a", "selinux/lib.a",
"shell/lib.a", "sysklogd/lib.a", "util-linux/lib.a",
"util-linux/volume_id/lib.a", "archival/built-in.o",
"archival/libarchive/built-in.o", "console-tools/built-in.o",
"coreutils/built-in.o", "coreutils/libcoreutils/built-in.o",
"debianutils/built-in.o", "klibc-utils/built-in.o", "e2fsprogs/built-in.o",
"editors/built-in.o", "findutils/built-in.o", "init/built-in.o",
"libbb/built-in.o", "libpwdgrp/built-in.o", "loginutils/built-in.o",
"mailutils/built-in.o", "miscutils/built-in.o", "modutils/built-in.o",
"networking/built-in.o", "networking/libiproute/built-in.o",
"networking/udhcp/built-in.o", "printutils/built-in.o",
"procps/built-in.o", "runit/built-in.o", "selinux/built-in.o",
"shell/built-in.o", "sysklogd/built-in.o", "util-linux/built-in.o",
"util-linux/volume_id/built-in.o", "-Wl,--end-group", "-Wl,--start-group",
"-lcrypt", "-lm", "-lrt", "-Wl,--end-group"], 0x5654941e7a98 /* 138 vars
*/) = 0

So everything that is supposed to be there is actually present, and the
patch is indeed unneeded.

Alex

On Wed, 17 Nov 2021 at 23:05, Alexander Kanavin via lists.openembedded.org
<alex.kanavin=gmail.com@lists.openembedded.org> wrote:

> On Wed, 17 Nov 2021 at 21:43, Andre McCurdy <armccurdy@gmail.com> wrote:
>
>> On Wed, Nov 17, 2021 at 7:36 AM Alexander Kanavin
>> <alex.kanavin@gmail.com> wrote:
>> >
>> > This was added 10 years ago, is almost certainly non-upstreamable
>> > and it isn't clear what the issues it aims to fix are:
>> > the AB revealed no problems when the patch is removed.
>>
>> The comment in the patch was: This fixes the issue where LDFLAGS
>> escaped with -Wl are ignored during compilation. Seems fairly clear!
>>
>> Checking that the AB run didn't fail is probably not enough to justify
>> removing this patch. You should also confirm that LDFLAGS escaped with
>> -Wl (e.g. -Wl,-z,relro,-z,now when security flags are enabled, etc)
>> are still passed to the linker.
>>
>
> Thanks, I will double check.
>
> Alex
>
>
>>
>> > Signed-off-by: Alexander Kanavin <alex@linutronix.de>
>> > ---
>> >  ...ing-instead-of-LD-and-use-CFLAGS-and.patch | 114 ------------------
>> >  meta/recipes-core/busybox/busybox_1.34.1.bb   |   1 -
>> >  2 files changed, 115 deletions(-)
>> >  delete mode 100644
>> meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
>> >
>> > diff --git
>> a/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
>> b/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
>> > deleted file mode 100644
>> > index 2bf2b91c7e..0000000000
>> > ---
>> a/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
>> > +++ /dev/null
>> > @@ -1,114 +0,0 @@
>> > -From a9333eb6a7b8dbda735947cd5bc981ff9352a2c9 Mon Sep 17 00:00:00 2001
>> > -From: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
>> > -Date: Thu, 10 Mar 2011 00:27:08 -0500
>> > -Subject: [PATCH 1/2] Use $(CC) when linking instead of $(LD) and use
>> $(CFLAGS)
>> > - and $(EXTRA_CFLAGS) when linking.
>> > -
>> > -This fixes the issue where LDFLAGS escaped with -Wl are ignored during
>> > -compilation. It also simplifies using CFLAGS or EXTRA_CFLAGS (such as
>> > --m32 on x86_64 or -flto) which apply to both compilation and linking
>> > -situations.
>> > -
>> > -Signed-off-by: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
>> > ----
>> > -Upstream-Status: Pending
>> > -
>> > - Makefile               |  7 ++++---
>> > - scripts/Makefile.build |  8 ++++----
>> > - scripts/Makefile.lib   | 13 +++----------
>> > - 3 files changed, 11 insertions(+), 17 deletions(-)
>> > -
>> > -Index: busybox-1.23.2/Makefile
>> > -===================================================================
>> > ---- busybox-1.23.2.orig/Makefile
>> > -+++ busybox-1.23.2/Makefile
>> > -@@ -309,7 +309,8 @@ CHECKFLAGS     := -D__linux__ -Dlinux -D
>> > - MODFLAGS      = -DMODULE
>> > - CFLAGS_MODULE   = $(MODFLAGS)
>> > - AFLAGS_MODULE   = $(MODFLAGS)
>> > --LDFLAGS_MODULE  = -r
>> > -+LDFLAGS_RELOCATABLE = -r -nostdlib
>> > -+LDFLAGS_MODULE  = $(LDFLAGS_RELOCATABLE)
>> > - CFLAGS_KERNEL =
>> > - AFLAGS_KERNEL =
>> > -
>> > -@@ -331,7 +332,7 @@ KERNELVERSION = $(VERSION).$(PATCHLEVEL)
>> > - export        VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
>> \
>> > -       ARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC \
>> > -       CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL
>> UTS_MACHINE \
>> > --      HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
>> > -+      HOSTCXX HOSTCXXFLAGS LDFLAGS_RELOCATABLE LDFLAGS_MODULE CHECK
>> CHECKFLAGS
>> > -
>> > - export CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
>> > - export CFLAGS CFLAGS_KERNEL CFLAGS_MODULE
>> > -@@ -610,7 +611,7 @@ quiet_cmd_busybox__ ?= LINK    $@
>> > -       cmd_busybox__ ?= $(srctree)/scripts/trylink \
>> > -       "$@" \
>> > -       "$(CC)" \
>> > --      "$(CFLAGS) $(CFLAGS_busybox)" \
>> > -+      "$(CFLAGS) $(CFLAGS_busybox) $(EXTRA_CFLAGS)" \
>> > -       "$(LDFLAGS) $(EXTRA_LDFLAGS)" \
>> > -       "$(core-y)" \
>> > -       "$(libs-y)" \
>> > -Index: busybox-1.23.2/scripts/Makefile.build
>> > -===================================================================
>> > ---- busybox-1.23.2.orig/scripts/Makefile.build
>> > -+++ busybox-1.23.2/scripts/Makefile.build
>> > -@@ -174,7 +174,7 @@ cmd_modversions =
>>                 \
>> > -               | $(GENKSYMS) -a $(ARCH)
>> \
>> > -               > $(@D)/.tmp_$(@F:.o=.ver);
>>  \
>> > -
>>  \
>> > --              $(LD) $(LDFLAGS) -r -o $@ $(@D)/.tmp_$(@F)
>> \
>> > -+               $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@
>> $(@D)/.tmp_$(@F)        \
>> > -                       -T $(@D)/.tmp_$(@F:.o=.ver);
>> \
>> > -               rm -f $(@D)/.tmp_$(@F) $(@D)/.tmp_$(@F:.o=.ver);
>> \
>> > -       else
>> \
>> > -@@ -257,7 +257,7 @@ quiet_cmd_link_o_target = LD      $@
>> > - # If the list of objects to link is empty, just create an empty
>> built-in.o
>> > - # -nostdlib is added to make "make LD=gcc ..." work (some people use
>> that)
>> > - cmd_link_o_target = $(if $(strip $(obj-y)),\
>> > --              $(LD) -nostdlib $(ld_flags) -r -o $@ $(filter $(obj-y),
>> $^),\
>> > -+              $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@
>> $(filter $(obj-y), $^),\
>> > -               rm -f $@; $(AR) rcs $@)
>> > -
>> > - $(builtin-target): $(obj-y) FORCE
>> > -@@ -292,10 +292,10 @@ $($(subst $(obj)/,,$(@:.o=-objs)))    \
>> > - $($(subst $(obj)/,,$(@:.o=-y)))), $^)
>> > -
>> > - quiet_cmd_link_multi-y = LD      $@
>> > --cmd_link_multi-y = $(LD) $(ld_flags) -r -o $@ $(link_multi_deps)
>> > -+cmd_link_multi-y = $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE)
>> -o $@ $(link_multi_deps)
>> > -
>> > - quiet_cmd_link_multi-m = LD [M]  $@
>> > --cmd_link_multi-m = $(LD) $(ld_flags) $(LDFLAGS_MODULE) -o $@
>> $(link_multi_deps)
>> > -+cmd_link_multi-m = $(CC) $(ld_flags) $(LDFLAGS_MODULE) -o $@
>> $(link_multi_deps)
>> > -
>> > - # We would rather have a list of rules like
>> > - #     foo.o: $(foo-objs)
>> > -Index: busybox-1.23.2/scripts/Makefile.lib
>> > -===================================================================
>> > ---- busybox-1.23.2.orig/scripts/Makefile.lib
>> > -+++ busybox-1.23.2/scripts/Makefile.lib
>> > -@@ -121,7 +121,8 @@ cpp_flags      = -Wp,-MD,$(depfile) $(NO
>> > - # yet ld_flags is fed to ld.
>> > - #ld_flags       = $(LDFLAGS) $(EXTRA_LDFLAGS)
>> > - # Remove the -Wl, prefix from linker options normally passed through
>> gcc
>> > --ld_flags       = $(filter-out -Wl$(comma)%,$(LDFLAGS)
>> $(EXTRA_LDFLAGS))
>> > -+ld_flags       = $(filter-out -Wl$(comma)%,$(LDFLAGS)
>> $(EXTRA_LDFLAGS) $(CFLAGS) $(EXTRA_CFLAGS))
>> > -+ld_flags_partial = $($(filter-out -shared%, $(filter-out
>> -pie%,$(ld_flags))))
>> > -
>> > -
>> > - # Finds the multi-part object the current object will be linked into
>> > -@@ -151,10 +152,8 @@ $(obj)/%:: $(src)/%_shipped
>> > - # Linking
>> > - #
>> ---------------------------------------------------------------------------
>> > -
>> > --# TODO: LDFLAGS usually is supposed to contain gcc's flags, not ld's.
>> > --# but here we feed them to ld!
>> > --quiet_cmd_ld = LD      $@
>> > --cmd_ld = $(LD) $(LDFLAGS) $(EXTRA_LDFLAGS) $(LDFLAGS_$(@F)) \
>> > -+quiet_cmd_ld = CC    $@
>> > -+cmd_ld = $(CC) $(ld_flags) $(LDFLAGS_$(@F)) \
>> > -              $(filter-out FORCE,$^) -o $@
>> > -
>> > - # Objcopy
>> > diff --git a/meta/recipes-core/busybox/busybox_1.34.1.bb
>> b/meta/recipes-core/busybox/busybox_1.34.1.bb
>> > index 6aed0f0476..3651c06126 100644
>> > --- a/meta/recipes-core/busybox/busybox_1.34.1.bb
>> > +++ b/meta/recipes-core/busybox/busybox_1.34.1.bb
>> > @@ -26,7 +26,6 @@ SRC_URI = "
>> https://busybox.net/downloads/busybox-${PV}.tar.bz2;name=tarball \
>> >             file://login-utilities.cfg \
>> >             file://recognize_connmand.patch \
>> >             file://busybox-cross-menuconfig.patch \
>> > -
>>  file://0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch \
>> >             file://mount-via-label.cfg \
>> >             file://sha1sum.cfg \
>> >             file://sha256sum.cfg \
>> > --
>> > 2.20.1
>> >
>> >
>> >
>> >
>>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#158461):
> https://lists.openembedded.org/g/openembedded-core/message/158461
> Mute This Topic: https://lists.openembedded.org/mt/87122011/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Andre McCurdy Nov. 18, 2021, 11:14 p.m. UTC | #4
On Thu, Nov 18, 2021 at 2:32 AM Alexander Kanavin
<alex.kanavin@gmail.com> wrote:
>
> I double checked, with strace (can't trust the logs as they don't show the actual linker invocation :)
>
> execve("/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0/recipe-sysroot-native/usr/bin/x86_64-poky-linux/x86_64-poky-linux-gcc", ["x86_64-poky-linux-gcc", "-m64", "-march=core2", "-mtune=core2", "-msse3", "-mfpmath=sse", "-fstack-protector-strong", "-O2", "-D_FORTIFY_SOURCE=2", "--sysroot=/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0/recipe-sysroot", "-malign-data=abi", "-Wall", "-Wshadow", "-Wwrite-strings", "-Wundef", "-Wstrict-prototypes", "-Wunused", "-Wunused-parameter", "-Wunused-function", "-Wunused-value", "-Wmissing-prototypes", "-Wmissing-declarations", "-Wno-format-security", "-Wdeclaration-after-statement", "-Wold-style-definition", "-finline-limit=0", "-fno-builtin-strlen", "-fomit-frame-pointer", "-ffunction-sections", "-fdata-sections", "-fno-guess-branch-probability", "-funsigned-char", "-static-libgcc", "-falign-functions=1", "-falign-jumps=1", "-falign-labels=1", "-falign-loops=1", "-fno-unwind-tables", "-fno-asynchronous-unwind-tables", "-fno-builtin-printf", "-Os", "-O2", "-pipe", "-g", "-feliminate-unused-debug-types", "-m64", "-march=core2", "-mtune=core2", "-msse3", "-mfpmath=sse", "-fstack-protector-strong", "-O2", "-D_FORTIFY_SOURCE=2", "-Wl,-O1", "-Wl,--hash-style=gnu", "-Wl,--as-needed", "-fmacro-prefix-map=/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0=/usr/src/debug/busybox/1.34.1-r0", "-fdebug-prefix-map=/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0=/usr/src/debug/busybox/1.34.1-r0", "-fdebug-prefix-map=/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0/recipe-sysroot=", "-fdebug-prefix-map=/home/alex/development/poky/build-64-alt/tmp/work/core2-64-poky-linux/busybox/1.34.1-r0/recipe-sysroot-native=", "-Wl,-z,relro,-z,now", "-o", "busybox_unstripped", "-Wl,--sort-common", "-Wl,--sort-section,alignment", "-Wl,--gc-sections", "-Wl,--start-group", "applets/built-in.o", "archival/lib.a", "archival/libarchive/lib.a", "console-tools/lib.a", "coreutils/lib.a", "coreutils/libcoreutils/lib.a", "debianutils/lib.a", "klibc-utils/lib.a", "e2fsprogs/lib.a", "editors/lib.a", "findutils/lib.a", "init/lib.a", "libbb/lib.a", "libpwdgrp/lib.a", "loginutils/lib.a", "mailutils/lib.a", "miscutils/lib.a", "modutils/lib.a", "networking/lib.a", "networking/libiproute/lib.a", "networking/udhcp/lib.a", "printutils/lib.a", "procps/lib.a", "runit/lib.a", "selinux/lib.a", "shell/lib.a", "sysklogd/lib.a", "util-linux/lib.a", "util-linux/volume_id/lib.a", "archival/built-in.o", "archival/libarchive/built-in.o", "console-tools/built-in.o", "coreutils/built-in.o", "coreutils/libcoreutils/built-in.o", "debianutils/built-in.o", "klibc-utils/built-in.o", "e2fsprogs/built-in.o", "editors/built-in.o", "findutils/built-in.o", "init/built-in.o", "libbb/built-in.o", "libpwdgrp/built-in.o", "loginutils/built-in.o", "mailutils/built-in.o", "miscutils/built-in.o", "modutils/built-in.o", "networking/built-in.o", "networking/libiproute/built-in.o", "networking/udhcp/built-in.o", "printutils/built-in.o", "procps/built-in.o", "runit/built-in.o", "selinux/built-in.o", "shell/built-in.o", "sysklogd/built-in.o", "util-linux/built-in.o", "util-linux/volume_id/built-in.o", "-Wl,--end-group", "-Wl,--start-group", "-lcrypt", "-lm", "-lrt", "-Wl,--end-group"], 0x5654941e7a98 /* 138 vars */) = 0
>
> So everything that is supposed to be there is actually present, and the patch is indeed unneeded.

Looks good. If you have the full strace logs for both cases then it
might still be useful to check which (if any) command lines are
changed by removing the patch, but if not then removing the whole
patch does seem OK.

> Alex
>
> On Wed, 17 Nov 2021 at 23:05, Alexander Kanavin via lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org> wrote:
>>
>> On Wed, 17 Nov 2021 at 21:43, Andre McCurdy <armccurdy@gmail.com> wrote:
>>>
>>> On Wed, Nov 17, 2021 at 7:36 AM Alexander Kanavin
>>> <alex.kanavin@gmail.com> wrote:
>>> >
>>> > This was added 10 years ago, is almost certainly non-upstreamable
>>> > and it isn't clear what the issues it aims to fix are:
>>> > the AB revealed no problems when the patch is removed.
>>>
>>> The comment in the patch was: This fixes the issue where LDFLAGS
>>> escaped with -Wl are ignored during compilation. Seems fairly clear!
>>>
>>> Checking that the AB run didn't fail is probably not enough to justify
>>> removing this patch. You should also confirm that LDFLAGS escaped with
>>> -Wl (e.g. -Wl,-z,relro,-z,now when security flags are enabled, etc)
>>> are still passed to the linker.
>>
>>
>> Thanks, I will double check.
>>
>> Alex
>>
>>>
>>>
>>> > Signed-off-by: Alexander Kanavin <alex@linutronix.de>
>>> > ---
>>> >  ...ing-instead-of-LD-and-use-CFLAGS-and.patch | 114 ------------------
>>> >  meta/recipes-core/busybox/busybox_1.34.1.bb   |   1 -
>>> >  2 files changed, 115 deletions(-)
>>> >  delete mode 100644 meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
>>> >
>>> > diff --git a/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch b/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
>>> > deleted file mode 100644
>>> > index 2bf2b91c7e..0000000000
>>> > --- a/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
>>> > +++ /dev/null
>>> > @@ -1,114 +0,0 @@
>>> > -From a9333eb6a7b8dbda735947cd5bc981ff9352a2c9 Mon Sep 17 00:00:00 2001
>>> > -From: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
>>> > -Date: Thu, 10 Mar 2011 00:27:08 -0500
>>> > -Subject: [PATCH 1/2] Use $(CC) when linking instead of $(LD) and use $(CFLAGS)
>>> > - and $(EXTRA_CFLAGS) when linking.
>>> > -
>>> > -This fixes the issue where LDFLAGS escaped with -Wl are ignored during
>>> > -compilation. It also simplifies using CFLAGS or EXTRA_CFLAGS (such as
>>> > --m32 on x86_64 or -flto) which apply to both compilation and linking
>>> > -situations.
>>> > -
>>> > -Signed-off-by: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
>>> > ----
>>> > -Upstream-Status: Pending
>>> > -
>>> > - Makefile               |  7 ++++---
>>> > - scripts/Makefile.build |  8 ++++----
>>> > - scripts/Makefile.lib   | 13 +++----------
>>> > - 3 files changed, 11 insertions(+), 17 deletions(-)
>>> > -
>>> > -Index: busybox-1.23.2/Makefile
>>> > -===================================================================
>>> > ---- busybox-1.23.2.orig/Makefile
>>> > -+++ busybox-1.23.2/Makefile
>>> > -@@ -309,7 +309,8 @@ CHECKFLAGS     := -D__linux__ -Dlinux -D
>>> > - MODFLAGS      = -DMODULE
>>> > - CFLAGS_MODULE   = $(MODFLAGS)
>>> > - AFLAGS_MODULE   = $(MODFLAGS)
>>> > --LDFLAGS_MODULE  = -r
>>> > -+LDFLAGS_RELOCATABLE = -r -nostdlib
>>> > -+LDFLAGS_MODULE  = $(LDFLAGS_RELOCATABLE)
>>> > - CFLAGS_KERNEL =
>>> > - AFLAGS_KERNEL =
>>> > -
>>> > -@@ -331,7 +332,7 @@ KERNELVERSION = $(VERSION).$(PATCHLEVEL)
>>> > - export        VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION \
>>> > -       ARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC \
>>> > -       CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE \
>>> > --      HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
>>> > -+      HOSTCXX HOSTCXXFLAGS LDFLAGS_RELOCATABLE LDFLAGS_MODULE CHECK CHECKFLAGS
>>> > -
>>> > - export CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
>>> > - export CFLAGS CFLAGS_KERNEL CFLAGS_MODULE
>>> > -@@ -610,7 +611,7 @@ quiet_cmd_busybox__ ?= LINK    $@
>>> > -       cmd_busybox__ ?= $(srctree)/scripts/trylink \
>>> > -       "$@" \
>>> > -       "$(CC)" \
>>> > --      "$(CFLAGS) $(CFLAGS_busybox)" \
>>> > -+      "$(CFLAGS) $(CFLAGS_busybox) $(EXTRA_CFLAGS)" \
>>> > -       "$(LDFLAGS) $(EXTRA_LDFLAGS)" \
>>> > -       "$(core-y)" \
>>> > -       "$(libs-y)" \
>>> > -Index: busybox-1.23.2/scripts/Makefile.build
>>> > -===================================================================
>>> > ---- busybox-1.23.2.orig/scripts/Makefile.build
>>> > -+++ busybox-1.23.2/scripts/Makefile.build
>>> > -@@ -174,7 +174,7 @@ cmd_modversions =                                                  \
>>> > -               | $(GENKSYMS) -a $(ARCH)                                \
>>> > -               > $(@D)/.tmp_$(@F:.o=.ver);                             \
>>> > -                                                                       \
>>> > --              $(LD) $(LDFLAGS) -r -o $@ $(@D)/.tmp_$(@F)              \
>>> > -+               $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@ $(@D)/.tmp_$(@F)        \
>>> > -                       -T $(@D)/.tmp_$(@F:.o=.ver);                    \
>>> > -               rm -f $(@D)/.tmp_$(@F) $(@D)/.tmp_$(@F:.o=.ver);        \
>>> > -       else                                                            \
>>> > -@@ -257,7 +257,7 @@ quiet_cmd_link_o_target = LD      $@
>>> > - # If the list of objects to link is empty, just create an empty built-in.o
>>> > - # -nostdlib is added to make "make LD=gcc ..." work (some people use that)
>>> > - cmd_link_o_target = $(if $(strip $(obj-y)),\
>>> > --              $(LD) -nostdlib $(ld_flags) -r -o $@ $(filter $(obj-y), $^),\
>>> > -+              $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@ $(filter $(obj-y), $^),\
>>> > -               rm -f $@; $(AR) rcs $@)
>>> > -
>>> > - $(builtin-target): $(obj-y) FORCE
>>> > -@@ -292,10 +292,10 @@ $($(subst $(obj)/,,$(@:.o=-objs)))    \
>>> > - $($(subst $(obj)/,,$(@:.o=-y)))), $^)
>>> > -
>>> > - quiet_cmd_link_multi-y = LD      $@
>>> > --cmd_link_multi-y = $(LD) $(ld_flags) -r -o $@ $(link_multi_deps)
>>> > -+cmd_link_multi-y = $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@ $(link_multi_deps)
>>> > -
>>> > - quiet_cmd_link_multi-m = LD [M]  $@
>>> > --cmd_link_multi-m = $(LD) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
>>> > -+cmd_link_multi-m = $(CC) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
>>> > -
>>> > - # We would rather have a list of rules like
>>> > - #     foo.o: $(foo-objs)
>>> > -Index: busybox-1.23.2/scripts/Makefile.lib
>>> > -===================================================================
>>> > ---- busybox-1.23.2.orig/scripts/Makefile.lib
>>> > -+++ busybox-1.23.2/scripts/Makefile.lib
>>> > -@@ -121,7 +121,8 @@ cpp_flags      = -Wp,-MD,$(depfile) $(NO
>>> > - # yet ld_flags is fed to ld.
>>> > - #ld_flags       = $(LDFLAGS) $(EXTRA_LDFLAGS)
>>> > - # Remove the -Wl, prefix from linker options normally passed through gcc
>>> > --ld_flags       = $(filter-out -Wl$(comma)%,$(LDFLAGS) $(EXTRA_LDFLAGS))
>>> > -+ld_flags       = $(filter-out -Wl$(comma)%,$(LDFLAGS) $(EXTRA_LDFLAGS) $(CFLAGS) $(EXTRA_CFLAGS))
>>> > -+ld_flags_partial = $($(filter-out -shared%, $(filter-out -pie%,$(ld_flags))))
>>> > -
>>> > -
>>> > - # Finds the multi-part object the current object will be linked into
>>> > -@@ -151,10 +152,8 @@ $(obj)/%:: $(src)/%_shipped
>>> > - # Linking
>>> > - # ---------------------------------------------------------------------------
>>> > -
>>> > --# TODO: LDFLAGS usually is supposed to contain gcc's flags, not ld's.
>>> > --# but here we feed them to ld!
>>> > --quiet_cmd_ld = LD      $@
>>> > --cmd_ld = $(LD) $(LDFLAGS) $(EXTRA_LDFLAGS) $(LDFLAGS_$(@F)) \
>>> > -+quiet_cmd_ld = CC    $@
>>> > -+cmd_ld = $(CC) $(ld_flags) $(LDFLAGS_$(@F)) \
>>> > -              $(filter-out FORCE,$^) -o $@
>>> > -
>>> > - # Objcopy
>>> > diff --git a/meta/recipes-core/busybox/busybox_1.34.1.bb b/meta/recipes-core/busybox/busybox_1.34.1.bb
>>> > index 6aed0f0476..3651c06126 100644
>>> > --- a/meta/recipes-core/busybox/busybox_1.34.1.bb
>>> > +++ b/meta/recipes-core/busybox/busybox_1.34.1.bb
>>> > @@ -26,7 +26,6 @@ SRC_URI = "https://busybox.net/downloads/busybox-${PV}.tar.bz2;name=tarball \
>>> >             file://login-utilities.cfg \
>>> >             file://recognize_connmand.patch \
>>> >             file://busybox-cross-menuconfig.patch \
>>> > -           file://0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch \
>>> >             file://mount-via-label.cfg \
>>> >             file://sha1sum.cfg \
>>> >             file://sha256sum.cfg \
>>> > --
>>> > 2.20.1
>>> >
>>> >
>>> >
>>> >
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#158461): https://lists.openembedded.org/g/openembedded-core/message/158461
>> Mute This Topic: https://lists.openembedded.org/mt/87122011/1686489
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alex.kanavin@gmail.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
Alexander Kanavin Nov. 19, 2021, 8:17 a.m. UTC | #5
On Fri, 19 Nov 2021 at 00:14, Andre McCurdy <armccurdy@gmail.com> wrote:

> > So everything that is supposed to be there is actually present, and the
> patch is indeed unneeded.
>
> Looks good. If you have the full strace logs for both cases then it
> might still be useful to check which (if any) command lines are
> changed by removing the patch, but if not then removing the whole
> patch does seem OK.
>

There is no tooling to compare strace files, they have process ids, and
probably other unrelated files that needs to be stripped out or matched.

Just to reiterate: this is for the master branch, and I do insist on "if AB
is green then it works as intended" - if issues are found later, then the
right way to deal with them is to improve the AB tests, not ask maintainers
do even more manual work. Can you help me review the remaining 400
"upstream-status: pending" patches?

Alex
Andre McCurdy Nov. 19, 2021, 8:56 a.m. UTC | #6
On Fri, Nov 19, 2021 at 12:17 AM Alexander Kanavin
<alex.kanavin@gmail.com> wrote:
>
> On Fri, 19 Nov 2021 at 00:14, Andre McCurdy <armccurdy@gmail.com> wrote:
>>
>> > So everything that is supposed to be there is actually present, and the patch is indeed unneeded.
>>
>> Looks good. If you have the full strace logs for both cases then it
>> might still be useful to check which (if any) command lines are
>> changed by removing the patch, but if not then removing the whole
>> patch does seem OK.
>
> There is no tooling to compare strace files, they have process ids, and probably other unrelated files that needs to be stripped out or matched.

I guess looking at the two logs in a graphical tool such as meld would
highlight if command line options completely disappear, even with the
noise of differences in process IDs etc.

> Just to reiterate: this is for the master branch, and I do insist on "if AB is green then it works as intended" - if issues are found later, then the right way to deal with them is to improve the AB tests, not ask maintainers do even more manual work.

Yes, and I'm agreeing with you!

> Can you help me review the remaining 400 "upstream-status: pending" patches?

No. I made the decision some time ago to stop contributing to OE. Not
planning on restarting now.

Patch

diff --git a/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch b/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
deleted file mode 100644
index 2bf2b91c7e..0000000000
--- a/meta/recipes-core/busybox/busybox/0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch
+++ /dev/null
@@ -1,114 +0,0 @@ 
-From a9333eb6a7b8dbda735947cd5bc981ff9352a2c9 Mon Sep 17 00:00:00 2001
-From: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
-Date: Thu, 10 Mar 2011 00:27:08 -0500
-Subject: [PATCH 1/2] Use $(CC) when linking instead of $(LD) and use $(CFLAGS)
- and $(EXTRA_CFLAGS) when linking.
-
-This fixes the issue where LDFLAGS escaped with -Wl are ignored during
-compilation. It also simplifies using CFLAGS or EXTRA_CFLAGS (such as
--m32 on x86_64 or -flto) which apply to both compilation and linking
-situations.
-
-Signed-off-by: Nathan Phillip Brink <ohnobinki@ohnopublishing.net>
----
-Upstream-Status: Pending
-
- Makefile               |  7 ++++---
- scripts/Makefile.build |  8 ++++----
- scripts/Makefile.lib   | 13 +++----------
- 3 files changed, 11 insertions(+), 17 deletions(-)
-
-Index: busybox-1.23.2/Makefile
-===================================================================
---- busybox-1.23.2.orig/Makefile
-+++ busybox-1.23.2/Makefile
-@@ -309,7 +309,8 @@ CHECKFLAGS     := -D__linux__ -Dlinux -D
- MODFLAGS	= -DMODULE
- CFLAGS_MODULE   = $(MODFLAGS)
- AFLAGS_MODULE   = $(MODFLAGS)
--LDFLAGS_MODULE  = -r
-+LDFLAGS_RELOCATABLE = -r -nostdlib
-+LDFLAGS_MODULE  = $(LDFLAGS_RELOCATABLE)
- CFLAGS_KERNEL	=
- AFLAGS_KERNEL	=
- 
-@@ -331,7 +332,7 @@ KERNELVERSION = $(VERSION).$(PATCHLEVEL)
- export	VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION \
- 	ARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC \
- 	CPP AR NM STRIP OBJCOPY OBJDUMP MAKE AWK GENKSYMS PERL UTS_MACHINE \
--	HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
-+	HOSTCXX HOSTCXXFLAGS LDFLAGS_RELOCATABLE LDFLAGS_MODULE CHECK CHECKFLAGS
- 
- export CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
- export CFLAGS CFLAGS_KERNEL CFLAGS_MODULE
-@@ -610,7 +611,7 @@ quiet_cmd_busybox__ ?= LINK    $@
-       cmd_busybox__ ?= $(srctree)/scripts/trylink \
-       "$@" \
-       "$(CC)" \
--      "$(CFLAGS) $(CFLAGS_busybox)" \
-+      "$(CFLAGS) $(CFLAGS_busybox) $(EXTRA_CFLAGS)" \
-       "$(LDFLAGS) $(EXTRA_LDFLAGS)" \
-       "$(core-y)" \
-       "$(libs-y)" \
-Index: busybox-1.23.2/scripts/Makefile.build
-===================================================================
---- busybox-1.23.2.orig/scripts/Makefile.build
-+++ busybox-1.23.2/scripts/Makefile.build
-@@ -174,7 +174,7 @@ cmd_modversions =							\
- 		| $(GENKSYMS) -a $(ARCH)				\
- 		> $(@D)/.tmp_$(@F:.o=.ver);				\
- 									\
--		$(LD) $(LDFLAGS) -r -o $@ $(@D)/.tmp_$(@F) 		\
-+               $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@ $(@D)/.tmp_$(@F)        \
- 			-T $(@D)/.tmp_$(@F:.o=.ver);			\
- 		rm -f $(@D)/.tmp_$(@F) $(@D)/.tmp_$(@F:.o=.ver);	\
- 	else								\
-@@ -257,7 +257,7 @@ quiet_cmd_link_o_target = LD      $@
- # If the list of objects to link is empty, just create an empty built-in.o
- # -nostdlib is added to make "make LD=gcc ..." work (some people use that)
- cmd_link_o_target = $(if $(strip $(obj-y)),\
--		$(LD) -nostdlib $(ld_flags) -r -o $@ $(filter $(obj-y), $^),\
-+		$(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@ $(filter $(obj-y), $^),\
- 		rm -f $@; $(AR) rcs $@)
- 
- $(builtin-target): $(obj-y) FORCE
-@@ -292,10 +292,10 @@ $($(subst $(obj)/,,$(@:.o=-objs)))    \
- $($(subst $(obj)/,,$(@:.o=-y)))), $^)
- 
- quiet_cmd_link_multi-y = LD      $@
--cmd_link_multi-y = $(LD) $(ld_flags) -r -o $@ $(link_multi_deps)
-+cmd_link_multi-y = $(CC) $(ld_flags_partial) $(LDFLAGS_RELOCATABLE) -o $@ $(link_multi_deps)
- 
- quiet_cmd_link_multi-m = LD [M]  $@
--cmd_link_multi-m = $(LD) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
-+cmd_link_multi-m = $(CC) $(ld_flags) $(LDFLAGS_MODULE) -o $@ $(link_multi_deps)
- 
- # We would rather have a list of rules like
- # 	foo.o: $(foo-objs)
-Index: busybox-1.23.2/scripts/Makefile.lib
-===================================================================
---- busybox-1.23.2.orig/scripts/Makefile.lib
-+++ busybox-1.23.2/scripts/Makefile.lib
-@@ -121,7 +121,8 @@ cpp_flags      = -Wp,-MD,$(depfile) $(NO
- # yet ld_flags is fed to ld.
- #ld_flags       = $(LDFLAGS) $(EXTRA_LDFLAGS)
- # Remove the -Wl, prefix from linker options normally passed through gcc
--ld_flags       = $(filter-out -Wl$(comma)%,$(LDFLAGS) $(EXTRA_LDFLAGS))
-+ld_flags       = $(filter-out -Wl$(comma)%,$(LDFLAGS) $(EXTRA_LDFLAGS) $(CFLAGS) $(EXTRA_CFLAGS))
-+ld_flags_partial = $($(filter-out -shared%, $(filter-out -pie%,$(ld_flags))))
- 
- 
- # Finds the multi-part object the current object will be linked into
-@@ -151,10 +152,8 @@ $(obj)/%:: $(src)/%_shipped
- # Linking
- # ---------------------------------------------------------------------------
- 
--# TODO: LDFLAGS usually is supposed to contain gcc's flags, not ld's.
--# but here we feed them to ld!
--quiet_cmd_ld = LD      $@
--cmd_ld = $(LD) $(LDFLAGS) $(EXTRA_LDFLAGS) $(LDFLAGS_$(@F)) \
-+quiet_cmd_ld = CC    $@
-+cmd_ld = $(CC) $(ld_flags) $(LDFLAGS_$(@F)) \
- 	       $(filter-out FORCE,$^) -o $@
- 
- # Objcopy
diff --git a/meta/recipes-core/busybox/busybox_1.34.1.bb b/meta/recipes-core/busybox/busybox_1.34.1.bb
index 6aed0f0476..3651c06126 100644
--- a/meta/recipes-core/busybox/busybox_1.34.1.bb
+++ b/meta/recipes-core/busybox/busybox_1.34.1.bb
@@ -26,7 +26,6 @@  SRC_URI = "https://busybox.net/downloads/busybox-${PV}.tar.bz2;name=tarball \
            file://login-utilities.cfg \
            file://recognize_connmand.patch \
            file://busybox-cross-menuconfig.patch \
-           file://0001-Use-CC-when-linking-instead-of-LD-and-use-CFLAGS-and.patch \
            file://mount-via-label.cfg \
            file://sha1sum.cfg \
            file://sha256sum.cfg \