This small series contains five cleanups to kernel.spec. The basic theme is the way we run "make *config" targets on our .config files.
(There was talk, recently, of generating these .config files differently. I do not know what happenend there. But this series only touches kernel.spec once we already have .config files, so that's another stage in our build.)
This was only tested for x64_64 on a branch based of f24. It does apply cleanly to master. A run on the build farms (of which I know nothing) might uncover bugs in areas my testing didn't cover. I hope it doesn't.
Paul Bolle (5): Run oldnoconfig make targets silently Run make listnewconfig only if listnewconfig_fail is set Only run listnewconfig and oldnoconfig for one arch Replace 'all_x86' with 'x86_32' Remove references to (31 bits) s390
kernel.spec | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
In the %prep stage of the kernel build we run the "make oldnoconfig" target fifteen times. And every time it prints: scripts/kconfig/conf --olddefconfig Kconfig # # configuration written to .config #
Run this target silently so these four lines are suppressed.
Signed-off-by: Paul Bolle pebolle@tiscali.nl --- kernel.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel.spec b/kernel.spec index cf54b60c75b7..08bfd7a9eac1 100644 --- a/kernel.spec +++ b/kernel.spec @@ -1258,7 +1258,7 @@ do fi %endif rm -f .newoptions - make ARCH=$Arch oldnoconfig + make -s ARCH=$Arch oldnoconfig echo "# $Arch" > configs/$i cat .config >> configs/$i done
On Thu, 2016-11-10 at 17:08 +0100, Paul Bolle wrote:
In the %prep stage of the kernel build we run the "make oldnoconfig" target fifteen times. And every time it prints: scripts/kconfig/conf --olddefconfig Kconfig # # configuration written to .config #
Run this target silently so these four lines are suppressed.
Josh objected to 3/5. So I cobbled together an alternative to this patch that does most I want in 1/5 and 3/5. It should be more palatable to Josh. See below.
I'll resend this series in a week or so after (other) people have had a chance to look at patches 2 to 5 too.
Thanks,
Paul Bolle
Subject: [PATCH] Don't run oldnoconfig twice
The .config file used to build the kernel is generated with "make oldnoconfig" during the %build stage. So there's no need to also call "make oldnoconfig" during the %prep stage, as that can't possibly change anything.
Not-yet-signed-off-by: Paul Bolle pebolle@tiscali.nl --- kernel.spec | 1 - 1 file changed, 1 deletion(-)
diff --git a/kernel.spec b/kernel.spec index abe0ab6a5ef8..51dc62dae014 100644 --- a/kernel.spec +++ b/kernel.spec @@ -1308,7 +1308,6 @@ do fi rm -f .newoptions %endif - make ARCH=$Arch oldnoconfig echo "# $Arch" > configs/$i cat .config >> configs/$i done
On Thu, Nov 10, 2016 at 6:43 PM, Paul Bolle pebolle@tiscali.nl wrote:
On Thu, 2016-11-10 at 17:08 +0100, Paul Bolle wrote:
In the %prep stage of the kernel build we run the "make oldnoconfig" target fifteen times. And every time it prints: scripts/kconfig/conf --olddefconfig Kconfig # # configuration written to .config #
Run this target silently so these four lines are suppressed.
Josh objected to 3/5. So I cobbled together an alternative to this patch that does most I want in 1/5 and 3/5. It should be more palatable to Josh. See below.
I agree with Josh, what is it that you're actually trying to achieve here?
Peter
On Thu, 2016-11-10 at 20:28 +0000, Peter Robinson wrote:
I agree with Josh, what is it that you're actually trying to achieve here?
What I want to achieve is to make the build a bit more quiet. And I prefer it to not do things for no good reason.
But it turns out I can get what I want if we remove the call of "make oldnoconfig" during %prep. There really is no need to do that for all config files in configs/, as only one of those files will be used for the build. And the file that actually will be used during %build will go through "make oldnoconfig". So why bother doing that earlier during %prep?
And that's what the draft patch a few messages back in this thread does.
Thanks,
Paul Bolle
On Thu, Nov 10, 2016 at 8:59 PM, Paul Bolle pebolle@tiscali.nl wrote:
On Thu, 2016-11-10 at 20:28 +0000, Peter Robinson wrote:
I agree with Josh, what is it that you're actually trying to achieve here?
What I want to achieve is to make the build a bit more quiet. And I prefer it to not do things for no good reason.
But it turns out I can get what I want if we remove the call of "make oldnoconfig" during %prep. There really is no need to do that for all config files in configs/, as only one of those files will be used for the build. And the file that actually will be used during %build will go through "make oldnoconfig". So why bother doing that earlier during %prep?
The reason it's done during prep for all config files is so that maintainers can do a basic "fedpkg prep" to make sure they haven't broken anything before pushing it. I do this numerous times a day when working on kernel stuff.
Peter
On Thu, Nov 10, 2016 at 4:03 PM, Peter Robinson pbrobinson@gmail.com wrote:
On Thu, Nov 10, 2016 at 8:59 PM, Paul Bolle pebolle@tiscali.nl wrote:
On Thu, 2016-11-10 at 20:28 +0000, Peter Robinson wrote:
I agree with Josh, what is it that you're actually trying to achieve here?
What I want to achieve is to make the build a bit more quiet. And I prefer it to not do things for no good reason.
But it turns out I can get what I want if we remove the call of "make oldnoconfig" during %prep. There really is no need to do that for all config files in configs/, as only one of those files will be used for the build. And the file that actually will be used during %build will go through "make oldnoconfig". So why bother doing that earlier during %prep?
The reason it's done during prep for all config files is so that maintainers can do a basic "fedpkg prep" to make sure they haven't broken anything before pushing it. I do this numerous times a day when working on kernel stuff.
Right.
Paul, your changes make some logical sense but they break the workflow of the people that have to maintain the package. That's where most of the pushback is going to come from.
josh
On Thu, 2016-11-10 at 16:09 -0500, Josh Boyer wrote:
Paul, your changes make some logical sense but they break the workflow of the people that have to maintain the package. That's where most of the pushback is going to come from.
(Did I sound annoyed about any pushback? I hope I didn't.)
So perhaps I'll end up with the original patch here (ie, adding the "-s" flag) as the messages "make oldnoconfig" now generates hopefully aren't relevant for your workflow. We'll see.
You're not, well, principally opposed to patches that make sense from the perspective of people rebuilding locally, are you?
Thanks,
Paul Bolle
On Thu, Nov 10, 2016 at 4:32 PM, Paul Bolle pebolle@tiscali.nl wrote:
On Thu, 2016-11-10 at 16:09 -0500, Josh Boyer wrote:
Paul, your changes make some logical sense but they break the workflow of the people that have to maintain the package. That's where most of the pushback is going to come from.
(Did I sound annoyed about any pushback? I hope I didn't.)
No, not at all. And I didn't think you were. I was just offering extra context.
So perhaps I'll end up with the original patch here (ie, adding the "-s" flag) as the messages "make oldnoconfig" now generates hopefully aren't relevant for your workflow. We'll see.
I think that makes sense. We'd have to try them to be sure.
You're not, well, principally opposed to patches that make sense from the perspective of people rebuilding locally, are you?
Not opposed at all, but it can't be at the expense of people that have to do things with this package multiple times a day.
josh
On Thu, 2016-11-10 at 19:38 -0500, Josh Boyer wrote:
[...] but it can't be at the expense of people that have to do things with this package multiple times a day.
Sure.
But - to keep my reply at a reasonable length - setting defaults to values that casual builders, probably, expect shouldn't be seen as doing something at the expense of the people working with the package on a day to day basis.
Paul Bolle
On Mon, Nov 14, 2016 at 2:08 PM, Paul Bolle pebolle@tiscali.nl wrote:
On Thu, 2016-11-10 at 19:38 -0500, Josh Boyer wrote:
[...] but it can't be at the expense of people that have to do things with this package multiple times a day.
Sure.
But - to keep my reply at a reasonable length - setting defaults to values that casual builders, probably, expect shouldn't be seen as doing something at the expense of the people working with the package on a day to day basis.
You keep saying things like that, but I honestly have lost all context of what you are actually wanting to change here and why.
Could you start with a summary email of the workflow you are desiring, with some examples perhaps? That would be helpful.
josh
On Mon, 2016-11-14 at 14:47 -0500, Josh Boyer wrote:
You keep saying things like that, but I honestly have lost all context of what you are actually wanting to change here and why.
My overall goals are pretty vague, so I suppose my objections are pretty vague too. My goals for this series are pretty clear, however
And as far as this series is concerned: only patch 3/5 is controversial, right?
Could you start with a summary email of the workflow you are desiring, with some examples perhaps? That would be helpful.
Well, hmm. I guess there are three kind of reasons to build the kernel rpm locally.
(If none of these reasons make sense, or, even worse, you suspect almost no one is actually building the kernel rpm locally in the first place we might as well stop this discussion right here. In that case I'm inclined to take "_we_ don't care" as a reasonable objection to any patches I submit.)
1) Rebuilding a minimal set of the kernel packages as quickly, basically, etc. as possible. Probably to test the stack of local patches one has. That's what I do. It requires "--with vanilla" and a few "--without" rpmbuild flags.
2) Rebuilding the entire set of kernel packages for your local architecture, with all Fedora specific patches, etc. Probably also to test some local patches. I never do that.
3) Play Fauxdora and rebuild all kernel packages for all supported architectures. I never do that, and can't imagine I'll ever want to do that.
Anyhow, my sort of, kind a goal is to make 1) as easy as possible. Does this clarify things a bit?
Paul Bolle
On 11/14/2016 12:36 PM, Paul Bolle wrote:
On Mon, 2016-11-14 at 14:47 -0500, Josh Boyer wrote:
You keep saying things like that, but I honestly have lost all context of what you are actually wanting to change here and why.
My overall goals are pretty vague, so I suppose my objections are pretty vague too. My goals for this series are pretty clear, however
And as far as this series is concerned: only patch 3/5 is controversial, right?
Could you start with a summary email of the workflow you are desiring, with some examples perhaps? That would be helpful.
Well, hmm. I guess there are three kind of reasons to build the kernel rpm locally.
(If none of these reasons make sense, or, even worse, you suspect almost no one is actually building the kernel rpm locally in the first place we might as well stop this discussion right here. In that case I'm inclined to take "_we_ don't care" as a reasonable objection to any patches I submit.)
- Rebuilding a minimal set of the kernel packages as quickly, basically, etc.
as possible. Probably to test the stack of local patches one has. That's what I do. It requires "--with vanilla" and a few "--without" rpmbuild flags.
- Rebuilding the entire set of kernel packages for your local architecture,
with all Fedora specific patches, etc. Probably also to test some local patches. I never do that.
- Play Fauxdora and rebuild all kernel packages for all supported
architectures. I never do that, and can't imagine I'll ever want to do that.
Anyhow, my sort of, kind a goal is to make 1) as easy as possible. Does this clarify things a bit?
These are all fine reasons and motivations but I'm not seeing the benefit of this patch specifically. The cost to generate a few extra arch configs is negligible compared to the actual time spent building the kernel. Do you have numbers showing how much your build time is sped up?
Paul Bolle
Thanks, Laura
On Mon, 2016-11-14 at 14:48 -0800, Laura Abbott wrote:
Do you have numbers showing how much your build time is sped up?
It appears I'm pretty good at presenting my case unconvincingly. So good actually that I don't have to present daft reasons to be unconvincing.
Anyhow, I don't think I even implied this series is about decreasing build times.
Paul Bolle
We run make listnewconfig even if listnewconfig_fail is not set. Eg, if we build using "--with vanilla". But in that case we simply ignore the results of that make target.
If we don't care about the results we might as well not run this make target. So only run it if listnewconfig_fail is set.
Signed-off-by: Paul Bolle pebolle@tiscali.nl --- kernel.spec | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel.spec b/kernel.spec index 08bfd7a9eac1..f0fe21fef94b 100644 --- a/kernel.spec +++ b/kernel.spec @@ -1250,14 +1250,14 @@ for i in *.config do mv $i .config Arch=`head -1 .config | cut -b 3-` - make ARCH=$Arch listnewconfig | grep -E '^CONFIG_' >.newoptions || true %if %{listnewconfig_fail} + make ARCH=$Arch listnewconfig | grep -E '^CONFIG_' >.newoptions || true if [ -s .newoptions ]; then cat .newoptions exit 1 fi -%endif rm -f .newoptions +%endif make -s ARCH=$Arch oldnoconfig echo "# $Arch" > configs/$i cat .config >> configs/$i
During the %prep phase we run "make listnewconfig" and "make oldnoconfig" for all six supported architectures (arm64, arm, i386, powerpc, s390, and x86_64). We only care about the set of .configs that is relevant for the current build architecture. So skip these two make targets for the other architectures.
Signed-off-by: Paul Bolle pebolle@tiscali.nl --- kernel.spec | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/kernel.spec b/kernel.spec index f0fe21fef94b..e791d2565efc 100644 --- a/kernel.spec +++ b/kernel.spec @@ -1250,6 +1250,9 @@ for i in *.config do mv $i .config Arch=`head -1 .config | cut -b 3-` + if [ $Arch != %{hdrarch} ]; then + continue + fi %if %{listnewconfig_fail} make ARCH=$Arch listnewconfig | grep -E '^CONFIG_' >.newoptions || true if [ -s .newoptions ]; then
On Thu, Nov 10, 2016 at 11:08 AM, Paul Bolle pebolle@tiscali.nl wrote:
During the %prep phase we run "make listnewconfig" and "make oldnoconfig" for all six supported architectures (arm64, arm, i386, powerpc, s390, and x86_64). We only care about the set of .configs that is relevant for the current build architecture. So skip these two make targets for the other architectures.
Signed-off-by: Paul Bolle pebolle@tiscali.nl
NAK on this one.
We used to only run it for the current build architecture. However, when we did that what wound up happening is that we'd have the new options configured for that specific architecture (which is always x86_64 because that is what people use), and it would then fail in koji on the other architectures because of unset new configs.
So we run it for all arches to make sure we catch all new config options as they come in.
josh
kernel.spec | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/kernel.spec b/kernel.spec index f0fe21fef94b..e791d2565efc 100644 --- a/kernel.spec +++ b/kernel.spec @@ -1250,6 +1250,9 @@ for i in *.config do mv $i .config Arch=`head -1 .config | cut -b 3-`
- if [ $Arch != %{hdrarch} ]; then
- continue
- fi
%if %{listnewconfig_fail} make ARCH=$Arch listnewconfig | grep -E '^CONFIG_' >.newoptions || true if [ -s .newoptions ]; then -- 2.7.4 _______________________________________________ kernel mailing list -- kernel@lists.fedoraproject.org To unsubscribe send an email to kernel-leave@lists.fedoraproject.org
On Thu, 2016-11-10 at 11:16 -0500, Josh Boyer wrote:
We used to only run it for the current build architecture. However, when we did that what wound up happening is that we'd have the new options configured for that specific architecture (which is always x86_64 because that is what people use), and it would then fail in koji on the other architectures because of unset new configs.
On the bright side: at least koji did what it is supposed to do. But I guess starting a new run on koji requires a non-trivial effort.
So we run it for all arches to make sure we catch all new config options as they come in.
So this about the listnewconfig errors specifically, isn't it?
Because in that case there's an alternative approach to this patch (and 1/5) that achieves most of what I actually care about.
Thanks,
Paul Bolle
On Thu, Nov 10, 2016 at 1:09 PM, Paul Bolle pebolle@tiscali.nl wrote:
On Thu, 2016-11-10 at 11:16 -0500, Josh Boyer wrote:
We used to only run it for the current build architecture. However, when we did that what wound up happening is that we'd have the new options configured for that specific architecture (which is always x86_64 because that is what people use), and it would then fail in koji on the other architectures because of unset new configs.
On the bright side: at least koji did what it is supposed to do. But I guess starting a new run on koji requires a non-trivial effort.
Right. It results in multiple config runs and starts and commits and bleh.
So we run it for all arches to make sure we catch all new config options as they come in.
So this about the listnewconfig errors specifically, isn't it?
Because in that case there's an alternative approach to this patch (and 1/5) that achieves most of what I actually care about.
Mostly, I think.
josh
The %all_x86 macro is a bit ambiguous. Rename it to %x86_32, as that is less ambiguous.
Signed-off-by: Paul Bolle pebolle@tiscali.nl --- kernel.spec | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel.spec b/kernel.spec index e791d2565efc..05e0190eea19 100644 --- a/kernel.spec +++ b/kernel.spec @@ -212,11 +212,11 @@ Summary: The Linux kernel %define with_perf 0 %endif
-%define all_x86 i386 i686 +%define x86_32 i386 i686
%if %{with_vdso_install} # These arches install vdso/ directories. -%define vdso_arches %{all_x86} x86_64 %{power64} s390 s390x aarch64 +%define vdso_arches %{x86_32} x86_64 %{power64} s390 s390x aarch64 %endif
# Overrides for generic default options @@ -245,7 +245,7 @@ Summary: The Linux kernel
# Per-arch tweaks
-%ifarch %{all_x86} +%ifarch %{x86_32} %define asmarch x86 %define hdrarch i386 %define pae PAE @@ -359,7 +359,7 @@ Version: %{rpmversion} Release: %{pkg_release} # DO NOT CHANGE THE 'ExclusiveArch' LINE TO TEMPORARILY EXCLUDE AN ARCHITECTURE BUILD. # SET %%nobuildarches (ABOVE) INSTEAD -ExclusiveArch: %{all_x86} x86_64 ppc64 ppc64p7 s390 s390x %{arm} aarch64 ppc64le +ExclusiveArch: %{x86_32} x86_64 ppc64 ppc64p7 s390 s390x %{arm} aarch64 ppc64le ExclusiveOS: Linux %ifnarch %{nobuildarches} Requires: kernel-core-uname-r = %{KVERREL}%{?variant}
On Thursday, November 10, 2016 5:08:41 PM CST Paul Bolle wrote:
The %all_x86 macro is a bit ambiguous. Rename it to %x86_32, as that is less ambiguous.
Signed-off-by: Paul Bolle pebolle@tiscali.nl
kernel.spec | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel.spec b/kernel.spec index e791d2565efc..05e0190eea19 100644 --- a/kernel.spec +++ b/kernel.spec @@ -212,11 +212,11 @@ Summary: The Linux kernel %define with_perf 0 %endif
-%define all_x86 i386 i686 +%define x86_32 i386 i686
why not just use rpms %{ix86} it does include geode, i486 i586 as well but it seems simpler to me
Dennis
%if %{with_vdso_install} # These arches install vdso/ directories. -%define vdso_arches %{all_x86} x86_64 %{power64} s390 s390x aarch64 +%define vdso_arches %{x86_32} x86_64 %{power64} s390 s390x aarch64 %endif
# Overrides for generic default options @@ -245,7 +245,7 @@ Summary: The Linux kernel
# Per-arch tweaks
-%ifarch %{all_x86} +%ifarch %{x86_32} %define asmarch x86 %define hdrarch i386 %define pae PAE @@ -359,7 +359,7 @@ Version: %{rpmversion} Release: %{pkg_release} # DO NOT CHANGE THE 'ExclusiveArch' LINE TO TEMPORARILY EXCLUDE AN ARCHITECTURE BUILD. # SET %%nobuildarches (ABOVE) INSTEAD -ExclusiveArch: %{all_x86} x86_64 ppc64 ppc64p7 s390 s390x %{arm} aarch64 ppc64le +ExclusiveArch: %{x86_32} x86_64 ppc64 ppc64p7 s390 s390x %{arm} aarch64 ppc64le ExclusiveOS: Linux %ifnarch %{nobuildarches} Requires: kernel-core-uname-r = %{KVERREL}%{?variant}
We don't build for (31 bits) s390 but only for (64 bits) s390x. So remove a few irrelevant references to s390.
Signed-off-by: Paul Bolle pebolle@tiscali.nl --- kernel.spec | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel.spec b/kernel.spec index 05e0190eea19..434059e759b3 100644 --- a/kernel.spec +++ b/kernel.spec @@ -216,7 +216,7 @@ Summary: The Linux kernel
%if %{with_vdso_install} # These arches install vdso/ directories. -%define vdso_arches %{x86_32} x86_64 %{power64} s390 s390x aarch64 +%define vdso_arches %{x86_32} x86_64 %{power64} s390x aarch64 %endif
# Overrides for generic default options @@ -324,7 +324,7 @@ Summary: The Linux kernel # Which is a BadThing(tm).
# We only build kernel-headers on the following... -%define nobuildarches i386 s390 +%define nobuildarches i386
%ifarch %nobuildarches %define with_up 0 @@ -359,7 +359,7 @@ Version: %{rpmversion} Release: %{pkg_release} # DO NOT CHANGE THE 'ExclusiveArch' LINE TO TEMPORARILY EXCLUDE AN ARCHITECTURE BUILD. # SET %%nobuildarches (ABOVE) INSTEAD -ExclusiveArch: %{x86_32} x86_64 ppc64 ppc64p7 s390 s390x %{arm} aarch64 ppc64le +ExclusiveArch: %{x86_32} x86_64 ppc64 ppc64p7 s390x %{arm} aarch64 ppc64le ExclusiveOS: Linux %ifnarch %{nobuildarches} Requires: kernel-core-uname-r = %{KVERREL}%{?variant} @@ -380,7 +380,7 @@ BuildRequires: sparse %if %{with_perf} BuildRequires: zlib-devel binutils-devel newt-devel python-devel perl(ExtUtils::Embed) bison flex xz-devel BuildRequires: audit-libs-devel -%ifnarch s390 s390x %{arm} +%ifnarch s390x %{arm} BuildRequires: numactl-devel %endif %endif
On Thu, Nov 10, 2016 at 11:08 AM, Paul Bolle pebolle@tiscali.nl wrote:
We don't build for (31 bits) s390 but only for (64 bits) s390x. So remove a few irrelevant references to s390.
Signed-off-by: Paul Bolle pebolle@tiscali.nl
Seems fine.
josh
kernel.spec | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel.spec b/kernel.spec index 05e0190eea19..434059e759b3 100644 --- a/kernel.spec +++ b/kernel.spec @@ -216,7 +216,7 @@ Summary: The Linux kernel
%if %{with_vdso_install} # These arches install vdso/ directories. -%define vdso_arches %{x86_32} x86_64 %{power64} s390 s390x aarch64 +%define vdso_arches %{x86_32} x86_64 %{power64} s390x aarch64 %endif
# Overrides for generic default options @@ -324,7 +324,7 @@ Summary: The Linux kernel # Which is a BadThing(tm).
# We only build kernel-headers on the following... -%define nobuildarches i386 s390 +%define nobuildarches i386
%ifarch %nobuildarches %define with_up 0 @@ -359,7 +359,7 @@ Version: %{rpmversion} Release: %{pkg_release} # DO NOT CHANGE THE 'ExclusiveArch' LINE TO TEMPORARILY EXCLUDE AN ARCHITECTURE BUILD. # SET %%nobuildarches (ABOVE) INSTEAD -ExclusiveArch: %{x86_32} x86_64 ppc64 ppc64p7 s390 s390x %{arm} aarch64 ppc64le +ExclusiveArch: %{x86_32} x86_64 ppc64 ppc64p7 s390x %{arm} aarch64 ppc64le ExclusiveOS: Linux %ifnarch %{nobuildarches} Requires: kernel-core-uname-r = %{KVERREL}%{?variant} @@ -380,7 +380,7 @@ BuildRequires: sparse %if %{with_perf} BuildRequires: zlib-devel binutils-devel newt-devel python-devel perl(ExtUtils::Embed) bison flex xz-devel BuildRequires: audit-libs-devel -%ifnarch s390 s390x %{arm} +%ifnarch s390x %{arm} BuildRequires: numactl-devel %endif %endif -- 2.7.4 _______________________________________________ kernel mailing list -- kernel@lists.fedoraproject.org To unsubscribe send an email to kernel-leave@lists.fedoraproject.org
kernel@lists.fedoraproject.org
-
Dennis Gilmore -
Josh Boyer -
Laura Abbott -
Paul Bolle -
Peter Robinson