Bug 811777 - ApplyPatch (kernel.spec) can't cope with patch names which are defined using environment variables
Summary: ApplyPatch (kernel.spec) can't cope with patch names which are defined using ...
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: kernel
Version: 17
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Kernel Maintainer List
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-04-12 00:13 UTC by Mr-4
Modified: 2012-04-17 15:13 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-04-17 15:13:53 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Mr-4 2012-04-12 00:13:08 UTC
Description of problem:
When I have something like this in my kernel.spec file:

Patch9999: some-patch-$__id.patch

and then use ApplyPatch further down my kernel.spec file, like so:

ApplyPatch some-patch-$__id.patch

bearing in mind that $__id is an environment variable (say __id=17), I get the following error when executing rpmbuild: 

ERROR: Patch  some-patch-17.patch not listed as a source patch in specfile

which is wrong as I have this defined and listed.

Version-Release number of selected component (if applicable):
3.3.1 version of the kernel, but I suspect this is also valid for previous kernel versions.

How reproducible:
Always

Steps to Reproduce:
1. Modify kernel.spec as indicated above and execute rpmbuild -bb
2.
3.
  
Actual results:
ERROR: Patch  some-patch-17.patch not listed as a source patch in specfile


Expected results:
The specified patch to be applied without much fuss.

Additional info:
I have a minor request to the kernel devs: Currently the ApplyPatch section has a little marker defining the end of the ApplyPatch sections - this marker is specified with "# END OF PATCH APPLICATIONS". 

I use that marker with sed to add custom patch applications at the end of that section, but there is no such marker within the PatchXXXX section of kernel.spec. 

It would be nice if one is added (like "# END OF PATCH DEFINITIONS") so that I could use sed to add custom PatchXXX definitions without having to edit kernel.spec manually. Thanks!

