[OS-BUILD PATCH] Fix LTO issues with kernel-tools

[GitLab Bridge on behalf of dzickusrh]

From: Don Zickus <dzickus(a)redhat.com&gt;

There are two parts to this fix.  One is using the recommended way
to disable LTO.  The other is to make it work for the kernel.spec
file.

Various kernel-tool programs (like perf) can not handle LTO yet, so
they are disabled.

This is done with '%define _lto_cflags {nil}'.

However that doesn't quite work for the kernel for the %install
section.  It works for the %build section.

Oddly, back at the birth of dist-git, the initial kernel.spec
file was imported with a line at the top

%global __spec_install_pre %{___build_pre}

For whatever reason, the kernel was deemed special and that line
pre-built the %install scripts _before_ the lto_cflags could
dynamically be disabled.

Moving the _lto_cflags line above the _pre line disables LTO
for both the %build and %install sections of the spec file.

However, because that _pre line is unintiutive and caused hours
of debugging headache, I hacked up the output to see what the
%__spec_install_pre and ___build_pre looked like at the
beginning of the %install section.  The idea was __build_pre
is what we want going forward.

Of the 20 or so env vars defined, the only differences besides
the LTO flags was -fcommon.  This made sense because
_legacy_common_support is also set (enabled -fcommon) after
the _pre line.  Therefore the %install section was correctly
using -fno-common.

In other words having
%global __spec_install_pre %{___build_pre}

is now useless after 10 years.  Let's just get rid of
that and save future headaches and issues and use the
recommended way of disabling LTO.

Cc: Jeff Law <law(a)redhat.com&gt;

Signed-off-by: Don Zickus <dzickus(a)redhat.com&gt;
---
 redhat/kernel.spec.template | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/redhat/kernel.spec.template b/redhat/kernel.spec.template
index 7cbfb9c139eb..439c22be33c9 100644
--- a/redhat/kernel.spec.template
+++ b/redhat/kernel.spec.template
@@ -1,5 +1,4 @@
-# We have to override the new %%install behavior because, well... the kernel is special.
-%global __spec_install_pre %{___build_pre}
+%global _lto_cflags %{nil}
 
 # Short-term fix so the package builds with GCC 10.
 # This should go away soon.
-- 
GitLab

[OS-BUILD PATCHv2] Fix LTO issues with kernel-tools

[GitLab Bridge on behalf of dzickusrh]

From: Don Zickus <dzickus(a)redhat.com&gt;

There are two parts to this fix.  One is using the recommended way
to disable LTO.  The other is to make it work for the kernel.spec
file.

Various kernel-tool programs (like perf) can not handle LTO yet, so
they are disabled.

This is done with '%define _lto_cflags {nil}'.

However that doesn't quite work for the kernel for the %install
section.  It works for the %build section.

Oddly, back at the birth of dist-git, the initial kernel.spec
file was imported with a line at the top

%global __spec_install_pre %{___build_pre}

For whatever reason, the kernel was deemed special and that line
pre-built the %install scripts _before_ the lto_cflags could
dynamically be disabled.

Moving the _lto_cflags line above the _pre line disables LTO for both the
%build and %install sections of the spec file successfully.

However, because that _pre line is unintiutive and caused hours
of debugging headache, I hacked up the output to see what the
%__spec_install_pre and ___build_pre looked like at the
beginning of the %install section.  The idea was __build_pre
is what we want going forward.

Unfortunately, after examining the results, I learned the %install
section expects a clean RPM_BUILD_ROOT.  But the kernel %build
section puts each compiled variant into the RPM_BUILD_ROOT as
it completes.  Thus is gets removed on %install setup.  So the
%__spec_install_pre line has to stay.

Instead I add a bunch of comments explaining why it is necessary
and where to add changes like _lto_cflags.  This hopefully
reduces headaches in the future.

V2: restore __spec_install_pre line and add comments.

Cc: Jeff Law <law(a)redhat.com&gt;
Signed-off-by: Don Zickus <dzickus(a)redhat.com&gt;
---
 redhat/kernel.spec.template | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/redhat/kernel.spec.template b/redhat/kernel.spec.template
