All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl
@ 2016-02-07 13:41 gustavo.zacarias at free-electrons.com
  2016-02-07 14:50 ` Thomas Petazzoni
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: gustavo.zacarias at free-electrons.com @ 2016-02-07 13:41 UTC (permalink / raw)
  To: buildroot

From: Gustavo Zacarias <gustavo.zacarias@free-electrons.com>

Kernels older than 3.9 (not counting stable releases) used the
timeconst.pl perl script for their build process.
The problem with this script is that it used deprecated perl features,
namely defined(@array) which was removed for the perl 5.22 release,
causing build failure of older kernels on newer distributions.

To fix this instead of going the hard way (moving to the new
timeconst.bc script) use the easy way by patching timeconst.pl with an
upstream patch used for stable releases.

First try a dry-run on the patch to see if it applies, if it does then
call a proper APPLY_PATCHES to it.

Tested against an arbitrary 2.6.30 kernel (applies and builds), against
4.4.1 for a missing timeconst.pl (does not apply since it's missing) and
3.8.13 (does not apply since it's fixed already).

Known broken distributions: fedora 23.

Signed-off-by: Gustavo Zacarias <gustavo.zacarias@free-electrons.com>
---
v2: rebase on top of master

 ...nst.pl-Eliminate-Perl-warning.patch.conditional | 45 ++++++++++++++++++++++
 linux/linux.mk                                     | 10 +++++
 2 files changed, 55 insertions(+)
 create mode 100644 linux/0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional

diff --git a/linux/0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional b/linux/0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional
new file mode 100644
index 0000000..02abadd
--- /dev/null
+++ b/linux/0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional
@@ -0,0 +1,45 @@
+From 63a3f603413ffe82ad775f2d62a5afff87fd94a0 Mon Sep 17 00:00:00 2001
+From: "H. Peter Anvin" <hpa@linux.intel.com>
+Date: Thu, 7 Feb 2013 17:14:08 -0800
+Subject: [PATCH] timeconst.pl: Eliminate Perl warning
+
+defined(@array) is deprecated in Perl and gives off a warning.
+Restructure the code to remove that warning.
+
+[ hpa: it would be interesting to revert to the timeconst.bc script.
+  It appears that the failures reported by akpm during testing of
+  that script was due to a known broken version of make, not a problem
+  with bc.  The Makefile rules could probably be restructured to avoid
+  the make bug, or it is probably old enough that it doesn't matter. ]
+
+Reported-by: Andi Kleen <ak@linux.intel.com>
+Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
+Cc: Andrew Morton <akpm@linux-foundation.org>
+Cc: <stable@vger.kernel.org>
+Signed-off-by: Gustavo Zacarias <gustavo.zacarias@free-electrons.com>
+---
+Patch status: upstream
+
+ kernel/timeconst.pl | 6 ++----
+ 1 file changed, 2 insertions(+), 4 deletions(-)
+
+diff --git a/kernel/timeconst.pl b/kernel/timeconst.pl
+index eb51d76..3f42652 100644
+--- a/kernel/timeconst.pl
++++ b/kernel/timeconst.pl
+@@ -369,10 +369,8 @@ if ($hz eq '--can') {
+ 		die "Usage: $0 HZ\n";
+ 	}
+ 
+-	@val = @{$canned_values{$hz}};
+-	if (!defined(@val)) {
+-		@val = compute_values($hz);
+-	}
++	$cv = $canned_values{$hz};
++	@val = defined($cv) ? @$cv : compute_values($hz);
+ 	output($hz, @val);
+ }
+ exit 0;
+-- 
+2.4.10
+
diff --git a/linux/linux.mk b/linux/linux.mk
index 578fea1..7e20255 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -184,6 +184,16 @@ endef
 
 LINUX_POST_PATCH_HOOKS += LINUX_APPLY_LOCAL_PATCHES
 
+# Older linux kernels use deprecated perl constructs in timeconst.pl
+# that were removed for perl 5.22+ so it breaks on newer distributions
+# Try a dry-run patch to see if this applies, if it does go ahead
+define LINUX_TRY_PATCH_TIMECONST
+	@if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional >/dev/null ; then \
+		$(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional ; \
+	fi
+endef
+LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_TIMECONST
+
 ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y)
 LINUX_KCONFIG_DEFCONFIG = $(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig
 else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
-- 
2.4.10

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

* [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl
  2016-02-07 13:41 [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl gustavo.zacarias at free-electrons.com
@ 2016-02-07 14:50 ` Thomas Petazzoni
  2016-02-09 23:08 ` Arnout Vandecappelle
  2016-02-10  7:12 ` Peter Korsgaard
  2 siblings, 0 replies; 8+ messages in thread
