{"thread":{"id":"22160","subject":"[PATCH] Remove empty directories when checking out a commit with fewer submodules","startedAt":"2010-01-11T02:59:54Z","lastAt":"2010-01-11T09:53:31Z","messageCount":5,"participants":["Peter Collingbourne","Johannes Schindelin","Johan Herland","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"131251","messageId":"1263178794-3140-1-git-send-email-peter@pcc.me.uk","threadId":"22160","inReplyTo":null,"subject":"[PATCH] Remove empty directories when checking out a commit with fewer submodules","fromName":"Peter Collingbourne","fromEmail":"peter@pcc.me.uk","sentAt":"2010-01-11T02:59:54Z","receivedAt":"2010-01-11T02:59:54Z","isPatch":true,"sender":{"key":"peter@pcc.me.uk","avatar":"https://avatars.githubusercontent.com/u/425024?v=4"},"body":"Change the unlink_entry function to use rmdir to remove submodule\ndirectories.  Currently we try to use unlink, which will never succeed.\n\nOf course rmdir will only succeed for empty (i.e. not checked out)\nsubmodule directories.  Behaviour if a submodule is checked out stays\nessentially the same: print a warning message and keep the submodule\ndirectory.\n\nSigned-off-by: Peter Collingbourne <peter@pcc.me.uk>\n---\n t/t7400-submodule-basic.sh |    9 +++++++++\n unpack-trees.c             |   12 ++++++++++--\n 2 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex a0cc99a..1a4dc5f 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -299,6 +299,15 @@ test_expect_success 'ls-files gracefully handles trailing slash' '\n \n '\n \n+test_expect_success 'moving to a commit without submodule does not leave empty dir' '\n+\trm -rf init &&\n+\tmkdir init &&\n+\tgit reset --hard &&\n+\tgit checkout initial &&\n+\ttest ! -d init &&\n+\tgit checkout second\n+'\n+\n test_expect_success 'submodule <invalid-path> warns' '\n \n \tgit submodule no-such-submodule 2> output.err &&\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex dd5999c..b69847d 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -61,8 +61,16 @@ static void unlink_entry(struct cache_entry *ce)\n {\n \tif (has_symlink_or_noent_leading_path(ce->name, ce_namelen(ce)))\n \t\treturn;\n-\tif (unlink_or_warn(ce->name))\n-\t\treturn;\n+\tif (S_ISGITLINK(ce->ce_mode)) {\n+\t\tif (rmdir(ce->name)) {\n+\t\t\twarning(\"unable to rmdir %s: %s\",\n+\t\t\t\tce->name, strerror(errno));\n+\t\t\treturn;\n+\t\t}\n+\t}\n+\telse\n+\t\tif (unlink_or_warn(ce->name))\n+\t\t\treturn;\n \tschedule_dir_for_removal(ce->name, ce_namelen(ce));\n }\n \n-- \n1.6.5\n"},{"id":"131274","messageId":"alpine.DEB.1.00.1001110954410.4985@pacific.mpi-cbg.de","threadId":"22160","inReplyTo":"1263178794-3140-1-git-send-email-peter@pcc.me.uk","subject":"Re: [PATCH] Remove empty directories when checking out a commit with fewer submodules","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-01-11T08:57:15Z","receivedAt":"2010-01-11T08:57:15Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 11 Jan 2010, Peter Collingbourne wrote:\n\n> Change the unlink_entry function to use rmdir to remove submodule\n> directories.\n\nNAK.  We should not even try to _unlink_ submodule subdirectories; it \nwould be _way_ too easy to lose data that way.  Remember, submodules are a \ntotally different beast from regular files.  They can contain valuable, \nyet uncommitted data, that is not even meant to be committed.\n\nSo you say if the submodule directories are empty, it is safe?  Not so.  \nThey will never be empty: there is always .git/, and _that_ can contain \nvaluable information that you do not want to throw away, too.  Think of \nunpushed branches, for example.  That would be _fatal_ if you rmdir() that \nfor me.\n\nSo please, no,\nDscho\n"},{"id":"131276","messageId":"201001111032.45637.johan@herland.net","threadId":"22160","inReplyTo":"alpine.DEB.1.00.1001110954410.4985@pacific.mpi-cbg.de","subject":"Re: [PATCH] Remove empty directories when checking out a commit with fewer submodules","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2010-01-11T09:32:45Z","receivedAt":"2010-01-11T09:32:45Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Monday 11 January 2010, Johannes Schindelin wrote:\n> Hi,\n>\n> On Mon, 11 Jan 2010, Peter Collingbourne wrote:\n> > Change the unlink_entry function to use rmdir to remove submodule\n> > directories.\n>\n> NAK.  We should not even try to _unlink_ submodule subdirectories; it\n> would be _way_ too easy to lose data that way.  Remember, submodules\n> are a totally different beast from regular files.  They can contain\n> valuable, yet uncommitted data, that is not even meant to be\n> committed.\n>\n> So you say if the submodule directories are empty, it is safe?  Not\n> so. They will never be empty: there is always .git/, and _that_ can\n> contain valuable information that you do not want to throw away, too.\n>  Think of unpushed branches, for example.  That would be _fatal_ if\n> you rmdir() that for me.\n>\n> So please, no,\n\nI believe what Peter is referring to is the _empty_ directories (and \nthat includes no .git/) that are placeholders for submodules that are \ndeliberately not cloned/checked out. This lets you do things like:\n\n\tgit clone url:to/some/project\n\tcd project\n\tgit checkout some-other-branch-with-different-submodules\n\tgit submodule update --init\n\nOf course, once you clone/checkout a submodule, there will be contents \nin that directory (including the .git/), and Git should not try to \nremove it.\n\n\nHave fun! :)\n\n...Johan\n\n\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"131277","messageId":"alpine.DEB.1.00.1001111044140.4985@pacific.mpi-cbg.de","threadId":"22160","inReplyTo":"201001111032.45637.johan@herland.net","subject":"Re: [PATCH] Remove empty directories when checking out a commit with fewer submodules","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-01-11T09:45:20Z","receivedAt":"2010-01-11T09:45:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 11 Jan 2010, Johan Herland wrote:\n\n> On Monday 11 January 2010, Johannes Schindelin wrote:\n> > Hi,\n> >\n> > On Mon, 11 Jan 2010, Peter Collingbourne wrote:\n> > > Change the unlink_entry function to use rmdir to remove submodule\n> > > directories.\n> >\n> > NAK.  We should not even try to _unlink_ submodule subdirectories; it\n> > would be _way_ too easy to lose data that way.  Remember, submodules\n> > are a totally different beast from regular files.  They can contain\n> > valuable, yet uncommitted data, that is not even meant to be\n> > committed.\n> >\n> > So you say if the submodule directories are empty, it is safe?  Not\n> > so. They will never be empty: there is always .git/, and _that_ can\n> > contain valuable information that you do not want to throw away, too.\n> >  Think of unpushed branches, for example.  That would be _fatal_ if\n> > you rmdir() that for me.\n> >\n> > So please, no,\n> \n> I believe what Peter is referring to is the _empty_ directories (and \n> that includes no .git/) that are placeholders for submodules that are \n> deliberately not cloned/checked out. This lets you do things like:\n> \n> \tgit clone url:to/some/project\n> \tcd project\n> \tgit checkout some-other-branch-with-different-submodules\n> \tgit submodule update --init\n> \n> Of course, once you clone/checkout a submodule, there will be contents \n> in that directory (including the .git/), and Git should not try to \n> remove it.\n\nYes, this might very well have been my confusion.  Peter, could you please \nrefer to such submodules as \"uninitialized\" rather than \"empty\" in the \nfuture?  This would help simple minds like mine to understand you better.\n\nCiao,\nDscho\n"},{"id":"131278","messageId":"7vocl1t1b8.fsf@alter.siamese.dyndns.org","threadId":"22160","inReplyTo":"alpine.DEB.1.00.1001110954410.4985@pacific.mpi-cbg.de","subject":"Re: [PATCH] Remove empty directories when checking out a commit with fewer submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-11T09:53:31Z","receivedAt":"2010-01-11T09:53:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> NAK.  We should not even try to _unlink_ submodule subdirectories; it \n> would be _way_ too easy to lose data that way.  Remember, submodules are a \n> totally different beast from regular files.  They can contain valuable, \n> yet uncommitted data, that is not even meant to be committed.\n>\n> So you say if the submodule directories are empty, it is safe?  Not so.  \n> They will never be empty: there is always .git/...\n\nNACK on NAK.\n\nDon't worry, your data will be safe.  The only case rmdir would actually\nremove it is (1) you check out superproject that has submodule A, but you\nchoose not to \"submodule init/update\" it, because you don't need a\ncheckout of that part of the tree for your job, and then (2) you switch to\na different version of the superproject that doesn't anymore (or didn't\nback then) have that submodule.  In such a use case, you will have only an\nempty directory for A in step (1).  The unnecessary empty directory A will\nbe left behind, even after switching to a version that shouldn't have the\ndirectory there in step (2), if you do not rmdir it.  So the patch is a\nstrict bugfix (it attempted to unlink, which is a bug; it really meant\n\"rmdir\" and not \"rm -rf\" which you seem to be worried about).\n\nIt is a separate matter to _enhance_ the codepath to actually either (A)\nrefuse to overwrite (if the version of the superproject you are switching\nto in step (2) had a regular file or a directory that is part of the\nsuperproject there, and/or (B) move it away to somewhere safe (recall the\ndiscussion of \".git/modules/$submodule\" hierarchy of the superproject?)\nautomatically when it will disappear.  Such enhancements will help people\nwho _do_ \"submodule init/update\" the submodule in step (1) and switch to a\nversion of the superproject that lacks it in step (2).\n"}]}