Bug 773503 - rpmbuild silents patch which may cause confusion
Summary: rpmbuild silents patch which may cause confusion
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: rpm
Version: 6.4
Hardware: Unspecified
OS: Unspecified
low
low
Target Milestone: rc
: ---
Assignee: Panu Matilainen
QA Contact: Patrik Kis
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-01-12 01:56 UTC by Jan Pokorný [poki]
Modified: 2014-01-28 03:22 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of: 678000
Environment:
Last Closed: 2013-02-21 10:51:13 UTC


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2013:0461 normal SHIPPED_LIVE rpm bug fix and enhancement update 2013-02-20 21:07:23 UTC
Red Hat Bugzilla 471005 None CLOSED Allow rpmbuild user to prevent application of hunks into wrong contexts 2019-08-18 22:30:25 UTC

Internal Links: 471005

Description Jan Pokorný [poki] 2012-01-12 01:56:05 UTC
I've just ran into similar problem as described in bug 678000 (thus
this is to be its effective clone).  It was while making brew build
for RHEL 5.8, but the same is with the current Fedora and I don't think
anything has changed for RHEL 6 so I am targetting this bug/proposal
to be considered for RHEL 6.4 (and perhaps RHEL 5.9) or later.


Let's have a look what can be a successful application of a patch
when rpmbuild'ing for RHEL 6 (hopefully I covered all the possibilities):

  level of match    | ``patch -s --fuzz=0'' patch application (RHEL 6)
--------------------+-----------------------------------------------------
 A - fuzz=0 match   |
   - hunks start    | perfect application: everything OK, no side effects
     lines match    |
--------------------+-----------------------------------------------------
 B - fuzz=0 match   | successful application, everything seems OK, but
   - hunk start     | there is a side efect: creating <patched>.orig
     line w/ offset | without any indication (due to ``-s'' option)
--------------------+-----------------------------------------------------

AFAIK there is no other variant of successful patch application when
using mentioned flags to ``patch'' as enforced by rpmbuild internals
(I noticed ``%patch1'' in spec generally does not support all
the options ``patch'' does).

On RHEL 5 (5.8), ``--fuzz=0'' is not enforced meaning that default
fuzz boundary (2) is used instead so there would be another 0<fuzz<3
entry (regardless whether hunk start line matches exactly or not)
in the table, call this variant C, with the same description as B.

Apparently, ideal state is when the patch matches as in case A.
The case of B (C) will, due to partial mismatch, force creating that
backup file (with trailing ``.orig'' by default) while remaining silent
due to enforced ``-s''.  Undesirably, such unnoticed file can then find
its way to the final package (fortunately, rpmdiff can spot this);
just a too liberal ``make install'' in a patched 3rd package without
filtering out artificial objects and <patched>.orig is brought in.


Now the question is whether there would be a profit in restricting
patches into the well-fitting A-case only or not.  As ``patch'' itself
seems not to provide any mean for such spartan restriction, there would
have to be a wrapper around ``patch'' on the rpmbuild's side that would
fail either with ``patch'' failure or if the <patched>.orig file has been
generated (or change in ``patch'' upstream would be feasibe?).

If this is not the right way (B/C also OK), I would join original issuer
from bug 678000 in suggesting removal of ``-s'' option for ``patch''
invocations by rpmbuild.


Either would make the packaging a bit more pleasant and less error-prone.
In either case, one would get immediate feedback that something is wrong
when running ``rpmbuild -bp'' locally and when the overall build has
failed (case A) or at least from build.log stating that the patch hasn't
been applied cleanly (B+C cases).

Thanks for considering this!

Comment 2 Jan Pokorný [poki] 2012-01-12 02:01:36 UTC
s/3rd package/3rd party package/

Comment 3 Panu Matilainen 2012-03-02 10:07:29 UTC
Moving to rhel-6.4 for further consideration, this isn't exactly a critical issue.
Also you can already override %_default_patch_flags to remove the -s from %patch invocations.

Comment 4 Panu Matilainen 2012-08-02 15:29:54 UTC
FWIW, default changed to non-silent in upstream now.

Comment 5 Jan Pokorný [poki] 2012-08-02 17:15:49 UTC
Thanks.  Any chance to get RPM patching process to be completely sane
(perfect match or fail/break the build) as expressed in [comment 0]?

Please note, IIRC, that is a default behavior of git-patch, and git
is known to do the right things :)

Comment 6 Panu Matilainen 2012-08-03 13:00:46 UTC
Rpm already defaults to zero fuzz tolerance, but AFAIK patch doesn't have a "strict mode" beyond that. I'd be happy to support it in rpm if it did...

Comment 7 Jan Pokorný [poki] 2012-08-07 19:21:42 UTC
Yep, just noticed the brewtap tool and its "warning" assesment when
nonzero fuzz is used to build a RHEL6+ package ([bug 692142]).

Comment 8 RHEL Product and Program Management 2012-08-14 22:00:06 UTC
This request was evaluated by Red Hat Product Management for
inclusion in a Red Hat Enterprise Linux release.  Product
Management has requested further review of this request by
Red Hat Engineering, for potential inclusion in a Red Hat
Enterprise Linux release for currently deployed products.
This request is not yet committed for inclusion in a release.

Comment 9 Jan Pokorný [poki] 2012-08-22 18:12:35 UTC
Related issue, patch tool used by RPM does not recognize GIT binary
patches, but it does not complain about this neither(!) [1,2]

Simply, there should be much higher guarantees that nothing goes
wrong with the patches because this patch-based model is so vital.

I also filed a RFE for BrewTap (perhaps indirectly Rpmdiff):
[bug 850918].

[1] http://pkgs.devel.redhat.com/cgit/rpms/conga/tree/bz786372bz607179.patch?h=rhel-5.9&id=6feeb5279021a436ba0ec6a7be3a4a48484aeddd#n1391
[2] http://download.devel.redhat.com/brewroot/packages/conga/0.12.2/60.el5/data/logs/x86_64/build.log
    (scroll down a bit to "Patch #68 (bz786372bz607179.patch):")

Comment 10 Jan Pokorný [poki] 2012-08-22 18:44:47 UTC
Btw. it seems that we are quite behind the upstream regarding "patch."

See 2 years old upstream commit [3]; I tried the attached test case
and at both RHEL 5.8 and F17, I am getting:

$ patch -p1  < f.diff
patch: **** Only garbage was found in the patch input.

If at least the referenced commit was applied at our side, I would be
warned in the mentioned case (one of possible desired outcomes) :-/

Comment 11 Karel Srot 2012-08-23 11:24:10 UTC
I am a bit lost here. Could you please summarize the changes planned for fixing this bug? Also please update severity/priority. Thank you.

Comment 12 Panu Matilainen 2012-08-23 12:17:27 UTC
The change to rpm in its entirety is this one-line change to the default macros :)
http://rpm.org/gitweb?p=rpm.git;a=commitdiff;h=933a3e32dd94c5d54d10c24c55394488824de1f0

Comment 13 Jan Pokorný [poki] 2012-08-23 13:26:13 UTC
To avoid described issues, it seems most viable to me the solution
described at [3] I was indirectly pointed to.
Unfortunately and as a bit of a surprise, RHEL 5 does not offer git
directly (requested with [bug 851184]).
In the longer term, the solution to [4] looks promising, thanks
Panu.

[3] http://rwmj.wordpress.com/2011/08/09/nice-rpm-git-patch-management-trick/
[4] http://www.rpm.org/ticket/54

Comment 20 errata-xmlrpc 2013-02-21 10:51:13 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2013-0461.html


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