{"thread":{"id":"17557","subject":"[PATCH] filter-branch: do not consider diverging submodules a 'dirty worktree'","startedAt":"2009-02-04T14:40:58Z","lastAt":"2009-02-05T17:39:13Z","messageCount":9,"participants":["Johannes Schindelin","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103139","messageId":"alpine.DEB.1.00.0902041540130.10279@pacific.mpi-cbg.de","threadId":"17557","inReplyTo":"cover.1233758410u.git.johannes.schindelin@gmx.de","subject":"[PATCH] filter-branch: do not consider diverging submodules a 'dirty worktree'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-04T14:40:58Z","receivedAt":"2009-02-04T14:40:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"As filter-branch could not care less about submodules' actual contents,\nit does not make sense to check if the checked-out submodules are\nup-to-date before running filter-branch.  So do not do it.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n git-filter-branch.sh |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 8cbce4e..066f9c3 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -108,8 +108,8 @@ OPTIONS_SPEC=\n . git-sh-setup\n \n if [ \"$(is_bare_repository)\" = false ]; then\n-\tgit diff-files --quiet &&\n-\tgit diff-index --cached --quiet HEAD -- ||\n+\tgit diff-files --ignore-submodules --quiet &&\n+\tgit diff-index --ignore-submodules --cached --quiet HEAD -- ||\n \tdie \"Cannot rewrite branch(es) with a dirty working directory.\"\n fi\n \n-- \n1.6.1.2.582.g3fdd5\n"},{"id":"103168","messageId":"7vvdrqunog.fsf@gitster.siamese.dyndns.org","threadId":"17557","inReplyTo":"alpine.DEB.1.00.0902041540130.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] filter-branch: do not consider diverging submodules a 'dirty worktree'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-04T17:17:19Z","receivedAt":"2009-02-04T17:17:19Z","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> As filter-branch could not care less about submodules' actual contents,\n> it does not make sense to check if the checked-out submodules are\n> up-to-date before running filter-branch.  So do not do it.\n\nSorry, but I am confused.  Is that because even the tree-filter does not\nuse the actual work tree but works in the temporary area .git-rewrite, and\na diverged submodule cannot possibly matter (and index-filter works solely\non the index anyway)?\n\nIf so, why do we even check dirtiness of anything at all?\n\nThis is not a \"wouldn't this better?\" proposal patch, but a \"why isn't the\npatch like this?\" question patch.\n\n git-filter-branch.sh |    6 ------\n 1 files changed, 0 insertions(+), 6 deletions(-)\n\ndiff --git i/git-filter-branch.sh w/git-filter-branch.sh\nindex eb62f71..dda32e0 100755\n--- i/git-filter-branch.sh\n+++ w/git-filter-branch.sh\n@@ -107,12 +107,6 @@ USAGE=\"[--env-filter <command>] [--tree-filter <command>] \\\n OPTIONS_SPEC=\n . git-sh-setup\n \n-if [ \"$(is_bare_repository)\" = false ]; then\n-\tgit diff-files --quiet &&\n-\tgit diff-index --cached --quiet HEAD -- ||\n-\tdie \"Cannot rewrite branch(es) with a dirty working directory.\"\n-fi\n-\n tempdir=.git-rewrite\n filter_env=\n filter_tree=\n"},{"id":"103169","messageId":"alpine.DEB.1.00.0902041823330.22763@intel-tinevez-2-302","threadId":"17557","inReplyTo":"7vvdrqunog.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] filter-branch: do not consider diverging submodules a 'dirty worktree'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-04T17:24:44Z","receivedAt":"2009-02-04T17:24:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Feb 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n> \n> > As filter-branch could not care less about submodules' actual \n> > contents, it does not make sense to check if the checked-out \n> > submodules are up-to-date before running filter-branch.  So do not do \n> > it.\n> \n> Sorry, but I am confused.  Is that because even the tree-filter does not \n> use the actual work tree but works in the temporary area .git-rewrite, \n> and a diverged submodule cannot possibly matter (and index-filter works \n> solely on the index anyway)?\n> \n> If so, why do we even check dirtiness of anything at all?\n\nI guess it is because we could update the working directory with read-tree \n-u -m HEAD at the end.  Actually, that is exactly what we do.\n\nSubmodules do not matter much here, as nothing will be overwritten, \nreally, but a dirty working directory matters.\n\nCiao,\nDscho\n"},{"id":"103170","messageId":"4989CF79.2070209@viscovery.net","threadId":"17557","inReplyTo":"7vvdrqunog.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] filter-branch: do not consider diverging submodules a 'dirty worktree'","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-04T17:25:13Z","receivedAt":"2009-02-04T17:25:13Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> If so, why do we even check dirtiness of anything at all?\n> \n> This is not a \"wouldn't this better?\" proposal patch, but a \"why isn't the\n> patch like this?\" question patch.\n> \n>  git-filter-branch.sh |    6 ------\n>  1 files changed, 0 insertions(+), 6 deletions(-)\n> \n> diff --git i/git-filter-branch.sh w/git-filter-branch.sh\n> index eb62f71..dda32e0 100755\n> --- i/git-filter-branch.sh\n> +++ w/git-filter-branch.sh\n> @@ -107,12 +107,6 @@ USAGE=\"[--env-filter <command>] [--tree-filter <command>] \\\n>  OPTIONS_SPEC=\n>  . git-sh-setup\n>  \n> -if [ \"$(is_bare_repository)\" = false ]; then\n> -\tgit diff-files --quiet &&\n> -\tgit diff-index --cached --quiet HEAD -- ||\n> -\tdie \"Cannot rewrite branch(es) with a dirty working directory.\"\n> -fi\n> -\n>  tempdir=.git-rewrite\n>  filter_env=\n>  filter_tree=\n\nBecause if the repository is non-bare, then filter-branch updates the\nwork-tree at the end of the run; we don't want to overwrite uncommitted\nwork in this case.\n\nThis behavior is a relic from cg-admin-rewritehist, I think. I've never\nfound it useful.\n\n-- Hannes\n"},{"id":"103173","messageId":"7vvdrqt74k.fsf@gitster.siamese.dyndns.org","threadId":"17557","inReplyTo":"4989CF79.2070209@viscovery.net","subject":"Re: [PATCH] filter-branch: do not consider diverging submodules a 'dirty worktree'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-04T18:00:11Z","receivedAt":"2009-02-04T18:00:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt <j.sixt@viscovery.net> writes:\n\n> Because if the repository is non-bare, then filter-branch updates the\n> work-tree at the end of the run; we don't want to overwrite uncommitted\n> work in this case.\n>\n> This behavior is a relic from cg-admin-rewritehist, I think. I've never\n> found it useful.\n\nOk, I think \"read-tree -m -u HEAD\" at the end sort of makes sense, if you\nfiltered the branch you are currently sitting on.  Does that mean we do\nnot have to barf on dirtyness if we can tell if the filter-branch will not\ntouch the current branch at all?  Again, I am not suggesting it as an\nimprovement, but I am trying to see if I am talking a total nonsense.\n\nIs the reason why you haven't found it is useful is because you never\nfilter the current branch?\n"},{"id":"103176","messageId":"alpine.DEB.1.00.0902041915070.22763@intel-tinevez-2-302","threadId":"17557","inReplyTo":"7vvdrqt74k.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] filter-branch: do not consider diverging submodules a 'dirty worktree'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-04T18:15:53Z","receivedAt":"2009-02-04T18:15:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Feb 2009, Junio C Hamano wrote:\n\n> Johannes Sixt <j.sixt@viscovery.net> writes:\n> \n> > Because if the repository is non-bare, then filter-branch updates the\n> > work-tree at the end of the run; we don't want to overwrite uncommitted\n> > work in this case.\n> >\n> > This behavior is a relic from cg-admin-rewritehist, I think. I've never\n> > found it useful.\n> \n> Ok, I think \"read-tree -m -u HEAD\" at the end sort of makes sense, if you\n> filtered the branch you are currently sitting on.  Does that mean we do\n> not have to barf on dirtyness if we can tell if the filter-branch will not\n> touch the current branch at all?  Again, I am not suggesting it as an\n> improvement, but I am trying to see if I am talking a total nonsense.\n\nThat's correct.  Checking if we would touch the current branch is too \nexpensive here, that's why we don't do it.\n\n> Is the reason why you haven't found it is useful is because you never \n> filter the current branch?\n\nWell, _I_ certainly do, and I find it very useful.\n\nCiao,\nDscho\n"},{"id":"103182","messageId":"7vbptit4hm.fsf@gitster.siamese.dyndns.org","threadId":"17557","inReplyTo":"alpine.DEB.1.00.0902041915070.22763@intel-tinevez-2-302","subject":"Re: [PATCH] filter-branch: do not consider diverging submodules a 'dirty worktree'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-04T18:57:09Z","receivedAt":"2009-02-04T18:57:09Z","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> On Wed, 4 Feb 2009, Junio C Hamano wrote:\n>\n>> Johannes Sixt <j.sixt@viscovery.net> writes:\n>> \n>> > Because if the repository is non-bare, then filter-branch updates the\n>> > work-tree at the end of the run; we don't want to overwrite uncommitted\n>> > work in this case.\n>> >\n>> > This behavior is a relic from cg-admin-rewritehist, I think. I've never\n>> > found it useful.\n>> \n>> Ok, I think \"read-tree -m -u HEAD\" at the end sort of makes sense, if you\n>> filtered the branch you are currently sitting on.  Does that mean we do\n>> not have to barf on dirtyness if we can tell if the filter-branch will not\n>> touch the current branch at all?  Again, I am not suggesting it as an\n>> improvement, but I am trying to see if I am talking a total nonsense.\n>\n> That's correct.  Checking if we would touch the current branch is too \n> expensive here, that's why we don't do it.\n\nOk, so these exchange suggests that the commit log message needs a bit\nmore explanation why the check matters before describing why submodules\nshould not be checked.  Something like this, perhaps?\n\n    At the end of filter-branch in a non-bare repository, the work tree is\n    updated with \"read-tree -m -u HEAD\", to carry the change forward in\n    case the current branch was rewritten.  In order to avoid losing any\n    local change during this step, filter-branch refuses to work when\n    there are local changes in the work tree.\n\n    This \"read-tree -m -u HEAD\" operation does not affect what commit is\n    checked out in a submodule (iow, it does not touch .git/HEAD in a\n    submodule checkout), and checking if there is any local change to the\n    submodule is not useful.\n\nWhile I think it makes sense to ignore submodules for the diff-files\ncheck, I do not think it is correct to do so in the check to see if you\nhave staged changes.  If you updated what commit should be checked out in\nyour index, and if you run \"read-tree -m -u HEAD\", it can conflict the\nsame way as a staged change to a regular blob.  The most trivial example\nwould be if your filtering were to remove any submodule.  Your work tree\nchange wanted to modify while the branch switching is to remove and you\nhave a modify/remove conflict right there.  Or am I again confused?\n"},{"id":"103277","messageId":"498A8C04.40602@viscovery.net","threadId":"17557","inReplyTo":"7vvdrqt74k.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] filter-branch: do not consider diverging submodules a 'dirty worktree'","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-05T06:49:40Z","receivedAt":"2009-02-05T06:49:40Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> Is the reason why you haven't found it is useful is because you never\n> filter the current branch?\n\nSort of. When I did some serious branch filtering in the past, I always\ndid it in a bare repository.\n\n-- Hannes\n"},{"id":"103359","messageId":"alpine.DEB.1.00.0902051838310.7491@intel-tinevez-2-302","threadId":"17557","inReplyTo":"7vbptit4hm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] filter-branch: do not consider diverging submodules a 'dirty worktree'","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-05T17:39:13Z","receivedAt":"2009-02-05T17:39:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 4 Feb 2009, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > On Wed, 4 Feb 2009, Junio C Hamano wrote:\n> >\n> >> Johannes Sixt <j.sixt@viscovery.net> writes:\n> >> \n> >> > Because if the repository is non-bare, then filter-branch updates the\n> >> > work-tree at the end of the run; we don't want to overwrite uncommitted\n> >> > work in this case.\n> >> >\n> >> > This behavior is a relic from cg-admin-rewritehist, I think. I've never\n> >> > found it useful.\n> >> \n> >> Ok, I think \"read-tree -m -u HEAD\" at the end sort of makes sense, if you\n> >> filtered the branch you are currently sitting on.  Does that mean we do\n> >> not have to barf on dirtyness if we can tell if the filter-branch will not\n> >> touch the current branch at all?  Again, I am not suggesting it as an\n> >> improvement, but I am trying to see if I am talking a total nonsense.\n> >\n> > That's correct.  Checking if we would touch the current branch is too \n> > expensive here, that's why we don't do it.\n> \n> Ok, so these exchange suggests that the commit log message needs a bit\n> more explanation why the check matters before describing why submodules\n> should not be checked.  Something like this, perhaps?\n> \n>     At the end of filter-branch in a non-bare repository, the work tree is\n>     updated with \"read-tree -m -u HEAD\", to carry the change forward in\n>     case the current branch was rewritten.  In order to avoid losing any\n>     local change during this step, filter-branch refuses to work when\n>     there are local changes in the work tree.\n> \n>     This \"read-tree -m -u HEAD\" operation does not affect what commit is\n>     checked out in a submodule (iow, it does not touch .git/HEAD in a\n>     submodule checkout), and checking if there is any local change to the\n>     submodule is not useful.\n\nGreat!\n\n> While I think it makes sense to ignore submodules for the diff-files\n> check, I do not think it is correct to do so in the check to see if you\n> have staged changes.  If you updated what commit should be checked out in\n> your index, and if you run \"read-tree -m -u HEAD\", it can conflict the\n> same way as a staged change to a regular blob.  The most trivial example\n> would be if your filtering were to remove any submodule.  Your work tree\n> change wanted to modify while the branch switching is to remove and you\n> have a modify/remove conflict right there.  Or am I again confused?\n\nI removed that change, and added a few words why it makes sense to check \nthat.\n\nThanks,\nDscho\n"}]}