From: Thomas Petazzoni @ 2016-02-07 14:50 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun,  7 Feb 2016 10:41:48 -0300, gustavo.zacarias at free-electrons.com
wrote:

> +# Older linux kernels use deprecated perl constructs in timeconst.pl
> +# that were removed for perl 5.22+ so it breaks on newer distributions
> +# Try a dry-run patch to see if this applies, if it does go ahead
> +define LINUX_TRY_PATCH_TIMECONST
> +	@if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional >/dev/null ; then \
> +		$(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional ; \
> +	fi
> +endef
> +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_TIMECONST

I really don't like this hack, but I am not sure to see a better
solution. So I'll leave a few days before applying to see if other
people chime in with other comments/suggestions, and if nobody
comments, I'll apply.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

* [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl
  2016-02-07 13:41 [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl gustavo.zacarias at free-electrons.com
  2016-02-07 14:50 ` Thomas Petazzoni
@ 2016-02-09 23:08 ` Arnout Vandecappelle
  2016-02-09 23:19   ` Gustavo Zacarias
  2016-02-10  7:11   ` Peter Korsgaard
  2016-02-10  7:12 ` Peter Korsgaard
  2 siblings, 2 replies; 8+ messages in thread
From: Arnout Vandecappelle @ 2016-02-09 23:08 UTC (permalink / raw)
  To: buildroot

On 07-02-16 14:41, gustavo.zacarias at free-electrons.com wrote:
> From: Gustavo Zacarias <gustavo.zacarias@free-electrons.com>
> 
> Kernels older than 3.9 (not counting stable releases) used the
> timeconst.pl perl script for their build process.
> The problem with this script is that it used deprecated perl features,
> namely defined(@array) which was removed for the perl 5.22 release,
> causing build failure of older kernels on newer distributions.
> 
> To fix this instead of going the hard way (moving to the new
> timeconst.bc script) use the easy way by patching timeconst.pl with an
> upstream patch used for stable releases.
> 
> First try a dry-run on the patch to see if it applies, if it does then
> call a proper APPLY_PATCHES to it.
> 
> Tested against an arbitrary 2.6.30 kernel (applies and builds), against
> 4.4.1 for a missing timeconst.pl (does not apply since it's missing) and
> 3.8.13 (does not apply since it's fixed already).
> 
> Known broken distributions: fedora 23.

 and debian testing (stretch) and unstable (sid).

 As Thomas said, it's dirty, but we really need this.

> 
> Signed-off-by: Gustavo Zacarias <gustavo.zacarias@free-electrons.com>
> ---
> v2: rebase on top of master
> 
>  ...nst.pl-Eliminate-Perl-warning.patch.conditional | 45 ++++++++++++++++++++++
>  linux/linux.mk                                     | 10 +++++
>  2 files changed, 55 insertions(+)
>  create mode 100644 linux/0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional
> 

[snip]

> diff --git a/linux/linux.mk b/linux/linux.mk
> index 578fea1..7e20255 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -184,6 +184,16 @@ endef
>  
>  LINUX_POST_PATCH_HOOKS += LINUX_APPLY_LOCAL_PATCHES
>  
> +# Older linux kernels use deprecated perl constructs in timeconst.pl
> +# that were removed for perl 5.22+ so it breaks on newer distributions
> +# Try a dry-run patch to see if this applies, if it does go ahead
> +define LINUX_TRY_PATCH_TIMECONST
> +	@if patch -p1 --dry-run -f -s -d $(@D) <$(LINUX_PKGDIR)/0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional >/dev/null ; then \
> +		$(APPLY_PATCHES) $(@D) $(LINUX_PKGDIR) 0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional ; \

 The only reason to use APPLY_PATCHES is that it updates .applied_patches_list,
right? In that case, perhaps it's better to do that directly here. So instead of
a dry-run, just apply the patch right away, and if it succeeds add it to
.applied_patches_list.

> +	fi
> +endef
> +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_TIMECONST

 This is a patch that we probably _do_ want to apply even in case of
OVERRIDE_SRCDIR. So maybe add it to LINUX_PRE_CONFIGURE_HOOKS instead. Even
though that's even more of a hack (and conflicts with the out-of-tree build
support).


 Regards,
 Arnout

> +
>  ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y)
>  LINUX_KCONFIG_DEFCONFIG = $(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig
>  else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

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

* [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl
  2016-02-09 23:08 ` Arnout Vandecappelle
@ 2016-02-09 23:19   ` Gustavo Zacarias
  2016-02-10  0:37     ` Arnout Vandecappelle
  2016-02-10  7:11   ` Peter Korsgaard
  1 sibling, 1 reply; 8+ messages in thread
From: Gustavo Zacarias @ 2016-02-09 23:19 UTC (permalink / raw)
  To: buildroot

On 09/02/16 20:08, Arnout Vandecappelle wrote:

>   and debian testing (stretch) and unstable (sid).
>
>   As Thomas said, it's dirty, but we really need this.

Hi Arnout.
Yes, i couldn't figure out a cleaner way to do this, continues below...

>   The only reason to use APPLY_PATCHES is that it updates .applied_patches_list,
> right? In that case, perhaps it's better to do that directly here. So instead of
> a dry-run, just apply the patch right away, and if it succeeds add it to
> .applied_patches_list.

I could have probed the timeconst.pl file presence and relevancy (if it 
contains defined(@array)...), but then a dry-run patch does that for me 
just fine with the proper parameters.
Yes, that's the reason for APPLY_PATCHES, however since it's already 
there why bother duplicating code?
Also thought of extending apply-patches.sh to add an option for 
try-do/discard, but i believe it's a double-edged sword and not worth it 
quite yet.

>   This is a patch that we probably _do_ want to apply even in case of
> OVERRIDE_SRCDIR. So maybe add it to LINUX_PRE_CONFIGURE_HOOKS instead. Even
> though that's even more of a hack (and conflicts with the out-of-tree build
> support).

Possibly, let's hear other people thoughts on this.
Regards.

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

* [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl
  2016-02-09 23:19   ` Gustavo Zacarias
@ 2016-02-10  0:37     ` Arnout Vandecappelle
  2016-02-10  1:27       ` Gustavo Zacarias
  0 siblings, 1 reply; 8+ messages in thread
From: Arnout Vandecappelle @ 2016-02-10  0:37 UTC (permalink / raw)
  To: buildroot



On 10-02-16 00:19, Gustavo Zacarias wrote:
> On 09/02/16 20:08, Arnout Vandecappelle wrote:
> 
>>   and debian testing (stretch) and unstable (sid).
>>
>>   As Thomas said, it's dirty, but we really need this.
> 
> Hi Arnout.
> Yes, i couldn't figure out a cleaner way to do this, continues below...
> 
>>   The only reason to use APPLY_PATCHES is that it updates .applied_patches_list,
>> right? In that case, perhaps it's better to do that directly here. So instead of
>> a dry-run, just apply the patch right away, and if it succeeds add it to
>> .applied_patches_list.
> 
> I could have probed the timeconst.pl file presence and relevancy (if it contains
> defined(@array)...), but then a dry-run patch does that for me just fine with
> the proper parameters.

 My thoughts exactly.

> Yes, that's the reason for APPLY_PATCHES, however since it's already there why
> bother duplicating code?

 Well, you're already duplicating code because you first to patch and then
APPLY_PATCHES. My idea was something like:

	$(Q)cd $(@D); patch -p1 -f -s >/dev/null 2>&1 \
		< $(LINUX_PKGDIR)/0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional \
		&& echo 0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional >
.applied_patches_list

 But really it's equally hacky so it doesn't matter much. Actually, my version
is especially hacky because it relies on the fact that the patch has a single
hunk so it will always pass or fail atomically.

> Also thought of extending apply-patches.sh to add an option for try-do/discard,
> but i believe it's a double-edged sword and not worth it quite yet.

 I considered that as well, like making it handle *.conditional this way. But
since this is the only patch for the time being, it's a bit redundant...


 Bottom line: keep it as it is.

 Regards,
 Arnout

> 
>>   This is a patch that we probably _do_ want to apply even in case of
>> OVERRIDE_SRCDIR. So maybe add it to LINUX_PRE_CONFIGURE_HOOKS instead. Even
>> though that's even more of a hack (and conflicts with the out-of-tree build
>> support).
> 
> Possibly, let's hear other people thoughts on this.
> Regards.
> 

-- 
Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
Essensium, Mind division . . . . . . . . . . . . . . http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium . . . . . BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

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

* [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl
  2016-02-10  0:37     ` Arnout Vandecappelle
@ 2016-02-10  1:27       ` Gustavo Zacarias
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo Zacarias @ 2016-02-10  1:27 UTC (permalink / raw)
  To: buildroot

On 09/02/16 21:37, Arnout Vandecappelle wrote:

>   Well, you're already duplicating code because you first to patch and then
> APPLY_PATCHES. My idea was something like:
>
> 	$(Q)cd $(@D); patch -p1 -f -s >/dev/null 2>&1 \
> 		< $(LINUX_PKGDIR)/0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional \
> 		&& echo 0001-timeconst.pl-Eliminate-Perl-warning.patch.conditional >
> .applied_patches_list
>
>   But really it's equally hacky so it doesn't matter much. Actually, my version
> is especially hacky because it relies on the fact that the patch has a single
> hunk so it will always pass or fail atomically.

Remember that this will leave a reject file behind for the case where 
the kernel is already patched (few releases, but still...)

>   I considered that as well, like making it handle *.conditional this way. But
> since this is the only patch for the time being, it's a bit redundant...
>
>
>   Bottom line: keep it as it is.

If the need arises with one or two more patches like this then yes, we 
can add some *.patch.maybe logic (personally i wouldn't call it 
conditional since a condition must be met, though 'maybe' sucks as well).
Regards.

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

* [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl
  2016-02-09 23:08 ` Arnout Vandecappelle
  2016-02-09 23:19   ` Gustavo Zacarias
@ 2016-02-10  7:11   ` Peter Korsgaard
  1 sibling, 0 replies; 8+ messages in thread
From: Peter Korsgaard @ 2016-02-10  7:11 UTC (permalink / raw)
  To: buildroot

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >> +LINUX_POST_PATCH_HOOKS += LINUX_TRY_PATCH_TIMECONST

 >  This is a patch that we probably _do_ want to apply even in case of
 > OVERRIDE_SRCDIR. So maybe add it to LINUX_PRE_CONFIGURE_HOOKS instead. Even
 > though that's even more of a hack (and conflicts with the out-of-tree build
 > support).

We have so far said that it is the responsibility of the user overriding
to handle all patches they need, and it imho makes sense here as
well. If they build the kernel manually outside buildroot they also need
to patch this.

-- 
Bye, Peter Korsgaard

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

* [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl
  2016-02-07 13:41 [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl gustavo.zacarias at free-electrons.com
  2016-02-07 14:50 ` Thomas Petazzoni
  2016-02-09 23:08 ` Arnout Vandecappelle
@ 2016-02-10  7:12 ` Peter Korsgaard
  2 siblings, 0 replies; 8+ messages in thread
From: Peter Korsgaard @ 2016-02-10  7:12 UTC (permalink / raw)
  To: buildroot

>>>>> "gustavo" == gustavo zacarias <gustavo.zacarias@free-electrons.com> writes:

 > From: Gustavo Zacarias <gustavo.zacarias@free-electrons.com>
 > Kernels older than 3.9 (not counting stable releases) used the
 > timeconst.pl perl script for their build process.
 > The problem with this script is that it used deprecated perl features,
 > namely defined(@array) which was removed for the perl 5.22 release,
 > causing build failure of older kernels on newer distributions.

 > To fix this instead of going the hard way (moving to the new
 > timeconst.bc script) use the easy way by patching timeconst.pl with an
 > upstream patch used for stable releases.

 > First try a dry-run on the patch to see if it applies, if it does then
 > call a proper APPLY_PATCHES to it.

 > Tested against an arbitrary 2.6.30 kernel (applies and builds), against
 > 4.4.1 for a missing timeconst.pl (does not apply since it's missing) and
 > 3.8.13 (does not apply since it's fixed already).

 > Known broken distributions: fedora 23.

 > Signed-off-by: Gustavo Zacarias <gustavo.zacarias@free-electrons.com>
 > ---
 > v2: rebase on top of master

Yes, I'm afraid we need this :/

Committed, thanks.

-- 
Bye, Peter Korsgaard

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

end of thread, other threads:[~2016-02-10  7:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-07 13:41 [Buildroot] [PATCH v2] linux: add conditional patch for timeconst.pl gustavo.zacarias at free-electrons.com
2016-02-07 14:50 ` Thomas Petazzoni
2016-02-09 23:08 ` Arnout Vandecappelle
2016-02-09 23:19   ` Gustavo Zacarias
2016-02-10  0:37     ` Arnout Vandecappelle
2016-02-10  1:27       ` Gustavo Zacarias
2016-02-10  7:11   ` Peter Korsgaard
2016-02-10  7:12 ` Peter Korsgaard

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.