{"thread":{"id":"14605","subject":"[PATCH] git-filter-branch.sh: Allow running in bare repositories","startedAt":"2008-07-22T22:37:41Z","lastAt":"2008-07-23T22:29:51Z","messageCount":10,"participants":["Petr Baudis","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"84390","messageId":"20080722223710.29084.61373.stgit@localhost","threadId":"14605","inReplyTo":null,"subject":"[PATCH] git-filter-branch.sh: Allow running in bare repositories","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-07-22T22:37:41Z","receivedAt":"2008-07-22T22:37:41Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Commit 46eb449c restricted git-filter-branch to non-bare repositories\nunnecessarily; git-filter-branch can work on bare repositories just\nfine.\n\nThis also fixes suspicious shell boolean expression during a check\nfor dirty working tree.\n\nCc: Johannes Schindelin \nSigned-off-by: Petr Baudis \n---\n\n git-filter-branch.sh | 36 ++++++++++++++++++++----------------\n t/t7003-filter-branch.sh | 8 ++++++++\n 2 files changed, 28 insertions(+), 16 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex d04c346..dcfc6be 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -97,9 +97,11 @@ USAGE=\"[--env-filter ] [--tree-filter ] \\\n OPTIONS_SPEC=\n . git-sh-setup\n \n-git diff-files --quiet &&\n-\tgit diff-index --cached --quiet HEAD -- ||\n-\tdie \"Cannot rewrite branch(es) with a dirty working directory.\"\n+if [ \"$(is_bare_repository)\" = false ]; then\n+\t(git diff-files --quiet &&\n+\t git diff-index --cached --quiet HEAD --) ||\n+\t\tdie \"Cannot rewrite branch(es) with a dirty working directory.\"\n+fi\n \n tempdir=.git-rewrite\n filter_env=\n@@ -434,18 +436,20 @@ rm -rf \"$tempdir\"\n \n trap - 0\n \n-unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE\n-test -z \"$ORIG_GIT_DIR\" || {\n-\tGIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR\n-}\n-test -z \"$ORIG_GIT_WORK_TREE\" || {\n-\tGIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n-\texport GIT_WORK_TREE\n-}\n-test -z \"$ORIG_GIT_INDEX_FILE\" || {\n-\tGIT_INDEX_FILE=\"$ORIG_GIT_INDEX_FILE\" &&\n-\texport GIT_INDEX_FILE\n-}\n-git read-tree -u -m HEAD\n+if [ \"$(is_bare_repository)\" = false ]; then\n+\tunset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE\n+\ttest -z \"$ORIG_GIT_DIR\" || {\n+\t\tGIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR\n+\t}\n+\ttest -z \"$ORIG_GIT_WORK_TREE\" || {\n+\t\tGIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n+\t\texport GIT_WORK_TREE\n+\t}\n+\ttest -z \"$ORIG_GIT_INDEX_FILE\" || {\n+\t\tGIT_INDEX_FILE=\"$ORIG_GIT_INDEX_FILE\" &&\n+\t\texport GIT_INDEX_FILE\n+\t}\n+\tgit read-tree -u -m HEAD\n+fi\n \n exit $ret\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex e26f726..a0ab096 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -38,6 +38,14 @@ test_expect_success 'result is really identical' '\n \ttest $H = $(git rev-parse HEAD)\n '\n \n+test_expect_success 'rewrite bare repository identically' '\n+\t(git config core.bare true && cd .git && git-filter-branch branch)\n+'\n+git config core.bare false\n+test_expect_success 'result is really identical' '\n+\ttest $H = $(git rev-parse HEAD)\n+'\n+\n test_expect_success 'rewrite, renaming a specific file' '\n \tgit-filter-branch -f --tree-filter \"mv d doh || :\" HEAD\n '\n"},{"id":"84400","messageId":"alpine.DEB.1.00.0807230025050.8986@racer","threadId":"14605","inReplyTo":"20080722223710.29084.61373.stgit@localhost","subject":"Re: [PATCH] git-filter-branch.sh: Allow running in bare repositories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-22T23:27:07Z","receivedAt":"2008-07-22T23:27:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 23 Jul 2008, Petr Baudis wrote:\n\n> Commit 46eb449c restricted git-filter-branch to non-bare repositories\n> unnecessarily; git-filter-branch can work on bare repositories just\n> fine.\n\nI think this is fine.\n\n> This also fixes suspicious shell boolean expression during a check\n> for dirty working tree.\n\nIf you are talking about X && Y || Z, it is well established (and should \nnot be suspicious to a shell hacker like the creator of Cogito) that Z is \nexecuted if either X fails, or X succeeds and Y fails.\n\n> +test_expect_success 'rewrite bare repository identically' '\n> +\t(git config core.bare true && cd .git && git-filter-branch branch)\n> +'\n> +git config core.bare false\n\nAny reason why this is done outside the test?\n\nCiao,\nDscho\n"},{"id":"84403","messageId":"20080722233652.GT10151@machine.or.cz","threadId":"14605","inReplyTo":"alpine.DEB.1.00.0807230025050.8986@racer","subject":"Re: [PATCH] git-filter-branch.sh: Allow running in bare repositories","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-07-22T23:36:52Z","receivedAt":"2008-07-22T23:36:52Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":" Hi,\n\nOn Wed, Jul 23, 2008 at 12:27:07AM +0100, Johannes Schindelin wrote:\n> On Wed, 23 Jul 2008, Petr Baudis wrote:\n> \n> > This also fixes suspicious shell boolean expression during a check\n> > for dirty working tree.\n> \n> If you are talking about X && Y || Z, it is well established (and should \n> not be suspicious to a shell hacker like the creator of Cogito) that Z is \n> executed if either X fails, or X succeeds and Y fails.\n\n um, oops. I actually never got to know these by heart since I learnt\nto expliciply group the expressions early on. I guess my only excuse is\nthat I've stumbled at 0bdf93cbf earlier and understood it the _wrong_\nway around since I'm getting really sleepy. ;-)\n\n I still think my change improves the code readibility so it could be\nkept, but I'm fairly neutral on this.\n\n> > +test_expect_success 'rewrite bare repository identically' '\n> > +\t(git config core.bare true && cd .git && git-filter-branch branch)\n> > +'\n> > +git config core.bare false\n> \n> Any reason why this is done outside the test?\n\n If the test fails in the middle, not resetting this might negatively\naffect the rest of the testsuite.\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nAs in certain cults it is possible to kill a process if you know\nits true name. -- Ken Thompson and Dennis M. Ritchie\n"},{"id":"84408","messageId":"7v8wvtjwa8.fsf@gitster.siamese.dyndns.org","threadId":"14605","inReplyTo":"20080722233652.GT10151@machine.or.cz","subject":"Re: [PATCH] git-filter-branch.sh: Allow running in bare repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-23T00:09:51Z","receivedAt":"2008-07-23T00:09:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis writes:\n\n> um, oops. I actually never got to know these by heart since I learnt\n> to expliciply group the expressions early on. I guess my only excuse is\n> that I've stumbled at 0bdf93cbf earlier and understood it the _wrong_\n> way around since I'm getting really sleepy. ;-)\n>\n> I still think my change improves the code readibility so it could be\n> kept, but I'm fairly neutral on this.\n\nI cannot be neutral when a patch introduces unnecessary fork.\n\nThe quality of shell scripts in git.git seems to have deteriorated over\ntime but I do not think we would want to spend too much maintainer time to\ngo back and fix all of them. Please don't make things worse, at least.\n"},{"id":"84622","messageId":"20080723215509.32438.49155.stgit@localhost","threadId":"14605","inReplyTo":"7v8wvtjwa8.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] git-filter-branch.sh: Allow running in bare repositories","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-07-23T21:55:42Z","receivedAt":"2008-07-23T21:55:42Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Commit 46eb449c restricted git-filter-branch to non-bare repositories\nunnecessarily; git-filter-branch can work on bare repositories just\nfine.\n\nCc: Johannes Schindelin \nSigned-off-by: Petr Baudis \n---\n\n I have my own opinion about the readability-fork ratio in this particular\ncase, but there's no use arguing about this. ;-)\n\n git-filter-branch.sh | 36 ++++++++++++++++++++----------------\n t/t7003-filter-branch.sh | 8 ++++++++\n 2 files changed, 28 insertions(+), 16 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex d04c346..ddf7187 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -97,9 +97,11 @@ USAGE=\"[--env-filter ] [--tree-filter ] \\\n OPTIONS_SPEC=\n . git-sh-setup\n \n-git diff-files --quiet &&\n-\tgit diff-index --cached --quiet HEAD -- ||\n-\tdie \"Cannot rewrite branch(es) with a dirty working directory.\"\n+if [ \"$(is_bare_repository)\" = false ]; then\n+\tgit diff-files --quiet &&\n+\t\tgit diff-index --cached --quiet HEAD --) ||\n+\t\tdie \"Cannot rewrite branch(es) with a dirty working directory.\n+fi\n \n tempdir=.git-rewrite\n filter_env=\n@@ -434,18 +436,20 @@ rm -rf \"$tempdir\"\n \n trap - 0\n \n-unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE\n-test -z \"$ORIG_GIT_DIR\" || {\n-\tGIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR\n-}\n-test -z \"$ORIG_GIT_WORK_TREE\" || {\n-\tGIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n-\texport GIT_WORK_TREE\n-}\n-test -z \"$ORIG_GIT_INDEX_FILE\" || {\n-\tGIT_INDEX_FILE=\"$ORIG_GIT_INDEX_FILE\" &&\n-\texport GIT_INDEX_FILE\n-}\n-git read-tree -u -m HEAD\n+if [ \"$(is_bare_repository)\" = false ]; then\n+\tunset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE\n+\ttest -z \"$ORIG_GIT_DIR\" || {\n+\t\tGIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR\n+\t}\n+\ttest -z \"$ORIG_GIT_WORK_TREE\" || {\n+\t\tGIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n+\t\texport GIT_WORK_TREE\n+\t}\n+\ttest -z \"$ORIG_GIT_INDEX_FILE\" || {\n+\t\tGIT_INDEX_FILE=\"$ORIG_GIT_INDEX_FILE\" &&\n+\t\texport GIT_INDEX_FILE\n+\t}\n+\tgit read-tree -u -m HEAD\n+fi\n \n exit $ret\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex e26f726..a0ab096 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -38,6 +38,14 @@ test_expect_success 'result is really identical' '\n \ttest $H = $(git rev-parse HEAD)\n '\n \n+test_expect_success 'rewrite bare repository identically' '\n+\t(git config core.bare true && cd .git && git-filter-branch branch)\n+'\n+git config core.bare false\n+test_expect_success 'result is really identical' '\n+\ttest $H = $(git rev-parse HEAD)\n+'\n+\n test_expect_success 'rewrite, renaming a specific file' '\n \tgit-filter-branch -f --tree-filter \"mv d doh || :\" HEAD\n '\n"},{"id":"84623","messageId":"alpine.DEB.1.00.0807232257500.8986@racer","threadId":"14605","inReplyTo":"20080723215509.32438.49155.stgit@localhost","subject":"Re: [PATCH] git-filter-branch.sh: Allow running in bare repositories","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-23T21:59:03Z","receivedAt":"2008-07-23T21:59:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 23 Jul 2008, Petr Baudis wrote:\n\n> Commit 46eb449c restricted git-filter-branch to non-bare repositories\n> unnecessarily; git-filter-branch can work on bare repositories just\n> fine.\n> \n> Cc: Johannes Schindelin \n\nFunny, I did not get Cc:ed.\n\n> -git diff-files --quiet &&\n> -\tgit diff-index --cached --quiet HEAD -- ||\n> -\tdie \"Cannot rewrite branch(es) with a dirty working directory.\"\n> +if [ \"$(is_bare_repository)\" = false ]; then\n> +\tgit diff-files --quiet &&\n> +\t\tgit diff-index --cached --quiet HEAD --) ||\n\n ^\n\nI doubt this has ever passed the test suite, let alone run.\n\nBesides, this extra-funny extra indent is quite brutal on my eyes.\n\nCiao,\nDscho\n"},{"id":"84626","messageId":"20080723220657.GA10151@machine.or.cz","threadId":"14605","inReplyTo":"alpine.DEB.1.00.0807232257500.8986@racer","subject":"Re: [PATCH] git-filter-branch.sh: Allow running in bare repositories","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-07-23T22:06:57Z","receivedAt":"2008-07-23T22:06:57Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":" Hi,\n\nOn Wed, Jul 23, 2008 at 10:59:03PM +0100, Johannes Schindelin wrote:\n> On Wed, 23 Jul 2008, Petr Baudis wrote:\n> \n> > Commit 46eb449c restricted git-filter-branch to non-bare repositories\n> > unnecessarily; git-filter-branch can work on bare repositories just\n> > fine.\n> > \n> > Cc: Johannes Schindelin \n> \n> Funny, I did not get Cc:ed.\n\n sorry, I have to copy the Cc around manually with StGIT. :/\n\n> > -git diff-files --quiet &&\n> > -\tgit diff-index --cached --quiet HEAD -- ||\n> > -\tdie \"Cannot rewrite branch(es) with a dirty working directory.\"\n> > +if [ \"$(is_bare_repository)\" = false ]; then\n> > +\tgit diff-files --quiet &&\n> > +\t\tgit diff-index --cached --quiet HEAD --) ||\n> \n> ^\n> \n> I doubt this has ever passed the test suite, let alone run.\n\n It did... provided that I didn't make after modifying the script.\n\n> Besides, this extra-funny extra indent is quite brutal on my eyes.\n\n I kept the original indentation.\n\n\t\t\t\tPetr \"Pasky\" Baudis\n"},{"id":"84629","messageId":"20080723221221.12055.8509.stgit@localhost","threadId":"14605","inReplyTo":"20080723220657.GA10151@machine.or.cz","subject":"[PATCHv3] git-filter-branch.sh: Allow running in bare repositories","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2008-07-23T22:15:36Z","receivedAt":"2008-07-23T22:15:36Z","isPatch":false,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"Commit 46eb449c restricted git-filter-branch to non-bare repositories\nunnecessarily; git-filter-branch can work on bare repositories just\nfine.\n\nCc: Johannes Schindelin \nSigned-off-by: Petr Baudis \n---\n\n Sorry for the spam. This time I actually did make git-filter-branch\nbefore running the testsuite.\n\n git-filter-branch.sh | 36 ++++++++++++++++++++----------------\n t/t7003-filter-branch.sh | 8 ++++++++\n 2 files changed, 28 insertions(+), 16 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex d04c346..507c50e 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -97,9 +97,11 @@ USAGE=\"[--env-filter ] [--tree-filter ] \\\n OPTIONS_SPEC=\n . git-sh-setup\n \n-git diff-files --quiet &&\n-\tgit diff-index --cached --quiet HEAD -- ||\n-\tdie \"Cannot rewrite branch(es) with a dirty working directory.\"\n+if [ \"$(is_bare_repository)\" = false ]; then\n+\tgit diff-files --quiet &&\n+\t\tgit diff-index --cached --quiet HEAD -- ||\n+\t\tdie \"Cannot rewrite branch(es) with a dirty working directory.\"\n+fi\n \n tempdir=.git-rewrite\n filter_env=\n@@ -434,18 +436,20 @@ rm -rf \"$tempdir\"\n \n trap - 0\n \n-unset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE\n-test -z \"$ORIG_GIT_DIR\" || {\n-\tGIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR\n-}\n-test -z \"$ORIG_GIT_WORK_TREE\" || {\n-\tGIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n-\texport GIT_WORK_TREE\n-}\n-test -z \"$ORIG_GIT_INDEX_FILE\" || {\n-\tGIT_INDEX_FILE=\"$ORIG_GIT_INDEX_FILE\" &&\n-\texport GIT_INDEX_FILE\n-}\n-git read-tree -u -m HEAD\n+if [ \"$(is_bare_repository)\" = false ]; then\n+\tunset GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE\n+\ttest -z \"$ORIG_GIT_DIR\" || {\n+\t\tGIT_DIR=\"$ORIG_GIT_DIR\" && export GIT_DIR\n+\t}\n+\ttest -z \"$ORIG_GIT_WORK_TREE\" || {\n+\t\tGIT_WORK_TREE=\"$ORIG_GIT_WORK_TREE\" &&\n+\t\texport GIT_WORK_TREE\n+\t}\n+\ttest -z \"$ORIG_GIT_INDEX_FILE\" || {\n+\t\tGIT_INDEX_FILE=\"$ORIG_GIT_INDEX_FILE\" &&\n+\t\texport GIT_INDEX_FILE\n+\t}\n+\tgit read-tree -u -m HEAD\n+fi\n \n exit $ret\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex e26f726..a0ab096 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -38,6 +38,14 @@ test_expect_success 'result is really identical' '\n \ttest $H = $(git rev-parse HEAD)\n '\n \n+test_expect_success 'rewrite bare repository identically' '\n+\t(git config core.bare true && cd .git && git-filter-branch branch)\n+'\n+git config core.bare false\n+test_expect_success 'result is really identical' '\n+\ttest $H = $(git rev-parse HEAD)\n+'\n+\n test_expect_success 'rewrite, renaming a specific file' '\n \tgit-filter-branch -f --tree-filter \"mv d doh || :\" HEAD\n '\n"},{"id":"84631","messageId":"7vk5fc9qvb.fsf@gitster.siamese.dyndns.org","threadId":"14605","inReplyTo":"20080723215509.32438.49155.stgit@localhost","subject":"Re: [PATCH] git-filter-branch.sh: Allow running in bare repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-23T22:29:12Z","receivedAt":"2008-07-23T22:29:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Petr Baudis writes:\n\n> Commit 46eb449c restricted git-filter-branch to non-bare repositories\n> unnecessarily; git-filter-branch can work on bare repositories just\n> fine.\n>\n> Cc: Johannes Schindelin \n> Signed-off-by: Petr Baudis \n> ---\n>\n> I have my own opinion about the readability-fork ratio in this particular\n> case, but there's no use arguing about this. ;-)\n\nGrouping commands is perfectly fine when you think the readers may find it\neaslier to follow the logic if you grouped them. Use { } for that kind of\ngrouping; I do not have any problem with that.\n\nUse of subshell ( ) is often done by inexperienced people or by careless\npeople without thinking. Sometimes you would explicitly want to have a\nsubshell (e.g. when you want to chdir to do something there but do not\nwant to affect the main program), and sometimes you don't (e.g. you are\ngrouping just for precedence, and want assignments and side effects done\ninside the group visible by the main program).\n\nCareless uses of ( ) wastes reviewer's time because the code inside has to\nbe studied to find out if the writer really wanted to have an isolated\nseparate process that subshell gives, or just being plain careless.\n"},{"id":"84632","messageId":"7vfxq09qu8.fsf@gitster.siamese.dyndns.org","threadId":"14605","inReplyTo":"20080723221221.12055.8509.stgit@localhost","subject":"Re: [PATCHv3] git-filter-branch.sh: Allow running in bare repositories","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-23T22:29:51Z","receivedAt":"2008-07-23T22:29:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks. Applied.\n"}]}