Bug 234822

Summary: [RHEL5]patch doesn't backup files properly
Product: Red Hat Enterprise Linux 5 Reporter: Issue Tracker <tao>
Component: patchAssignee: Tim Waugh <twaugh>
Status: CLOSED ERRATA QA Contact: Brock Organ <borgan>
Severity: low Docs Contact:
Priority: low    
Version: 5.0CC: syeghiay, tao
Target Milestone: ---Keywords: Reopened
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-09-03 08:56:25 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 234825    
Attachments:
Description Flags
patch-posix-backup.patch none

Description Issue Tracker 2007-04-02 13:08:21 UTC
Escalated to Bugzilla from IssueTracker

Comment 1 Issue Tracker 2007-04-02 13:08:26 UTC
Same problem may also be in patch for RHEL5.

When one file name appears twice in a patch file, then patch doesn't backup the file properly. 

This really confuses quilt.

16:36 <grondo> I have a question. Does this seem like a bug?
16:37 <grondo> I have a patch that lists the same file twice, as if two dependent patches were concatenated together. So the first hunk patches the file, and then the second hunk patches it again, with context sensitive to the first hunk.
16:38 <neb> yeah that is kind of normal. The kernel team does that all the time.
16:38 <grondo> when I use patch --backup, the backup file is not the original it is the file + the first hunk/ So the sequence seems to be, backup file; apply first hunk; backup newly patched file, apply second hunk. Does that seem like a bug in patch?
16:39 <grondo> It *really* confuses quilt
16:41 <grondo> grondo@wopri /tmp/grondo >cat patchtest
16:41 <grondo> This is text in the original file.
16:41 <grondo> grondo@wopri /tmp/grondo >cat patchtest.patch
16:41 <grondo> --- patchtest   2007-02-07 16:31:15.000000000 -0800
16:41 <grondo> +++ patchtest.2 2007-02-07 16:31:41.000000000 -0800
16:41 <grondo> @@ -1 +1,3 @@ This is text in the original file.
16:41 <grondo> +
16:41 <grondo> +This is text added by patch1.
16:41 <grondo> --- patchtest   2007-02-07 16:31:41.000000000 -0800
16:41 <grondo> +++ patchtest   2007-02-07 16:32:09.000000000 -0800
16:41 <grondo> @@ -1,3 +1,6 @@
16:41 <grondo> +This is text added by patch2.
16:41 <grondo> + This is text in the original file.
16:41 <grondo> This is text added by patch1.
16:41 <grondo> +
16:41 <grondo> grondo@wopri /tmp/grondo >patch --backup -p0 < patchtest.patch
16:41 <grondo> patching file patchtest
16:41 <grondo> patching file patchtest
16:41 <grondo> grondo@wopri /tmp/grondo >cat patchtest.orig
16:41 <grondo> This is text in the original file.
16:41 <grondo> This is text added by patch1.
16:42 <neb> that is a pretty tough call. I would say that it is a bug but I'd be willing to be argued away from that point of view. In some ways it sounds like a malformed patch file.
16:43 <grondo> To me it seems like a bug, but I could understand how patch is just handling a stream of "patches" and in that stream are really two separate patches. But like you said, it is common (though this is first I've seen it)
16:43 <neb> I can almost visualize the logic in patch that doesn't work. OTOH I can also imagine that trying to fix that logic would be a pain in the ass.
16:44 <grondo> BTW, the particular patch that was giving me grief was linux-2.6.9-x8664-core.patch
16:45 <grondo> Quilt uses --backup to save a copy of the original file before patching. It can then refresh the patch by rediffing against the original
16:45 <neb> I was just in the process of suggesting that you rediff the patch
16:46 <grondo> However, since the original it is diffing against is now a half-modified file, you lose half the patch and your refreshed patch now has conflicts
16:46 <grondo> Yeah, but I only just figured out the problem
16:46 <neb> I imagine the logic is something like
16:46 <neb> read filename
16:46 <neb> copy file to file.orig
16:46 <neb> apply patch
16:47 <neb> loop back to next file
16:47 <neb> ...
16:47 <grondo> all they would have to do is cache the filenames that have already been backed up
16:47 <neb> What is happening that you hit the same file twice and so it overwrites the orig file that it just wrote.
16:47 <grondo> Yes, that is definitely what is happening
16:48 <neb> I'd have to think about it more but I think the caching the filenames that were already modified by patch and not recreating orig files for them might work.
This event sent from IssueTracker by jplans  [Support Engineering Group]
 issue 113310

Comment 2 Issue Tracker 2007-04-02 13:08:36 UTC
Action: Steps:
[ben@quince foo]$ vi file
[ben@quince foo]$ cat file
Original file
[ben@quince foo]$ cp file file.new
[ben@quince foo]$ vi file.new
[ben@quince foo]$ cat file.new
Original file
New line added
[ben@quince foo]$ diff -u file file.new > first.patch
[ben@quince foo]$ cp file.new file.newest
[ben@quince foo]$ vi file.newest
[ben@quince foo]$ cat file.newest
Original file
New line added
Another new line
[ben@quince foo]$ diff -u file.new file.newest > second.patch
[ben@quince foo]$ cat first.patch second.patch > combined.patch
[ben@quince foo]$ cat combined.patch
[ben@quince foo]$ cat first.patch second.patch
--- file        2007-02-09 11:53:00.000000000 -0800
+++ file.new    2007-02-09 11:53:42.000000000 -0800
@@ -1 +1,2 @@
 Original file
+New line added
--- file.new    2007-02-09 11:53:42.000000000 -0800
+++ file.newest 2007-02-09 11:55:04.000000000 -0800
@@ -1,2 +1,3 @@
 Original file
 New line added
+Another new line
[ben@quince foo]$ vi combined.patch
[ben@quince foo]$ cat combined.patch
--- file        2007-02-09 11:53:00.000000000 -0800
+++ file.new    2007-02-09 11:53:42.000000000 -0800
@@ -1 +1,2 @@
 Original file
+New line added
--- file        2007-02-09 11:53:42.000000000 -0800
+++ file.newest 2007-02-09 11:55:04.000000000 -0800
@@ -1,2 +1,3 @@
 Original file
 New line added
+Another new line
[ben@quince foo]$ cp file file.begin
[ben@quince foo]$ patch --backup < combined.patch
patching file file
patching file file
[ben@quince foo]$ cat file.orig
Original file
New line added
[ben@quince foo]$ cat file.begin
[ben@quince foo]$ cat file
Original file

So notice that the orig file represents the file after the first time that
file was patched not the original state of the file.


Issue escalated to Support Engineering Group by: kbaxley.
Internal Status set to 'Waiting on SEG'

This event sent from IssueTracker by jplans  [Support Engineering Group]
 issue 113310

Comment 12 RHEL Program Management 2007-06-05 20:32:36 UTC
This request was evaluated by Red Hat Product Management for
inclusion in a Red Hat Enterprise Linux release.  Since this
bugzilla is in a component that is not approved for the current
release, it has been closed with resolution deferred.  You may
reopen this bugzilla for consideration in the next release.

Comment 16 RHEL Program Management 2007-12-03 20:41:48 UTC
This request was evaluated by Red Hat Product Management for
inclusion, but this component is not scheduled to be updated in
the current Red Hat Enterprise Linux release.  This request will
be reviewed for a future Red Hat Enterprise Linux release.

Comment 18 Phil Knirsch 2008-04-30 15:55:15 UTC
Proposing for RHEL-5.3 and granting Devel ACK.

Read ya, Phil


Comment 21 Tim Waugh 2008-06-27 08:54:50 UTC
Created attachment 310414 [details]
patch-posix-backup.patch

Here is the patch I propose to add.  A version of this (rebased against the
alpha 2.5.9 version of patch) was sent to the upstream maintainer Paul Eggert
on June 16th.

Comment 28 errata-xmlrpc 2008-09-03 08:56:25 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2008-0844.html