Comment 1 Josh Boyer 2012-04-12 01:32:39 UTC
(In reply to comment #0)
> Description of problem:
> When I have something like this in my kernel.spec file:
> 
> Patch9999: some-patch-$__id.patch
> 
> and then use ApplyPatch further down my kernel.spec file, like so:
> 
> ApplyPatch some-patch-$__id.patch
> 
> bearing in mind that $__id is an environment variable (say __id=17), I get the
> following error when executing rpmbuild: 
> 
> ERROR: Patch  some-patch-17.patch not listed as a source patch in specfile
> 
> which is wrong as I have this defined and listed.
> 
> Version-Release number of selected component (if applicable):
> 3.3.1 version of the kernel, but I suspect this is also valid for previous
> kernel versions.
> 
> How reproducible:
> Always
> 
> Steps to Reproduce:
> 1. Modify kernel.spec as indicated above and execute rpmbuild -bb
> 2.
> 3.
> 
> Actual results:
> ERROR: Patch  some-patch-17.patch not listed as a source patch in specfile

I'm honestly surprised you get that far.  Unless you have a file literally named some-patch-$__id.patch, RPM should error out saying something like:

error: Bad source: some-patch-$__id.patch: No such file or directory

You might be getting lucky in that RPM does something odd like evaluate functions first or something.  I have no idea.

E.g.

[jwboyer@vader dtc]$ grep Patch2 dtc.spec 
Patch2:         dtc-$__id.patch
[jwboyer@vader dtc]$ ls dtc-*.patch
dtc-14.patch  dtc-check.patch  dtc-flattree.patch
[jwboyer@vader dtc]$ export __id=14
[jwboyer@vader dtc]$ echo $__id
14
[jwboyer@vader dtc]$ rpmbuild --define '_sourcedir /home/jwboyer/src/fedora/dtc' --define '_specdir /home/jwboyer/src/fedora/dtc' --define '_builddir /home/jwboyer/src/fedora/dtc' --define '_srcrpmdir /home/jwboyer/src/fedora/dtc' --define '_rpmdir /home/jwboyer/src/fedora/dtc' --define 'dist .fc18' --define 'fedora 18' --define 'fc18 1' --nodeps -bb ./dtc.spec
error: Bad source: /home/jwboyer/src/fedora/dtc/dtc-$__id.patch: No such file or directory
[jwboyer@vader dtc]$ touch dtc-\$__id.patch
[jwboyer@vader dtc]$ ls dtc-*.patch
dtc-14.patch  dtc-check.patch  dtc-flattree.patch  dtc-$__id.patch
[jwboyer@vader dtc]$ rpmbuild --define '_sourcedir /home/jwboyer/src/fedora/dtc' --define '_specdir /home/jwboyer/src/fedora/dtc' --define '_builddir /home/jwboyer/src/fedora/dtc' --define '_srcrpmdir /home/jwboyer/src/fedora/dtc' --define '_rpmdir /home/jwboyer/src/fedora/dtc' --define 'dist .fc18' --define 'fedora 18' --define 'fc18 1' --nodeps -bs ./dtc.spec
error: File /home/jwboyer/src/fedora/dtc/dtc-$__id.patch is smaller than 13 bytes
Wrote: /home/jwboyer/src/fedora/dtc/dtc-1.3.0-4.fc18.src.rpm
[jwboyer@vader dtc]$ rpm -qpl dtc-1.3.0-4.fc18.src.rpm 
dtc-$__id.patch
dtc-check.patch
dtc-flattree.patch
dtc-v1.3.0.tgz
dtc.spec
[jwboyer@vader dtc]$ 

If you do a -bb instead of -bp, it will build the package using dtc-14.patch, but you can see the SRPM from -bp is created with the literal dtc-$__id.patch.

> Expected results:
> The specified patch to be applied without much fuss.

That seems more like something to file against RPM itself given the above.

> Additional info:
> I have a minor request to the kernel devs: Currently the ApplyPatch section has
> a little marker defining the end of the ApplyPatch sections - this marker is
> specified with "# END OF PATCH APPLICATIONS". 
> 
> I use that marker with sed to add custom patch applications at the end of that
> section, but there is no such marker within the PatchXXXX section of
> kernel.spec. 
> 
> It would be nice if one is added (like "# END OF PATCH DEFINITIONS") so that I
> could use sed to add custom PatchXXX definitions without having to edit
> kernel.spec manually. Thanks!

That we can probably add.

Comment 2 Mr-4 2012-04-12 11:14:56 UTC
Thanks Josh, that was ... interesting! 

I always used shell environment variables (for one reason or another) in .spec files without encountering much problems, though, I have to admit I never used them as part of specifying patches or source files.

I also guess I was lucky in a way that I have not used -bp when executing rpmbuild.

The main reason for using shell variables is that I have a semi-automated process of building various custom images - which include building the kernel itself - and distribute them remotely to the appropriate machines (the "semi" part comes from me having to make manual adjustments to kernel.spec as I specified in my original report - I note in your response that this part could be eliminated as well, for which I am very grateful).

As part of that process I have to use shell variables (mainly to specify version numbers of various custom patches I have, depending on the target used for the build), therefore I would need this functionality. 

The problem, as indicated in the original report, seems to be in the following few lines in the ApplyPatch subroutine:

%if !%{using_upstream_branch}
  if ! grep -E "^Patch[0-9]+: $patch\$" %{_specdir}/${RPM_PACKAGE_NAME%%%%%{?variant}}.spec ; then
    if [ "${patch:0:8}" != "patch-3." ] ; then
      echo "ERROR: Patch  $patch  not listed as a source patch in specfile"
      exit 1
    fi
  fi 2>/dev/null
%endif

Once I comment out these lines (these are just checks and I am confident I'll pass those ;) ) the build succeeds, though, admittedly, I never used the -bp option (haven't tried -bs either).

So, what would you suggest my next step should be - could this be fixed in the kernel.spec, do you wish me to report this as RPM bug or do we do both?

Comment 3 Mr-4 2012-04-12 11:27:43 UTC
Apologies, I forgot to mention something else - as part of the kernel build, if I have to use menuconfig or oldconfig targets, I have to also add a small comment at the top of .config which indicates the target arch I am building for (like "i386" for all ix86 targets, or x86_64 for all 64-bit targets).

Is this truly necessary/needed? Could this be eliminated as it is a bit of a pain to add this every time I use either menuconfig or oldconfig?

I realise this has nothing to do with the bug I have submitted above, but I am hoping that you could shed some clarity on this as it is another rather annoying "feature" of the kernel build process I am seeking to eliminate. Thanks!

Comment 4 Josh Boyer 2012-04-12 14:09:55 UTC
(In reply to comment #2)
> So, what would you suggest my next step should be - could this be fixed in the
> kernel.spec, do you wish me to report this as RPM bug or do we do both?

If you really want to pursue this, I would suggest opening a new bug againt RPM.  We're not going to remove the lines from kernel.spec, but we'll use this bug to add the # END OF PATCH DEFINITIONS part.

(In reply to comment #3)
> Apologies, I forgot to mention something else - as part of the kernel build, if
> I have to use menuconfig or oldconfig targets, I have to also add a small
> comment at the top of .config which indicates the target arch I am building for
> (like "i386" for all ix86 targets, or x86_64 for all 64-bit targets).
> 
> Is this truly necessary/needed? Could this be eliminated as it is a bit of a
> pain to add this every time I use either menuconfig or oldconfig?
> 
> I realise this has nothing to do with the bug I have submitted above, but I am
> hoping that you could shed some clarity on this as it is another rather
> annoying "feature" of the kernel build process I am seeking to eliminate.
> Thanks!

I'm not sure exactly why you need to use oldconfig, as it's run in the spec already.  If you just need to add or override some config options you can use the existing config-local file in the SRPM.  That is intentionally left blank so people can put their personal config choices in there and it will get merged with the generic and arch specific config files.

Comment 5 Mr-4 2012-04-12 14:33:09 UTC
(In reply to comment #4)
> If you really want to pursue this, I would suggest opening a new bug againt
> RPM.  We're not going to remove the lines from kernel.spec, but we'll use this
> bug to add the # END OF PATCH DEFINITIONS part.
Yep, I'll probably do that, thanks.

> I'm not sure exactly why you need to use oldconfig, as it's run in the spec
> already.  If you just need to add or override some config options you can use
> the existing config-local file in the SRPM.  That is intentionally left blank
> so people can put their personal config choices in there and it will get merged
> with the generic and arch specific config files.
The problem is that I occasionally use the .config from previous versions of the kernel (say 3.2) to build a proper/current one (for, say, 3.3) using "make oldconfig" and enter/set the missing option values. Once that new .config file is made current I stash it away for future use. Of course I also use this as config-local as well.

The problem is that even if I have "# <destination-arch>" at the very top of this file, this is promptly erased as soon as I run either "make oldconfig" or "make config" so I have to add it manually as soon as that make execution is complete. 

This is provided I still need to have "# <destination-arch>" at the top of .config. If that is no longer the case then my request is a bit pointless and this whole debate is a storm in a teacup so to speak.

Comment 6 Josh Boyer 2012-04-17 15:13:53 UTC
(In reply to comment #4)
> but we'll use this bug to add the # END OF PATCH DEFINITIONS part.

OK.  I've added that tag to the spec on all branches now.


Note You need to log in before you can comment on or make changes to this bug.