bugQuilt - Bugs: bug #55839, push and pop can leave empty...

 
 

bug #55839: push and pop can leave empty directories behind

Submitter:  Filipus Klutiero <chealer>
Submitted:  Tue 05 Mar 2019 08:07:15 PM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Need Info
Privacy:  Public Assigned to:  khali
Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

Fri 05 Apr 2019 02:51:05 PM UTC, comment #3: 

Replying to comment #2:
A directory created by Quilt which is not empty when the patch which created it is unapplied should not be silently removed. Ideally, the user should be asked about the proper solution. That is what Subversion 1.9.5 does. It reports a conflicts and asks how to solve.

 Filipus Klutiero <chealer>
Thu 07 Mar 2019 10:42:29 AM UTC, comment #2: 

Another point to be clarified is, what to do if a directory which has been created as a result of "quilt push" is not empty after "quilt pop"?

For example, "quilt push" creates a new C source file, and the user builds it, creating an object file in the same directory. "quilt pop" will remove the C source file but not the object file. I don't fell comfortable deleting arbitrary files as the result of "quilt pop". You could end up deleting valuable files (for example a new source file the user intended to add to the patch but forgot to).

I wonder how subversion and other SCM tools deal with this situation.

 Jean Delvare <khali>
Group administrator
Thu 07 Mar 2019 10:36:20 AM UTC, comment #1: 

This is true.

Unfortunately, quilt doesn't deal with directories at all. It delegates the patching (quilt push) to "patch". The unpatching (quilt pop) is achieved with "backup-files -r", which itself depends on the backup generated by "patch" earlier. Both are dealing with files, not directories.

I see that "patch" will silently delete directories which are empty after a patch has been applied (or reverted). Maybe we could instruct "backup-files" to do the same, for the sake of consistency. However this would not reach your ideal of idempotency, because an empty directory would be removed even if it preexisted "quilt push". This would also come at the cost of a performance penalty. We may have to make it optional, also because not everybody may want that new behavior.

Actively tracking when a directory has been created as the result of "quilt push" looks non-trivial to me. We would basically have to call "quilt files" on each patch before applying it, compute a list of candidate directories from the list of files, check if each directory already exists, and if not, store the information somewhere under .pc/. After that, deleting the directory on "quilt pop" should be fairly easy. I'm worried about the performance penalty on "quilt push" though. Doesn't matter much for a single patch, but think of "quilt push -a" with a huge patch series...

 Jean Delvare <khali>
Group administrator
Tue 05 Mar 2019 08:07:15 PM UTC, original submission:  

Directory management appears to be implicit with Quilt. In other words, a directory is created automatically when a file it contains is created, not explicitly.

I can see that this may simplify the tool, but the fact that quilt push; followed by quilt pop; is not idempotent can be surprising, in particular for people used to version control systems. With Subversion, if I update from revision 1000 to revision 1001, which creates a directory, when I go back to revision 1000, the directory is removed.

$ file test
test: cannot open `test' (No such file or directory)

cloph2@PDP0088824 /cygdrive/c/xampp/htdocs/tiki/MERN18
$ cat patches/test_directories.diff
Index: MERN18/test/foo.txt
===================================================================
--- /dev/null
+++ MERN18/test/foo.txt
@@ -0,0 +1 @@
+yo
\ No newline at end of file

cloph2@PDP0088824 /cygdrive/c/xampp/htdocs/tiki/MERN18
$ quilt push
Applying patch patches/test_directories.diff
patching file test/foo.txt

Now at patch patches/test_directories.diff

cloph2@PDP0088824 /cygdrive/c/xampp/htdocs/tiki/MERN18
$ quilt pop
Removing patch patches/test_directories.diff
Removing test/foo.txt

Now at patch patches/cache_control_no_unset.diff

cloph2@PDP0088824 /cygdrive/c/xampp/htdocs/tiki/MERN18
$ file test
test: directory

cloph2@PDP0088824 /cygdrive/c/xampp/htdocs/tiki/MERN18
$


I am not sure what the best solution is, but if management remains implicit, I would suggest that Quilt track its directory creations, so that if directory foo/ is created when applying foo.diff, when foo.diff is unapplied, either:
1. foo/ is removed
2. Some warning is printed telling the user that the foo/ directory was left

 Filipus Klutiero <chealer>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by khali (Posted a comment)
  • -email is unavailable- added by chealer (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
    2019-03-12 khali StatusConfirmed Need Info
    2019-03-07 khali StatusNone Confirmed
        Assigned toNone khali

    Back to the top

    Copyright © 2024 Free Software Foundation, Inc.
    Verbatim copying and distribution of this entire article is permitted in any medium, provided this notice is preserved.
    The Levitating, Meditating, Flute-playing Gnu logo is a GNU GPL'ed image provided by the Nevrax Design Team.
    Page source code

    Powered by Savane 3.13-461b.
    Corresponding source code