index 7cbfb9c139eb..82d439cfefe3 100644
--- a/redhat/kernel.spec.template
+++ b/redhat/kernel.spec.template
@@ -1,4 +1,30 @@
-# We have to override the new %%install behavior because, well... the kernel is special.
+# All Global changes to build and install go here.
+# Per the below section about __spec_install_pre, any rpm
+# environment changes that affect %%install need to go
+# here before the %%install macro is pre-built.
+
+# Disable LTO in userspace packages.
+%global _lto_cflags %{nil}
+
+
+
+# The kernel's %%install section is special
+# Normally the %%install section starts by cleaning up the BUILD_ROOT
+# like so:
+#
+# %%__spec_install_pre %%{___build_pre}\
+#     [ "$RPM_BUILD_ROOT" != "/" ] && rm -rf
"${RPM_BUILD_ROOT}"\
+#     mkdir -p `dirname "$RPM_BUILD_ROOT"`\
+#     mkdir "$RPM_BUILD_ROOT"\
+# %%{nil}
+#
+# But because of kernel variants, the %%build section, specifically
+# BuildKernel(), moves each variant to its final destination as the
+# variant is built.  This violates the expectation of the %%install
+# section.  As a result we snapshot the current env variables and
+# purposely leave out the removal section.  All global wide changes
+# should be added above this line otherwise the %%install section
+# will not see them.
 %global __spec_install_pre %{___build_pre}
 
 # Short-term fix so the package builds with GCC 10.
-- 
GitLab

[OS-BUILD PATCHv2] Fix LTO issues with kernel-tools

[Brian Masney]

On Wed, Oct 28, 2020 at 08:03:37PM -0000, GitLab Bridge on behalf of dzickusrh wrote:
>  From: Don Zickus <dzickus(a)redhat.com&gt;
>  
>  There are two parts to this fix.  One is using the recommended way
>  to disable LTO.  The other is to make it work for the kernel.spec
>  file.
>  
>  Various kernel-tool programs (like perf) can not handle LTO yet, so
>  they are disabled.
>  
>  This is done with '%define _lto_cflags {nil}'.
>  
>  However that doesn't quite work for the kernel for the %install
>  section.  It works for the %build section.
>  
>  Oddly, back at the birth of dist-git, the initial kernel.spec
>  file was imported with a line at the top
>  
>  %global __spec_install_pre %{___build_pre}
>  
>  For whatever reason, the kernel was deemed special and that line
>  pre-built the %install scripts _before_ the lto_cflags could
>  dynamically be disabled.
>  
>  Moving the _lto_cflags line above the _pre line disables LTO for both the
>  %build and %install sections of the spec file successfully.
>  
>  However, because that _pre line is unintiutive and caused hours
>  of debugging headache, I hacked up the output to see what the
>  %__spec_install_pre and ___build_pre looked like at the
>  beginning of the %install section.  The idea was __build_pre
>  is what we want going forward.
>  
>  Unfortunately, after examining the results, I learned the %install
>  section expects a clean RPM_BUILD_ROOT.  But the kernel %build
>  section puts each compiled variant into the RPM_BUILD_ROOT as
>  it completes.  Thus is gets removed on %install setup.  So the
>  %__spec_install_pre line has to stay.
>  
>  Instead I add a bunch of comments explaining why it is necessary
>  and where to add changes like _lto_cflags.  This hopefully
>  reduces headaches in the future.
>  
>  V2: restore __spec_install_pre line and add comments.
>  
>  Cc: Jeff Law <law(a)redhat.com&gt;
>  Signed-off-by: Don Zickus <dzickus(a)redhat.com&gt; 
I like this version much better.

Acked-by: Brian Masney <bmasney(a)redhat.com&gt;

[Herton R. Krzesinski]

On Wed, Oct 28, 2020 at 03:23:03AM -0000, GitLab Bridge on behalf of dzickusrh wrote:
>  From: Don Zickus <dzickus(a)redhat.com&gt;
>   
(...)
>  Cc: Jeff Law <law(a)redhat.com&gt;
>  
>  Signed-off-by: Don Zickus <dzickus(a)redhat.com&gt;
>  ---
>   redhat/kernel.spec.template | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>  
>  diff --git a/redhat/kernel.spec.template b/redhat/kernel.spec.template
>  index 7cbfb9c139eb..439c22be33c9 100644
>  --- a/redhat/kernel.spec.template
>  +++ b/redhat/kernel.spec.template
>  @@ -1,5 +1,4 @@
>  -# We have to override the new %%install behavior because, well... the kernel is
> special.
>  -%global __spec_install_pre %{___build_pre}
>  +%global _lto_cflags %{nil}
>   
>   # Short-term fix so the package builds with GCC 10.
>   # This should go away soon. 
Acked-by: Herton R. Krzesinski <herton(a)redhat.com&gt;

-- 
[]'s
Herton