bugQuilt - Bugs: bug #25305, quilt revert does not appear to be...

 
 

bug #25305: quilt revert does not appear to be working properly

Submitted by:  Josh Boyer <jwboyer>
Submitted on:  Mon 12 Jan 2009 01:23:05 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: None
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Wed 17 Jun 2009 11:04:01 AM UTC, comment #3:

Proposed patch added, and I tried to clarify the help text. Thanks!

Andreas Gruenbacher <agruen>
Project Administrator
Thu 11 Jun 2009 07:23:49 AM UTC, comment #2:

Point 5 in the original report is valid, "quilt revert" accidentally creates a hard link, it shouldn't do that.

I think the reason is that apply_patch_temporarily() uses backup-files to create the temporary files, and by default backup-files uses hard links. For files which are modified by the patch, this isn't a problem because patch will unlink the file before modifying it. But for files which are not modified by the patch (as is the case of a.txt in the example), the hard link is preserved.

Then quilt revert also makes a hard link to restore the file, on the assumption that the temporary copy will be deleted right away. At this point, files not modified by the patch will have a link count of 3 (.pc/<patch>/<file>, d.*/<file> and <file>). When the temporary file is deleted, the link count is still 2 (.pc/<patch>/<file> and <file>).

A simple way to fix this is to always use cp instead of ln when restoring the files. Performance is not as good, but at least it is always correct. We could optimize the code to use ln when possible and cp for the other cases, but honestly I don't think it is worth the extra complexity. The speed of "quilt revert" is hardly critical, given how infrequently this command is used.

I have a fix, including a new test case. I'll attach it.

Jean Delvare <khali>
Project Administrator
Wed 10 Jun 2009 04:56:27 PM UTC, comment #1:

My understanding is that point 2 in the original report is the expected result. "quilt revert" reverts unrefreshed changes. Basically that's the same as "quilt pop -f" followed by "quilt push" but for just one file. So by definition, calling "quilt revert" right after "quilt refresh" is a no-op.

I can't disagree that documentation could be made clearer about the purpose of "quilt revert". Care to submit a patch?

Reverting all changes to a specified file used to be done by "quilt remove". This command was unfortunately removed some versions ago, while it was useful, and a number of people have asked for it to be restored [1]. I think we should restore it.

[1] http://lists.nongnu.org/archive/html/quilt-dev/2009-03/msg00000.html

Jean Delvare <khali>
Project Administrator
Mon 12 Jan 2009 01:23:05 PM UTC, original submission:

See https://bugzilla.redhat.com/show_bug.cgi?id=478586 for a fairly complete write up of the problem.

One thing I don't fully understand is if quilt revert is supposed to revert unrefreshed changes, or if it is supposed to revert all changes to the specified file that are in <foo>.patch.

Some clarification in the man page might help with that.

Josh Boyer <jwboyer>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #18248:  quilt-revert-should-not-hard-link.patch added by khali (2KiB - message/rfc822 - Proposed fix)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by agruen (Posted a comment)
  • -unavailable- added by khali (Posted a comment)
  • -unavailable- added by jwboyer (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 17 Jun 2009 11:04:01 AM UTCagruenStatusNone=>Fixed
      Open/ClosedOpen=>Closed
    Thu 11 Jun 2009 07:53:35 AM UTCkhaliAttached File-=>Added quilt-revert-should-not-hard-link.patch, #18248

    Back to the top


    Powered by Savane 3.1-cleanup1