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

 
 

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

Submitter:  Josh Boyer <jwboyer>
Submitted:  Mon 12 Jan 2009 01:23:05 PM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

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>
Group 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>
Group 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>
Group 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>
Group Member

 

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

Attach Files:
   
   
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
  • -email is unavailable- added by agruen (Posted a comment)
  • -email is unavailable- added by khali (Posted a comment)
  • -email is unavailable- added by jwboyer (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-06-17 agruen StatusNone Fixed
        Open/ClosedOpen Closed
    2009-06-11 khali Attached File- Added quilt-revert-should-not-hard-link.patch, #18248

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code