{"thread":{"id":"17794","subject":"[PATCH 2/4] notes: use GIT_EDITOR and core.editor over VISUAL/EDITOR","startedAt":"2009-02-14T19:15:03Z","lastAt":"2009-02-19T00:46:33Z","messageCount":47,"participants":["Thomas Rast","Johannes Schindelin","Junio C Hamano","Jeff King","Mike Ralphson","Brandon Casey","Jay Soffian","Wincent Colaiuta"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"104664","messageId":"b36bbe2f7f0ede6a2caa47190ada868dd4c0099c.1234638466.git.trast@student.ethz.ch","threadId":"17794","inReplyTo":null,"subject":"[PATCH 1/4] notes: only clean up message file when editing","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T19:15:03Z","receivedAt":"2009-02-14T19:15:03Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"We clean up the notes file when exiting. However, actions other than\n'edit' have nothing to do with the file, so they should not remove it\nwhen done.\n\nSigned-off-by: Thomas Rast \n---\n git-notes.sh | 10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-notes.sh b/git-notes.sh\nindex 9cbad02..246df65 100755\n--- a/git-notes.sh\n+++ b/git-notes.sh\n@@ -14,13 +14,13 @@ test -z \"$GIT_NOTES_REF\" && GIT_NOTES_REF=\"refs/notes/commits\"\n COMMIT=$(git rev-parse --verify --default HEAD \"$@\") ||\n die \"Invalid commit: $@\"\n \n-MESSAGE=\"$GIT_DIR\"/new-notes-$COMMIT\n-trap '\n-\ttest -f \"$MESSAGE\" && rm \"$MESSAGE\"\n-' 0\n-\n case \"$ACTION\" in\n edit)\n+\tMESSAGE=\"$GIT_DIR\"/new-notes-$COMMIT\n+\ttrap '\n+\t\ttest -f \"$MESSAGE\" && rm \"$MESSAGE\"\n+\t' 0\n+\n \tGIT_NOTES_REF= git log -1 $COMMIT | sed \"s/^/#/\" > \"$MESSAGE\"\n \n \tGIT_INDEX_FILE=\"$MESSAGE\".idx\n-- \n1.6.2.rc0.288.g6852b\n"},{"id":"104663","messageId":"d2ca964918f3f930505cd969cf8407d4a8d67fe3.1234638466.git.trast@student.ethz.ch","threadId":"17794","inReplyTo":"b36bbe2f7f0ede6a2caa47190ada868dd4c0099c.1234638466.git.trast@student.ethz.ch","subject":"[PATCH 2/4] notes: use GIT_EDITOR and core.editor over VISUAL/EDITOR","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T19:15:04Z","receivedAt":"2009-02-14T19:15:04Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"This is the order documented in git-config(1), so we should stick to\nit.\n\nSigned-off-by: Thomas Rast \n---\n\nIt's actually only _almost_ the documented order: the manpage says\nthat $GIT_EDITOR is used when it is set, presumably even when it is\nset but empty. I cannot see how that would be useful however.\n\n\n git-notes.sh | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/git-notes.sh b/git-notes.sh\nindex 246df65..6859470 100755\n--- a/git-notes.sh\n+++ b/git-notes.sh\n@@ -35,7 +35,8 @@ edit)\n \t\tgit cat-file blob :$COMMIT >> \"$MESSAGE\" 2> /dev/null\n \tfi\n \n-\t${VISUAL:-${EDITOR:-vi}} \"$MESSAGE\"\n+\tcore_editor=\"$(git config core.editor)\"\n+\t${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} \"$MESSAGE\"\n \n \tgrep -v ^# < \"$MESSAGE\" | git stripspace > \"$MESSAGE\".processed\n \tmv \"$MESSAGE\".processed \"$MESSAGE\"\n-- \n1.6.2.rc0.288.g6852b\n"},{"id":"104665","messageId":"21cefcfb71849535430c75575a9decf1f9fe9c06.1234638466.git.trast@student.ethz.ch","threadId":"17794","inReplyTo":"b36bbe2f7f0ede6a2caa47190ada868dd4c0099c.1234638466.git.trast@student.ethz.ch","subject":"[PATCH 3/4] t3301: fix confusing test for valid notes ref","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T19:15:05Z","receivedAt":"2009-02-14T19:15:05Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The test used single quotes in the test code, but also single quotes\nto wrap the entire test snippet, thus effectively skipping _out_ of\nquoted mode. Since it doesn't matter here, just drop the quotes to\ncause less confusion.\n\nAlso, the test passed a MSG variable to 'git notes show', but that\nnever calls an editor.\n\nSigned-off-by: Thomas Rast \n---\n\nAs an aside, is there a specific reason why all the negative tests use\n'! VAR=foo bar' instead of 'VAR=foo test_must_fail bar'? I thought\nthe latter was preferred to catch segfaults.\n\n\n t/t3301-notes.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex ff4ea05..b99271e 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -31,8 +31,8 @@ test_expect_success setup '\n '\n \n test_expect_success 'need valid notes ref' '\n-\t! MSG=1 GIT_NOTES_REF='/' git notes edit &&\n-\t! MSG=2 GIT_NOTES_REF='/' git notes show\n+\t! MSG=1 GIT_NOTES_REF=/ git notes edit &&\n+\t! GIT_NOTES_REF=/ git notes show\n '\n \n # 1 indicates caught gracefully by die, 128 means git-show barked\n-- \n1.6.2.rc0.288.g6852b\n"},{"id":"104666","messageId":"ac3c2ce272e8e7f3000349f682a5a4843aea34e6.1234638466.git.trast@student.ethz.ch","threadId":"17794","inReplyTo":"b36bbe2f7f0ede6a2caa47190ada868dd4c0099c.1234638466.git.trast@student.ethz.ch","subject":"[PATCH 4/4] notes: refuse to edit notes outside refs/notes/","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T19:15:06Z","receivedAt":"2009-02-14T19:15:06Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The current git-notes has no safety valves whatsoever, and will\nhappily edit notes in, e.g., refs/remotes/. This is dangerous, as\nthey will be overwritten during the next fetch of the remote they\nbelong to.\n\nSince it is supposed to be a plumbing frontend for the notes feature,\nsimply forbid editing notes with GIT_NOTES_REF pointing anywhere but\nrefs/notes/*.\n\nSigned-off-by: Thomas Rast \n---\n\nThis is the only patch of substance in this series :-)\n\nI'm not sure if anyone uses a notes ref outside of refs/notes/ yet, so\nthis may actually break some people's setups already. But now that we\n(at least me :-) offer notes for download in public repos, I can see\npeople accidentally edit a remote notes ref.\n\n\n git-notes.sh | 4 ++++\n t/t3301-notes.sh | 8 ++++++++\n 2 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/git-notes.sh b/git-notes.sh\nindex 6859470..6ec33c9 100755\n--- a/git-notes.sh\n+++ b/git-notes.sh\n@@ -16,6 +16,10 @@ die \"Invalid commit: $@\"\n \n case \"$ACTION\" in\n edit)\n+\tif [ \"${GIT_NOTES_REF#refs/notes/}\" = \"$GIT_NOTES_REF\" ]; then\n+\t\tdie \"Refusing to edit notes in $GIT_NOTES_REF (outside of refs/notes/)\"\n+\tfi\n+\n \tMESSAGE=\"$GIT_DIR\"/new-notes-$COMMIT\n \ttrap '\n \t\ttest -f \"$MESSAGE\" && rm \"$MESSAGE\"\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex b99271e..1503e79 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -35,6 +35,14 @@ test_expect_success 'need valid notes ref' '\n \t! GIT_NOTES_REF=/ git notes show\n '\n \n+test_expect_success 'refusing to edit in refs/heads/' '\n+\t! MSG=1 GIT_NOTES_REF=refs/heads/bogus git notes edit\n+'\n+\n+test_expect_success 'refusing to edit in refs/remotes/' '\n+\t! MSG=1 GIT_NOTES_REF=refs/remotes/bogus git notes edit\n+'\n+\n # 1 indicates caught gracefully by die, 128 means git-show barked\n test_expect_success 'handle empty notes gracefully' '\n \tgit notes show ; test 1 = $?\n-- \n1.6.2.rc0.288.g6852b\n"},{"id":"104669","messageId":"alpine.DEB.1.00.0902142029010.10279@pacific.mpi-cbg.de","threadId":"17794","inReplyTo":"b36bbe2f7f0ede6a2caa47190ada868dd4c0099c.1234638466.git.trast@student.ethz.ch","subject":"Re: [PATCH 1/4] notes: only clean up message file when editing","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-14T19:29:13Z","receivedAt":"2009-02-14T19:29:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 14 Feb 2009, Thomas Rast wrote:\n\n> We clean up the notes file when exiting. However, actions other than\n> 'edit' have nothing to do with the file, so they should not remove it\n> when done.\n\nACK.\n\nCiao,\nDscho\n"},{"id":"104670","messageId":"alpine.DEB.1.00.0902142029170.10279@pacific.mpi-cbg.de","threadId":"17794","inReplyTo":"d2ca964918f3f930505cd969cf8407d4a8d67fe3.1234638466.git.trast@student.ethz.ch","subject":"Re: [PATCH 2/4] notes: use GIT_EDITOR and core.editor over VISUAL/EDITOR","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-14T19:29:30Z","receivedAt":"2009-02-14T19:29:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 14 Feb 2009, Thomas Rast wrote:\n\n> This is the order documented in git-config(1), so we should stick to it.\n\nACK.\n\nCiao,\nDscho\n"},{"id":"104673","messageId":"alpine.DEB.1.00.0902142029420.10279@pacific.mpi-cbg.de","threadId":"17794","inReplyTo":"21cefcfb71849535430c75575a9decf1f9fe9c06.1234638466.git.trast@student.ethz.ch","subject":"Re: [PATCH 3/4] t3301: fix confusing test for valid notes ref","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-14T19:32:14Z","receivedAt":"2009-02-14T19:32:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 14 Feb 2009, Thomas Rast wrote:\n\n> The test used single quotes in the test code, but also single quotes\n> to wrap the entire test snippet, thus effectively skipping _out_ of\n> quoted mode. Since it doesn't matter here, just drop the quotes to\n> cause less confusion.\n\nACK.\n\n> Also, the test passed a MSG variable to 'git notes show', but that\n> never calls an editor.\n\nThat was actually what I tried to test by passing MSG=2... So I'd like to \nkeep it.\n\n> As an aside, is there a specific reason why all the negative tests use \n> '! VAR=foo bar' instead of 'VAR=foo test_must_fail bar'? I thought the \n> latter was preferred to catch segfaults.\n\nI guess that back then when I wrote the first version of notes (which I \nhoped Johan Herland would take custody of), test_must_fail either did not \nexist or was too new for me to have picked up yet.\n\nCiao,\nDscho\n"},{"id":"104674","messageId":"alpine.DEB.1.00.0902142032210.10279@pacific.mpi-cbg.de","threadId":"17794","inReplyTo":"ac3c2ce272e8e7f3000349f682a5a4843aea34e6.1234638466.git.trast@student.ethz.ch","subject":"Re: [PATCH 4/4] notes: refuse to edit notes outside refs/notes/","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-14T19:33:44Z","receivedAt":"2009-02-14T19:33:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 14 Feb 2009, Thomas Rast wrote:\n\n> The current git-notes has no safety valves whatsoever, and will\n> happily edit notes in, e.g., refs/remotes/. This is dangerous, as\n> they will be overwritten during the next fetch of the remote they\n> belong to.\n> \n> Since it is supposed to be a plumbing frontend for the notes feature,\n> simply forbid editing notes with GIT_NOTES_REF pointing anywhere but\n> refs/notes/*.\n\nACK.\n\nExcept maybe for calling it \"plumbing\". \"git notes\" is meant to be _the_ \nporcelain of the \"notes\", and that is why I like the idea to restrict it \nto refs/notes/* for editing (not for anything else, though).\n\nThanks,\nDscho\n"},{"id":"104677","messageId":"200902142056.42198.trast@student.ethz.ch","threadId":"17794","inReplyTo":"alpine.DEB.1.00.0902142032210.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH 4/4] notes: refuse to edit notes outside refs/notes/","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T19:56:38Z","receivedAt":"2009-02-14T19:56:38Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Johannes Schindelin wrote:\n> On Sat, 14 Feb 2009, Thomas Rast wrote:\n> > Since it is supposed to be a plumbing frontend for the notes feature,\n> > simply forbid editing notes with GIT_NOTES_REF pointing anywhere but\n> > refs/notes/*.\n> \n> ACK.\n> \n> Except maybe for calling it \"plumbing\". \"git notes\" is meant to be _the_ \n> porcelain of the \"notes\", and that is why I like the idea to restrict it \n> to refs/notes/* for editing (not for anything else, though).\n\nThat's what I actually wanted to write. Maybe I should go get more\ncoffee...\n\nThanks for the acks. Will resend with 3/4 corrected, and add a\nconversion to test_must_fail.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"104686","messageId":"4655551d70d8829a7a15219aef384deb4e63371a.1234642638.git.trast@student.ethz.ch","threadId":"17794","inReplyTo":"200902142056.42198.trast@student.ethz.ch","subject":"[PATCH v2 1/5] notes: only clean up message file when editing","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T20:23:21Z","receivedAt":"2009-02-14T20:23:21Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"We clean up the notes file when exiting. However, actions other than\n'edit' have nothing to do with the file, so they should not remove it\nwhen done.\n\nSigned-off-by: Thomas Rast \nAcked-by: Johannes Schindelin \n---\n\nSame as v1.\n\n git-notes.sh | 10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-notes.sh b/git-notes.sh\nindex 9cbad02..246df65 100755\n--- a/git-notes.sh\n+++ b/git-notes.sh\n@@ -14,13 +14,13 @@ test -z \"$GIT_NOTES_REF\" && GIT_NOTES_REF=\"refs/notes/commits\"\n COMMIT=$(git rev-parse --verify --default HEAD \"$@\") ||\n die \"Invalid commit: $@\"\n \n-MESSAGE=\"$GIT_DIR\"/new-notes-$COMMIT\n-trap '\n-\ttest -f \"$MESSAGE\" && rm \"$MESSAGE\"\n-' 0\n-\n case \"$ACTION\" in\n edit)\n+\tMESSAGE=\"$GIT_DIR\"/new-notes-$COMMIT\n+\ttrap '\n+\t\ttest -f \"$MESSAGE\" && rm \"$MESSAGE\"\n+\t' 0\n+\n \tGIT_NOTES_REF= git log -1 $COMMIT | sed \"s/^/#/\" > \"$MESSAGE\"\n \n \tGIT_INDEX_FILE=\"$MESSAGE\".idx\n-- \n1.6.2.rc0.296.ge2122\n"},{"id":"104684","messageId":"69c813ea5c4a3c3f43e729bd59a4064f74e72d66.1234642638.git.trast@student.ethz.ch","threadId":"17794","inReplyTo":"200902142056.42198.trast@student.ethz.ch","subject":"[PATCH v2 2/5] notes: use GIT_EDITOR and core.editor over VISUAL/EDITOR","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T20:23:22Z","receivedAt":"2009-02-14T20:23:22Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"This is the order documented in git-config(1), so we should stick to\nit.\n\nSigned-off-by: Thomas Rast \nAcked-by: Johannes Schindelin \n---\n\nSame as v1.\n\n git-notes.sh | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/git-notes.sh b/git-notes.sh\nindex 246df65..6859470 100755\n--- a/git-notes.sh\n+++ b/git-notes.sh\n@@ -35,7 +35,8 @@ edit)\n \t\tgit cat-file blob :$COMMIT >> \"$MESSAGE\" 2> /dev/null\n \tfi\n \n-\t${VISUAL:-${EDITOR:-vi}} \"$MESSAGE\"\n+\tcore_editor=\"$(git config core.editor)\"\n+\t${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} \"$MESSAGE\"\n \n \tgrep -v ^# < \"$MESSAGE\" | git stripspace > \"$MESSAGE\".processed\n \tmv \"$MESSAGE\".processed \"$MESSAGE\"\n-- \n1.6.2.rc0.296.ge2122\n"},{"id":"104685","messageId":"2d4b5321543974ec25921102faef900e42eea1d5.1234642638.git.trast@student.ethz.ch","threadId":"17794","inReplyTo":"200902142056.42198.trast@student.ethz.ch","subject":"[PATCH v2 3/5] t3301: fix confusing quoting in test for valid notes ref","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T20:23:23Z","receivedAt":"2009-02-14T20:23:23Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The test used single quotes in the test code, but also single quotes\nto wrap the entire test snippet, thus effectively skipping _out_ of\nquoted mode. Since it doesn't matter here, just drop the quotes to\ncause less confusion.\n\nSigned-off-by: Thomas Rast \nAcked-by: Johannes Schindelin \n---\n\nDropped the removal of MSG=2.\n\n t/t3301-notes.sh | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex ff4ea05..43d82a2 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -31,8 +31,8 @@ test_expect_success setup '\n '\n \n test_expect_success 'need valid notes ref' '\n-\t! MSG=1 GIT_NOTES_REF='/' git notes edit &&\n-\t! MSG=2 GIT_NOTES_REF='/' git notes show\n+\t! MSG=1 GIT_NOTES_REF=/ git notes edit &&\n+\t! MSG=2 GIT_NOTES_REF=/ git notes show\n '\n \n # 1 indicates caught gracefully by die, 128 means git-show barked\n-- \n1.6.2.rc0.296.ge2122\n"},{"id":"104687","messageId":"8c50889d27e5baec1cbbd9a5775fa80b986c6df6.1234642638.git.trast@student.ethz.ch","threadId":"17794","inReplyTo":"200902142056.42198.trast@student.ethz.ch","subject":"[PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T20:23:24Z","receivedAt":"2009-02-14T20:23:24Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"We should use test_must_fail to negate output status, so that\nsegfaults can be caught. One instance of ! negation remains, but that\nactually tests the return code of 'grep'.\n\nSigned-off-by: Thomas Rast \n---\n\nNew in this series.\n\n t/t3301-notes.sh | 8 ++++----\n 1 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 43d82a2..764fad5 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -16,7 +16,7 @@ VISUAL=./fake_editor.sh\n export VISUAL\n \n test_expect_success 'cannot annotate non-existing HEAD' '\n-\t! MSG=3 git notes edit\n+\tMSG=3 test_must_fail git notes edit\n '\n \n test_expect_success setup '\n@@ -31,8 +31,8 @@ test_expect_success setup '\n '\n \n test_expect_success 'need valid notes ref' '\n-\t! MSG=1 GIT_NOTES_REF=/ git notes edit &&\n-\t! MSG=2 GIT_NOTES_REF=/ git notes show\n+\tMSG=1 GIT_NOTES_REF=/ test_must_fail git notes edit &&\n+\tMSG=2 GIT_NOTES_REF=/ test_must_fail git notes show\n '\n \n # 1 indicates caught gracefully by die, 128 means git-show barked\n@@ -47,7 +47,7 @@ test_expect_success 'create notes' '\n \ttest 1 = $(git ls-tree refs/notes/commits | wc -l) &&\n \ttest b1 = $(git notes show) &&\n \tgit show HEAD^ &&\n-\t! git notes show HEAD^\n+\ttest_must_fail git notes show HEAD^\n '\n \n cat > expect << EOF\n-- \n1.6.2.rc0.296.ge2122\n"},{"id":"104688","messageId":"aecacfd80a8b68dfe3090b303c95b42fa1169e76.1234642638.git.trast@student.ethz.ch","threadId":"17794","inReplyTo":"200902142056.42198.trast@student.ethz.ch","subject":"[PATCH v2 5/5] notes: refuse to edit notes outside refs/notes/","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-14T20:23:25Z","receivedAt":"2009-02-14T20:23:25Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The current git-notes has no safety valves whatsoever, and will\nhappily edit notes in, e.g., refs/remotes/. This is dangerous, as\nthey will be overwritten during the next fetch of the remote they\nbelong to.\n\nSince it is supposed to be a porcelain frontend for the notes feature,\nsimply forbid editing notes with GIT_NOTES_REF pointing anywhere but\nrefs/notes/*.\n\nSigned-off-by: Thomas Rast \nAcked-by: Johannes Schindelin \n---\n\nSame as v1, except for s/plumbing/porcelain/ in the commit message.\n\n git-notes.sh | 4 ++++\n t/t3301-notes.sh | 8 ++++++++\n 2 files changed, 12 insertions(+), 0 deletions(-)\n\ndiff --git a/git-notes.sh b/git-notes.sh\nindex 6859470..6ec33c9 100755\n--- a/git-notes.sh\n+++ b/git-notes.sh\n@@ -16,6 +16,10 @@ die \"Invalid commit: $@\"\n \n case \"$ACTION\" in\n edit)\n+\tif [ \"${GIT_NOTES_REF#refs/notes/}\" = \"$GIT_NOTES_REF\" ]; then\n+\t\tdie \"Refusing to edit notes in $GIT_NOTES_REF (outside of refs/notes/)\"\n+\tfi\n+\n \tMESSAGE=\"$GIT_DIR\"/new-notes-$COMMIT\n \ttrap '\n \t\ttest -f \"$MESSAGE\" && rm \"$MESSAGE\"\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 764fad5..f68d8ee 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -35,6 +35,14 @@ test_expect_success 'need valid notes ref' '\n \tMSG=2 GIT_NOTES_REF=/ test_must_fail git notes show\n '\n \n+test_expect_success 'refusing to edit in refs/heads/' '\n+\t! MSG=1 GIT_NOTES_REF=refs/heads/bogus git notes edit\n+'\n+\n+test_expect_success 'refusing to edit in refs/remotes/' '\n+\t! MSG=1 GIT_NOTES_REF=refs/remotes/bogus git notes edit\n+'\n+\n # 1 indicates caught gracefully by die, 128 means git-show barked\n test_expect_success 'handle empty notes gracefully' '\n \tgit notes show ; test 1 = $?\n-- \n1.6.2.rc0.296.ge2122\n"},{"id":"104701","messageId":"alpine.DEB.1.00.0902142226200.10279@pacific.mpi-cbg.de","threadId":"17794","inReplyTo":"8c50889d27e5baec1cbbd9a5775fa80b986c6df6.1234642638.git.trast@student.ethz.ch","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-14T21:26:47Z","receivedAt":"2009-02-14T21:26:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 14 Feb 2009, Thomas Rast wrote:\n\n> We should use test_must_fail to negate output status, so that\n> segfaults can be caught. One instance of ! negation remains, but that\n> actually tests the return code of 'grep'.\n\nACK.\n\nCiao,\nDscho\n"},{"id":"104738","messageId":"7vvdrcnnl0.fsf@gitster.siamese.dyndns.org","threadId":"17794","inReplyTo":"8c50889d27e5baec1cbbd9a5775fa80b986c6df6.1234642638.git.trast@student.ethz.ch","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-15T07:52:59Z","receivedAt":"2009-02-15T07:52:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> +\tMSG=3 test_must_fail git notes edit\n\ntest_must_fail is a shell function, and we have precedence 2d60615 (tests:\nAvoid single-shot environment export for shell function invocation,\n2009-01-26) to avoid this construct.\n"},{"id":"104739","messageId":"7vr620nnid.fsf@gitster.siamese.dyndns.org","threadId":"17794","inReplyTo":"4655551d70d8829a7a15219aef384deb4e63371a.1234642638.git.trast@student.ethz.ch","subject":"Re: [PATCH v2 1/5] notes: only clean up message file when editing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-15T07:54:34Z","receivedAt":"2009-02-15T07:54:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thomas Rast writes:\n\n> +\tMESSAGE=\"$GIT_DIR\"/new-notes-$COMMIT\n> +\ttrap '\n> +\t\ttest -f \"$MESSAGE\" && rm \"$MESSAGE\"\n> +\t' 0\n> +\n\nHmph, why not unconditionally rm -f \"$MESSAGE\" without test?\n"},{"id":"104785","messageId":"200902151711.45099.trast@student.ethz.ch","threadId":"17794","inReplyTo":"7vvdrcnnl0.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-15T16:11:40Z","receivedAt":"2009-02-15T16:11:40Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano wrote:\n> Thomas Rast writes:\n> \n> > +\tMSG=3 test_must_fail git notes edit\n> \n> test_must_fail is a shell function, and we have precedence 2d60615 (tests:\n> Avoid single-shot environment export for shell function invocation,\n> 2009-01-26) to avoid this construct.\n\nBah. I really have a bad memory, don't I?\n\nIs there a \"bare minimum\" POSIX shell that I can run the tests under,\nto save myself such embarassment in the future?\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"104793","messageId":"20090215181818.GA2291@coredump.intra.peff.net","threadId":"17794","inReplyTo":"200902151711.45099.trast@student.ethz.ch","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-15T18:18:18Z","receivedAt":"2009-02-15T18:18:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 15, 2009 at 05:11:40PM +0100, Thomas Rast wrote:\n\n> > > +\tMSG=3 test_must_fail git notes edit\n> > \n> > test_must_fail is a shell function, and we have precedence 2d60615 (tests:\n> > Avoid single-shot environment export for shell function invocation,\n> > 2009-01-26) to avoid this construct.\n> \n> Bah. I really have a bad memory, don't I?\n> \n> Is there a \"bare minimum\" POSIX shell that I can run the tests under,\n> to save myself such embarassment in the future?\n\nUsing \"dash\" will catch bash-isms, and is pretty commonly available, I\nthink. But this behavior, IIRC, happens on FreeBSD's /bin/sh, which is\nderived from \"ash\" (so is \"dash\", but I they have long since diverged).\n\nThere are even more quirks on more exotic systems, I expect, though I\nhave to admit that I gave up on Solaris and just started using bash\nthere. :)\n\nSo I don't think there is a catch-all shell that will help you, but some\n\"ash\" variant is probably your best bet.\n\n-Peff\n"},{"id":"104833","messageId":"200902152307.14571.trast@student.ethz.ch","threadId":"17794","inReplyTo":"20090215181818.GA2291@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-15T22:07:09Z","receivedAt":"2009-02-15T22:07:09Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Jeff King wrote:\n> Using \"dash\" will catch bash-isms, and is pretty commonly available, I\n> think. But this behavior, IIRC, happens on FreeBSD's /bin/sh, which is\n> derived from \"ash\" (so is \"dash\", but I they have long since diverged).\n\nThanks, that worked well enough. It's not in the opensuse repos, but\nhey, they track it with git, so it must be good ;-)\n\n> So I don't think there is a catch-all shell that will help you, but some\n> \"ash\" variant is probably your best bet.\n\nI tried plain \"ash\" (which is in the opensuse repos) but that falls\ndown due to lack of 'cd -P'.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"105099","messageId":"200902170944.08827.trast@student.ethz.ch","threadId":"17794","inReplyTo":"7vvdrcnnl0.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-17T08:44:04Z","receivedAt":"2009-02-17T08:44:04Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano wrote:\n> Thomas Rast writes:\n> \n> > +\tMSG=3 test_must_fail git notes edit\n> \n> test_must_fail is a shell function, and we have precedence 2d60615 (tests:\n> Avoid single-shot environment export for shell function invocation,\n> 2009-01-26) to avoid this construct.\n\nI see you took this into the pu branch (currently as 891840b). I\nassumed you would simply drop it, given the incompatibility? I'd\nrather have the slight chance of missing a segfault while git is\ntrying to execute a shell script (what are the odds that it only does\nso for git-notes?) than be killed by an angry mob of ash users ;-)\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"105100","messageId":"200902170946.52093.trast@student.ethz.ch","threadId":"17794","inReplyTo":"200902170944.08827.trast@student.ethz.ch","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-17T08:46:48Z","receivedAt":"2009-02-17T08:46:48Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Thomas Rast wrote:\n> Junio C Hamano wrote:\n> > Thomas Rast writes:\n> > \n> > > +\tMSG=3 test_must_fail git notes edit\n> > \n> > test_must_fail is a shell function, and we have precedence 2d60615 (tests:\n> > Avoid single-shot environment export for shell function invocation,\n> > 2009-01-26) to avoid this construct.\n> \n> I see you took this into the pu branch (currently as 891840b). I\n> assumed you would simply drop it, given the incompatibility? I'd\n> rather have the slight chance of missing a segfault while git is\n> trying to execute a shell script (what are the odds that it only does\n> so for git-notes?) than be killed by an angry mob of ash users ;-)\n\nOh, you fixed it up locally, and the next one in the series too.\nThanks, and sorry for the noise!\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"105111","messageId":"e2b179460902170129s7ae613cehe237619be5e84936@mail.gmail.com","threadId":"17794","inReplyTo":"20090215181818.GA2291@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Mike Ralphson","fromEmail":"mike.ralphson@gmail.com","sentAt":"2009-02-17T09:29:57Z","receivedAt":"2009-02-17T09:29:57Z","isPatch":true,"sender":{"key":"mike.ralphson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/21603?v=4"},"body":"2009/2/15 Jeff King :\n> On Sun, Feb 15, 2009 at 05:11:40PM +0100, Thomas Rast wrote:\n>> Is there a \"bare minimum\" POSIX shell that I can run the tests under,\n>> to save myself such embarassment in the future?\n>\n> Using \"dash\" will catch bash-isms, and is pretty commonly available, I\n> think. But this behavior, IIRC, happens on FreeBSD's /bin/sh, which is\n> derived from \"ash\" (so is \"dash\", but I they have long since diverged).\n>\n> There are even more quirks on more exotic systems, I expect, though I\n> have to admit that I gave up on Solaris and just started using bash\n> there. :)\n>\n> So I don't think there is a catch-all shell that will help you, but some\n> \"ash\" variant is probably your best bet.\n\nposh? http://packages.debian.org/lenny/posh\n\nI've heard that if you unset POSIXLY_CORRECT it just sits there and\nhums until you set it again. 8-)\n\nMike\n"},{"id":"105174","messageId":"20090217163413.GB31297@sigill.intra.peff.net","threadId":"17794","inReplyTo":"e2b179460902170129s7ae613cehe237619be5e84936@mail.gmail.com","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-17T16:34:13Z","receivedAt":"2009-02-17T16:34:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 17, 2009 at 09:29:57AM +0000, Mike Ralphson wrote:\n\n> posh? http://packages.debian.org/lenny/posh\n> \n> I've heard that if you unset POSIXLY_CORRECT it just sits there and\n> hums until you set it again. 8-)\n\nHmm. I tried \"make SHELL_PATH=/bin/posh test\", and posh segfaulted during\nt0005. So I don't think it's quite ready for mainstream use. ;)\n\n-Peff\n"},{"id":"105177","messageId":"tJ6pp2ZnBjKVBLynGfYewBaGmSehbwZNLOShEvpXmtkD3IMXRugEKg@cipher.nrlssc.navy.mil","threadId":"17794","inReplyTo":"200902170946.52093.trast@student.ethz.ch","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-02-17T16:56:36Z","receivedAt":"2009-02-17T16:56:36Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Thomas Rast wrote:\n> Thomas Rast wrote:\n>> Junio C Hamano wrote:\n>>> Thomas Rast writes:\n>>>\n>>>> +\tMSG=3 test_must_fail git notes edit\n>>> test_must_fail is a shell function, and we have precedence 2d60615 (tests:\n>>> Avoid single-shot environment export for shell function invocation,\n>>> 2009-01-26) to avoid this construct.\n>> I see you took this into the pu branch (currently as 891840b). I\n>> assumed you would simply drop it, given the incompatibility? I'd\n>> rather have the slight chance of missing a segfault while git is\n>> trying to execute a shell script (what are the odds that it only does\n>> so for git-notes?) than be killed by an angry mob of ash users ;-)\n> \n> Oh, you fixed it up locally, and the next one in the series too.\n> Thanks, and sorry for the noise!\n\nYeah, but what's this, introduced by 94859732?\n\nt/t3301-notes.sh line 40:\ntest_expect_success 'refusing to edit in refs/heads/' '\n (MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n test_must_fail git notes edit)\n'\n\ntest_expect_success 'refusing to edit in refs/remotes/' '\n (MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n test_must_fail git notes edit)\n'\n\n\nNotice the 'export' lines. Comparing it to Thomas's v2 5/5 patch, it\nlooks like the first line which sets the variables is correct and the\nexport line should just be 'export MSG GIT_NOTES_REF' in both tests.\n\n-brandon\n"},{"id":"105193","messageId":"e2b179460902171000s605675dct1f499a9425c3ebb2@mail.gmail.com","threadId":"17794","inReplyTo":"20090217163413.GB31297@sigill.intra.peff.net","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Mike Ralphson","fromEmail":"mike.ralphson@gmail.com","sentAt":"2009-02-17T18:00:17Z","receivedAt":"2009-02-17T18:00:17Z","isPatch":true,"sender":{"key":"mike.ralphson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/21603?v=4"},"body":"2009/2/17 Jeff King :\n> On Tue, Feb 17, 2009 at 09:29:57AM +0000, Mike Ralphson wrote:\n>> posh? http://packages.debian.org/lenny/posh\n>\n> Hmm. I tried \"make SHELL_PATH=/bin/posh test\", and posh segfaulted during\n> t0005. So I don't think it's quite ready for mainstream use. ;)\n\nWorks ok for me as far as t3404-rebase-interactive on next, though\nwhat I actually grabbed in the end was 0.6.16[1], which claims to fix\na segfault bug[2], but then it also claims to be 0.6.12 as well, so\nwho knows? 8-)\n\nMaybe a Debian user would like to report a repeat of [3]?\n\nMike\n\n[1] http://packages.debian.org/sid/posh\n[2] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=515278\n[3] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=397596\n"},{"id":"105214","messageId":"20090217202731.GA16586@coredump.intra.peff.net","threadId":"17794","inReplyTo":"e2b179460902171000s605675dct1f499a9425c3ebb2@mail.gmail.com","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-17T20:27:32Z","receivedAt":"2009-02-17T20:27:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 17, 2009 at 06:00:17PM +0000, Mike Ralphson wrote:\n\n> > Hmm. I tried \"make SHELL_PATH=/bin/posh test\", and posh segfaulted during\n> > t0005. So I don't think it's quite ready for mainstream use. ;)\n> \n> Works ok for me as far as t3404-rebase-interactive on next, though\n> what I actually grabbed in the end was 0.6.16[1], which claims to fix\n> a segfault bug[2], but then it also claims to be 0.6.12 as well, so\n> who knows? 8-)\n> \n> Maybe a Debian user would like to report a repeat of [3]?\n\nHmm. It sort of fixes the segfault bug. With posh 0.6.16, I can do:\n\n $ posh -x\n $ ls\n + ls\n ...\n\nbut:\n\n $ posh -x t0005-signals.sh\n Segmentation fault\n\nOK, maybe it doesn't like non-interactive shells. Let's try sourcing the\nscript:\n\n $ posh -x\n $ . t0005-signals.sh\n + . t0005-signals.sh\n posh: .: t0005-signals.sh: not found\n\nOK, maybe it is looking in the PATH?\n\n $ PATH=$PATH:.\n Segmentation fault\n\nOops. How about a new shell with a more exact path?\n\n $ posh -x\n $ . ./t0005-signals.sh\n Segmentation fault\n\nOops. So I think there are some serious problems (at least with -x).\n\nOf course it's hard to diagnose the non \"-x\" segfault in t0005 without\n\"-x\". ;)\n\nBut this is just unproductive complaining to do it on the git list (I'm\njust amazed at how frequently it is segfaulting). I'll go file some\nDebian bug reports.\n\n-Peff\n"},{"id":"105222","messageId":"7vr61w3dug.fsf@gitster.siamese.dyndns.org","threadId":"17794","inReplyTo":"tJ6pp2ZnBjKVBLynGfYewBaGmSehbwZNLOShEvpXmtkD3IMXRugEKg@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-17T22:20:23Z","receivedAt":"2009-02-17T22:20:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey writes:\n\n> Notice the 'export' lines. Comparing it to Thomas's v2 5/5 patch, it\n> looks like the first line which sets the variables is correct and the\n> export line should just be 'export MSG GIT_NOTES_REF' in both tests.\n\nThanks for catching my late-night typos.\n\nSome shells do not like \"export var=val\", and the right way to write these\nis to do an usual assignment and then export just variable names.\n\nWe need to fix the following:\n\n$ git grep -n -e 'export .*=' --and --not -e '-export' pu -- t/\npu:t/t3301-notes.sh:42:\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\npu:t/t3301-notes.sh:48:\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\npu:t/t3302-notes-index-expensive.sh:32:\t\texport GIT_INDEX_FILE=.git/temp;\npu:t/t3302-notes-index-expensive.sh:66:\t\t\texport GIT_NOTES_REF=non-existing\npu:t/t9301-fast-export.sh:188:export GIT_AUTHOR_NAME='A U Thor'\npu:t/t9301-fast-export.sh:189:export GIT_COMMITTER_NAME='C O Mitter'\npu:t/test-lib.sh:101:\t\texport GIT_TEST_LONG=t; shift ;;\n"},{"id":"105227","messageId":"1234911188-36941-1-git-send-email-jaysoffian@gmail.com","threadId":"17794","inReplyTo":"7vr61w3dug.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] test suite: correct export var=val usage","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-17T22:53:08Z","receivedAt":"2009-02-17T22:53:08Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Some shells do not like \"export var=val\", and the right way to write\nthese is to do an usual assignment and then export just variable names.\n---\n t/t3301-notes.sh | 6 +++---\n t/t3302-notes-index-expensive.sh | 4 ++--\n t/t9301-fast-export.sh | 4 ++--\n t/test-lib.sh | 2 +-\n 4 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 2321316..7ea98f2 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -38,14 +38,14 @@ test_expect_success 'need valid notes ref' '\n '\n \n test_expect_success 'refusing to edit in refs/heads/' '\n-\t(MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n-\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n+\t(MSG=1 GIT_NOTES_REF=refs/heads/bogus\n+\t && export MSG GIT_NOTES_REF &&\n \t test_must_fail git notes edit)\n '\n \n test_expect_success 'refusing to edit in refs/remotes/' '\n \t(MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n-\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n+\t export MSG GIT_NOTES_REF &&\n \t test_must_fail git notes edit)\n '\n \ndiff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh\nindex 00d27bf..0ef3e95 100755\n--- a/t/t3302-notes-index-expensive.sh\n+++ b/t/t3302-notes-index-expensive.sh\n@@ -29,7 +29,7 @@ create_repo () {\n \tdone &&\n \tgit update-ref refs/heads/master $commit &&\n \t{\n-\t\texport GIT_INDEX_FILE=.git/temp;\n+\t\tGIT_INDEX_FILE=.git/temp; export GIT_INDEX_FILE;\n \t\tgit rev-list HEAD | cat -n | sed \"s/^[ \t][ \t]*/ /g\" |\n \t\twhile read nr sha1; do\n \t\t\tblob=$(echo note $nr | git hash-object -w --stdin) &&\n@@ -63,7 +63,7 @@ cat > time_notes << \\EOF\n \twhile [ $i -lt $2 ]; do\n \t\tcase $1 in\n \t\tno-notes)\n-\t\t\texport GIT_NOTES_REF=non-existing\n+\t\t\tGIT_NOTES_REF=non-existing; export GIT_NOTES_REF\n \t\t;;\n \t\tnotes)\n \t\t\tunset GIT_NOTES_REF\ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 9985721..86c3760 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -185,8 +185,8 @@ test_expect_success 'submodule fast-export | fast-import' '\n \n '\n \n-export GIT_AUTHOR_NAME='A U Thor'\n-export GIT_COMMITTER_NAME='C O Mitter'\n+GIT_AUTHOR_NAME='A U Thor'; export GIT_AUTHOR_NAME\n+GIT_COMMITTER_NAME='C O Mitter'; export GIT_COMMITTER_NAME\n \n test_expect_success 'setup copies' '\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 2021446..7a847ec 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -98,7 +98,7 @@ do\n \t-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)\n \t\timmediate=t; shift ;;\n \t-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)\n-\t\texport GIT_TEST_LONG=t; shift ;;\n+\t\tGIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;\n \t-h|--h|--he|--hel|--help)\n \t\thelp=t; shift ;;\n \t-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)\n-- \n1.6.2.rc1.218.g1b4fab\n"},{"id":"105228","messageId":"1234911240-37028-1-git-send-email-jaysoffian@gmail.com","threadId":"17794","inReplyTo":"7vr61w3dug.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] test suite: correct export var=val usage","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-17T22:54:00Z","receivedAt":"2009-02-17T22:54:00Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Some shells do not like \"export var=val\", and the right way to write\nthese is to do an usual assignment and then export just variable names.\n\nSigned-off-by: Jay Soffian \n---\nThis time with SOB. Sorry.\n\n t/t3301-notes.sh | 6 +++---\n t/t3302-notes-index-expensive.sh | 4 ++--\n t/t9301-fast-export.sh | 4 ++--\n t/test-lib.sh | 2 +-\n 4 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 2321316..7ea98f2 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -38,14 +38,14 @@ test_expect_success 'need valid notes ref' '\n '\n \n test_expect_success 'refusing to edit in refs/heads/' '\n-\t(MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n-\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n+\t(MSG=1 GIT_NOTES_REF=refs/heads/bogus\n+\t && export MSG GIT_NOTES_REF &&\n \t test_must_fail git notes edit)\n '\n \n test_expect_success 'refusing to edit in refs/remotes/' '\n \t(MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n-\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n+\t export MSG GIT_NOTES_REF &&\n \t test_must_fail git notes edit)\n '\n \ndiff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh\nindex 00d27bf..0ef3e95 100755\n--- a/t/t3302-notes-index-expensive.sh\n+++ b/t/t3302-notes-index-expensive.sh\n@@ -29,7 +29,7 @@ create_repo () {\n \tdone &&\n \tgit update-ref refs/heads/master $commit &&\n \t{\n-\t\texport GIT_INDEX_FILE=.git/temp;\n+\t\tGIT_INDEX_FILE=.git/temp; export GIT_INDEX_FILE;\n \t\tgit rev-list HEAD | cat -n | sed \"s/^[ \t][ \t]*/ /g\" |\n \t\twhile read nr sha1; do\n \t\t\tblob=$(echo note $nr | git hash-object -w --stdin) &&\n@@ -63,7 +63,7 @@ cat > time_notes << \\EOF\n \twhile [ $i -lt $2 ]; do\n \t\tcase $1 in\n \t\tno-notes)\n-\t\t\texport GIT_NOTES_REF=non-existing\n+\t\t\tGIT_NOTES_REF=non-existing; export GIT_NOTES_REF\n \t\t;;\n \t\tnotes)\n \t\t\tunset GIT_NOTES_REF\ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 9985721..86c3760 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -185,8 +185,8 @@ test_expect_success 'submodule fast-export | fast-import' '\n \n '\n \n-export GIT_AUTHOR_NAME='A U Thor'\n-export GIT_COMMITTER_NAME='C O Mitter'\n+GIT_AUTHOR_NAME='A U Thor'; export GIT_AUTHOR_NAME\n+GIT_COMMITTER_NAME='C O Mitter'; export GIT_COMMITTER_NAME\n \n test_expect_success 'setup copies' '\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 2021446..7a847ec 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -98,7 +98,7 @@ do\n \t-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)\n \t\timmediate=t; shift ;;\n \t-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)\n-\t\texport GIT_TEST_LONG=t; shift ;;\n+\t\tGIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;\n \t-h|--h|--he|--hel|--help)\n \t\thelp=t; shift ;;\n \t-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)\n-- \n1.6.2.rc1.218.g1b4fab\n"},{"id":"105232","messageId":"1234911457-37188-1-git-send-email-jaysoffian@gmail.com","threadId":"17794","inReplyTo":"7vr61w3dug.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] test suite: correct export var=val usage","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-17T22:57:37Z","receivedAt":"2009-02-17T22:57:37Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Some shells do not like \"export var=val\", and the right way to write\nthese is to do an usual assignment and then export just variable names.\n\nSigned-off-by: Jay Soffian \n---\nArgh. Last time. How come I only notice typos after they go to the list?\nThe prior version had a '&&' on the wrong line. I think it wouldn't be\na problem because it's in a (...) subshell but this way is better.\n\n t/t3301-notes.sh | 4 ++--\n t/t3302-notes-index-expensive.sh | 4 ++--\n t/t9301-fast-export.sh | 4 ++--\n t/test-lib.sh | 2 +-\n 4 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 2321316..18c4eb6 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -39,13 +39,13 @@ test_expect_success 'need valid notes ref' '\n \n test_expect_success 'refusing to edit in refs/heads/' '\n \t(MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n-\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n+\texport MSG GIT_NOTES_REF &&\n \t test_must_fail git notes edit)\n '\n \n test_expect_success 'refusing to edit in refs/remotes/' '\n \t(MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n-\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n+\t export MSG GIT_NOTES_REF &&\n \t test_must_fail git notes edit)\n '\n \ndiff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh\nindex 00d27bf..0ef3e95 100755\n--- a/t/t3302-notes-index-expensive.sh\n+++ b/t/t3302-notes-index-expensive.sh\n@@ -29,7 +29,7 @@ create_repo () {\n \tdone &&\n \tgit update-ref refs/heads/master $commit &&\n \t{\n-\t\texport GIT_INDEX_FILE=.git/temp;\n+\t\tGIT_INDEX_FILE=.git/temp; export GIT_INDEX_FILE;\n \t\tgit rev-list HEAD | cat -n | sed \"s/^[ \t][ \t]*/ /g\" |\n \t\twhile read nr sha1; do\n \t\t\tblob=$(echo note $nr | git hash-object -w --stdin) &&\n@@ -63,7 +63,7 @@ cat > time_notes << \\EOF\n \twhile [ $i -lt $2 ]; do\n \t\tcase $1 in\n \t\tno-notes)\n-\t\t\texport GIT_NOTES_REF=non-existing\n+\t\t\tGIT_NOTES_REF=non-existing; export GIT_NOTES_REF\n \t\t;;\n \t\tnotes)\n \t\t\tunset GIT_NOTES_REF\ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 9985721..86c3760 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -185,8 +185,8 @@ test_expect_success 'submodule fast-export | fast-import' '\n \n '\n \n-export GIT_AUTHOR_NAME='A U Thor'\n-export GIT_COMMITTER_NAME='C O Mitter'\n+GIT_AUTHOR_NAME='A U Thor'; export GIT_AUTHOR_NAME\n+GIT_COMMITTER_NAME='C O Mitter'; export GIT_COMMITTER_NAME\n \n test_expect_success 'setup copies' '\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 2021446..7a847ec 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -98,7 +98,7 @@ do\n \t-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)\n \t\timmediate=t; shift ;;\n \t-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)\n-\t\texport GIT_TEST_LONG=t; shift ;;\n+\t\tGIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;\n \t-h|--h|--he|--hel|--help)\n \t\thelp=t; shift ;;\n \t-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)\n-- \n1.6.2.rc1.218.g1b4fab\n"},{"id":"105243","messageId":"alpine.DEB.1.00.0902180046290.10279@pacific.mpi-cbg.de","threadId":"17794","inReplyTo":"1234911188-36941-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] test suite: correct export var=val usage","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-17T23:47:25Z","receivedAt":"2009-02-17T23:47:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Feb 2009, Jay Soffian wrote:\n\n> +\t(MSG=1 GIT_NOTES_REF=refs/heads/bogus\n> +\t && export MSG GIT_NOTES_REF &&\n\nHmm. Why does the \"&&\" at the start of a line strike me as odd. Ah, yes, \nit is because no code around it uses that style.\n\n*grins*\n\nCiao,\nDscho\n"},{"id":"105252","messageId":"76718490902171637j55f97a7fg2a0ead846b88d3e6@mail.gmail.com","threadId":"17794","inReplyTo":"alpine.DEB.1.00.0902180046290.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH] test suite: correct export var=val usage","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-18T00:37:38Z","receivedAt":"2009-02-18T00:37:38Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, Feb 17, 2009 at 6:47 PM, Johannes Schindelin\n wrote:\n> Hi,\n>\n> On Tue, 17 Feb 2009, Jay Soffian wrote:\n>\n>> + (MSG=1 GIT_NOTES_REF=refs/heads/bogus\n>> + && export MSG GIT_NOTES_REF &&\n>\n> Hmm. Why does the \"&&\" at the start of a line strike me as odd. Ah, yes,\n> it is because no code around it uses that style.\n>\n> *grins*\n\nSometimes authors catch their own mistakes, but you have to slow down\non the reply button to notice. It's not like I didn't mail out the\ncorrection less than a minute afterwards.\n\n*double-grins*\n\nAnd in case your curious how that happened, it was because I had a\nsingle line, realized it was too long, and hit return in the wrong\nplace. Unfortunately I didn't catch my mistake till after I mailed it\nout.\n\nSo there. (sticking tongue out at Dscho)\n\nj.\n"},{"id":"105274","messageId":"20090218064121.GA16611@coredump.intra.peff.net","threadId":"17794","inReplyTo":"20090217202731.GA16586@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-18T06:41:21Z","receivedAt":"2009-02-18T06:41:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 17, 2009 at 03:27:31PM -0500, Jeff King wrote:\n\n> But this is just unproductive complaining to do it on the git list (I'm\n> just amazed at how frequently it is segfaulting). I'll go file some\n> Debian bug reports.\n\nThe maintainer was very responsive, fixing my bugs in about an hour\nafter they were reported.\n\nI can now get as far as t3404, which seems to have problems due to\nrebase-interactive using parentheses, \"-a\", and \"-o\" with \"test\". I'm\nnot sure how much time it is worth spending on this; I'm not sure anyone\nactually uses a shell as featureless as posh in real life (though I\nsuppose it could eventually become the Debian shell).\n\n-Peff\n"},{"id":"105284","messageId":"0E74A2BF-1E59-4D82-A750-380697667F10@wincent.com","threadId":"17794","inReplyTo":"1234911457-37188-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] test suite: correct export var=val usage","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2009-02-18T10:06:34Z","receivedAt":"2009-02-18T10:06:34Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 17/2/2009, a las 23:57, Jay Soffian escribió:\n\n> test_expect_success 'refusing to edit in refs/heads/' '\n> \t(MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n> -\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n> +\texport MSG GIT_NOTES_REF &&\n> \t test_must_fail git notes edit)\n> '\n>\n> test_expect_success 'refusing to edit in refs/remotes/' '\n> \t(MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n> -\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n> +\t export MSG GIT_NOTES_REF &&\n> \t test_must_fail git notes edit)\n> '\n\nPerhaps my eyes are playing tricks on me but I see the original \nversion setting MSG to an empty string and exporting it, and your \nversion setting MSG to \"1\" and exporting it. So which one is wrong? \nThe original or yours?\n\nCheers,\nWincent\n"},{"id":"105285","messageId":"alpine.DEB.1.00.0902181113210.6274@intel-tinevez-2-302","threadId":"17794","inReplyTo":"20090218064121.GA16611@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-18T10:14:10Z","receivedAt":"2009-02-18T10:14:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Feb 2009, Jeff King wrote:\n\n> I can now get as far as t3404, which seems to have problems due to\n> rebase-interactive using parentheses, \"-a\", and \"-o\" with \"test\".\n\nThe parentheses will be fixed soon, but \"-a\" and \"-o\" really are \nnecessary.\n\nCiao,\nDscho\n"},{"id":"105286","messageId":"20090218101615.GA23035@coredump.intra.peff.net","threadId":"17794","inReplyTo":"alpine.DEB.1.00.0902181113210.6274@intel-tinevez-2-302","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-18T10:16:15Z","receivedAt":"2009-02-18T10:16:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 18, 2009 at 11:14:10AM +0100, Johannes Schindelin wrote:\n\n> > I can now get as far as t3404, which seems to have problems due to\n> > rebase-interactive using parentheses, \"-a\", and \"-o\" with \"test\".\n> \n> The parentheses will be fixed soon, but \"-a\" and \"-o\" really are \n> necessary.\n\nReally? Are they not easily replaced with\n\n-test cond1 -a cond2 -o cond3\n+test cond1 && test cond2 || test cond3\n\n? The trickier part is the grouping, but that is what the parentheses\nare doing.\n\n-Peff\n"},{"id":"105293","messageId":"alpine.DEB.1.00.0902181251160.6274@intel-tinevez-2-302","threadId":"17794","inReplyTo":"20090218101615.GA23035@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-18T11:53:37Z","receivedAt":"2009-02-18T11:53:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Feb 2009, Jeff King wrote:\n\n> On Wed, Feb 18, 2009 at 11:14:10AM +0100, Johannes Schindelin wrote:\n> \n> > > I can now get as far as t3404, which seems to have problems due to\n> > > rebase-interactive using parentheses, \"-a\", and \"-o\" with \"test\".\n> > \n> > The parentheses will be fixed soon, but \"-a\" and \"-o\" really are \n> > necessary.\n> \n> Really? Are they not easily replaced with\n> \n> -test cond1 -a cond2 -o cond3\n> +test cond1 && test cond2 || test cond3\n\n...which is substantially harder to read.\n\n> ? The trickier part is the grouping, but that is what the parentheses \n> are doing.\n\nI got rid of that code about two weeks ago.\n\nThe upcoming rebase -i -p series is unfortunately not in a good shape at \nthe moment; intermediate stages fail the tests, so I will have to clean up \na bit.\n\nBut at the moment, real world is taking my time, reducing my Git time \nbudget substantially. On the positive side (well, half of it), I \nfiddle with msysGit these days, as have to use Windows again.\n\nCiao,\nDscho\n"},{"id":"105297","messageId":"76718490902180519s3b5f4469m391aa34456059d37@mail.gmail.com","threadId":"17794","inReplyTo":"0E74A2BF-1E59-4D82-A750-380697667F10@wincent.com","subject":"Re: [PATCH] test suite: correct export var=val usage","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-18T13:19:15Z","receivedAt":"2009-02-18T13:19:15Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Feb 18, 2009 at 5:06 AM, Wincent Colaiuta wrote:\n> Perhaps my eyes are playing tricks on me but I see the original version\n> setting MSG to an empty string and exporting it, and your version setting\n> MSG to \"1\" and exporting it. So which one is wrong? The original or yours?\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/109897/focus=110462\n\nj.\n"},{"id":"105301","messageId":"76718490902180529w7520ba64kb7d0d6b284b406bd@mail.gmail.com","threadId":"17794","inReplyTo":"76718490902180519s3b5f4469m391aa34456059d37@mail.gmail.com","subject":"Re: [PATCH] test suite: correct export var=val usage","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-18T13:29:05Z","receivedAt":"2009-02-18T13:29:05Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Wed, Feb 18, 2009 at 8:19 AM, Jay Soffian wrote:\n> On Wed, Feb 18, 2009 at 5:06 AM, Wincent Colaiuta wrote:\n>> Perhaps my eyes are playing tricks on me but I see the original version\n>> setting MSG to an empty string and exporting it, and your version setting\n>> MSG to \"1\" and exporting it. So which one is wrong? The original or yours?\n>\n> http://thread.gmane.org/gmane.comp.version-control.git/109897/focus=110462\n\nRather, http://thread.gmane.org/gmane.comp.version-control.git/109897/focus=110457\n\n(Cursed frame interface.)\n\n(The original is wrong.)\n\nj.\n"},{"id":"105304","messageId":"alpine.DEB.1.00.0902181433550.6274@intel-tinevez-2-302","threadId":"17794","inReplyTo":"76718490902180529w7520ba64kb7d0d6b284b406bd@mail.gmail.com","subject":"Re: [PATCH] test suite: correct export var=val usage","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-18T13:34:51Z","receivedAt":"2009-02-18T13:34:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Feb 2009, Jay Soffian wrote:\n\n> On Wed, Feb 18, 2009 at 8:19 AM, Jay Soffian wrote:\n> > On Wed, Feb 18, 2009 at 5:06 AM, Wincent Colaiuta wrote:\n> >> Perhaps my eyes are playing tricks on me but I see the original version\n> >> setting MSG to an empty string and exporting it, and your version setting\n> >> MSG to \"1\" and exporting it. So which one is wrong? The original or yours?\n> >\n> > http://thread.gmane.org/gmane.comp.version-control.git/109897/focus=110462\n> \n> Rather, http://thread.gmane.org/gmane.comp.version-control.git/109897/focus=110457\n> \n> (Cursed frame interface.)\n> \n> (The original is wrong.)\n\nMaybe the time would be better spent paraphrasing the gist of the thread, \ninstead of cursing?\n\nBesides, where du you find a curses interface to gmane?\n\n:-)\n\nCiao,\nDscho\n"},{"id":"105322","messageId":"F21AE3A2-480B-4311-81E7-32B75B49A356@wincent.com","threadId":"17794","inReplyTo":"76718490902180529w7520ba64kb7d0d6b284b406bd@mail.gmail.com","subject":"Re: [PATCH] test suite: correct export var=val usage","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2009-02-18T16:56:38Z","receivedAt":"2009-02-18T16:56:38Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 18/2/2009, a las 14:29, Jay Soffian escribió:\n\n> On Wed, Feb 18, 2009 at 8:19 AM, Jay Soffian \n> wrote:\n>> On Wed, Feb 18, 2009 at 5:06 AM, Wincent Colaiuta \n>> wrote:\n>>> Perhaps my eyes are playing tricks on me but I see the original \n>>> version\n>>> setting MSG to an empty string and exporting it, and your version \n>>> setting\n>>> MSG to \"1\" and exporting it. So which one is wrong? The original \n>>> or yours?\n>>\n>> http://thread.gmane.org/gmane.comp.version-control.git/109897/focus=110462\n>\n> Rather, http://thread.gmane.org/gmane.comp.version-control.git/109897/focus=110457\n>\n> (Cursed frame interface.)\n>\n> (The original is wrong.)\n\nI'm definitely blind then, or perhaps I didn't explain myself. At \nleast, I saw nothing in the message you linked that answers my \nquestion. Let me try again.\n\n- Prior to your patch, in the two hunks I quoted we set MSG to an \nempty string and exported it\n\n- After your patch, the hunks now set MSG to \"1\" (not the same string) \nand export it\n\nIn other words, you not only changed the _style_ from \"assign and \nexport in a single step\" to \"assign and then export as two separate \nsteps\"; you also changed _what_ gets exported in two of those hunks.\n\nSo my question really was, was this intended?\n\nCheers,\nWincent\n"},{"id":"105325","messageId":"200902181805.44145.trast@student.ethz.ch","threadId":"17794","inReplyTo":"F21AE3A2-480B-4311-81E7-32B75B49A356@wincent.com","subject":"Re: [PATCH] test suite: correct export var=val usage","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2009-02-18T17:05:36Z","receivedAt":"2009-02-18T17:05:36Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Wincent Colaiuta wrote:\n> I'm definitely blind then, or perhaps I didn't explain myself. At \n> least, I saw nothing in the message you linked that answers my \n> question. Let me try again.\n> \n> - Prior to your patch, in the two hunks I quoted we set MSG to an \n> empty string and exported it\n> \n> - After your patch, the hunks now set MSG to \"1\" (not the same string) \n> and export it\n> \n> In other words, you not only changed the _style_ from \"assign and \n> export in a single step\" to \"assign and then export as two separate \n> steps\"; you also changed _what_ gets exported in two of those hunks.\n\nActually the original version was\n\n http://article.gmane.org/gmane.comp.version-control.git/109920\n\nwhich was subsequently fixed by Junio to not use a single-shot export.\nThen Brandon noticed that during the fix-up, the export line suddenly\nstarted exporting values itself, and different ones at that.\n\nSo while Jay's patch indeed changes the semantics, the net result is\nwhat was intended all along, except it's now (hopefully) compatible\nwith more shells.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"105339","messageId":"1234990515-1636-1-git-send-email-jaysoffian@gmail.com","threadId":"17794","inReplyTo":"7vr61w3dug.fsf@gitster.siamese.dyndns.org","subject":"[PATCH v2] test suite: correct export var=val usage","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-18T20:55:15Z","receivedAt":"2009-02-18T20:55:15Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Some shells do not like \"export var=val\", and the right way to write\nthese is to do an usual assignment and then export just variable names.\n\nAlso corrects a typo in the tests added by 94859732 and noticed by\nBrandon Casey (gmane 110412):\n\n test_expect_success 'refusing to edit in refs/heads/' '\n (MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n test_must_fail git notes edit)\n '\n\n test_expect_success 'refusing to edit in refs/remotes/' '\n (MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n test_must_fail git notes edit)\n '\n\n Notice the 'export' lines. Comparing it to Thomas's v2 5/5 patch, it\n looks like the first line which sets the variables is correct and\n the export line should just be 'export MSG GIT_NOTES_REF' in both\n tests.\n\nSigned-off-by: Jay Soffian \n---\nThis time with a better (I hope...) commit message to clarify\nthat we are fixing a typo and not introducing one.\n\n t/t3301-notes.sh | 4 ++--\n t/t3302-notes-index-expensive.sh | 4 ++--\n t/t9301-fast-export.sh | 4 ++--\n t/test-lib.sh | 2 +-\n 4 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 2321316..18c4eb6 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -39,13 +39,13 @@ test_expect_success 'need valid notes ref' '\n \n test_expect_success 'refusing to edit in refs/heads/' '\n \t(MSG=1 GIT_NOTES_REF=refs/heads/bogus &&\n-\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n+\texport MSG GIT_NOTES_REF &&\n \t test_must_fail git notes edit)\n '\n \n test_expect_success 'refusing to edit in refs/remotes/' '\n \t(MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&\n-\t export MSG= GIT_NOTES_REF=refs/heads/bogus &&\n+\t export MSG GIT_NOTES_REF &&\n \t test_must_fail git notes edit)\n '\n \ndiff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh\nindex 00d27bf..0ef3e95 100755\n--- a/t/t3302-notes-index-expensive.sh\n+++ b/t/t3302-notes-index-expensive.sh\n@@ -29,7 +29,7 @@ create_repo () {\n \tdone &&\n \tgit update-ref refs/heads/master $commit &&\n \t{\n-\t\texport GIT_INDEX_FILE=.git/temp;\n+\t\tGIT_INDEX_FILE=.git/temp; export GIT_INDEX_FILE;\n \t\tgit rev-list HEAD | cat -n | sed \"s/^[ \t][ \t]*/ /g\" |\n \t\twhile read nr sha1; do\n \t\t\tblob=$(echo note $nr | git hash-object -w --stdin) &&\n@@ -63,7 +63,7 @@ cat > time_notes << \\EOF\n \twhile [ $i -lt $2 ]; do\n \t\tcase $1 in\n \t\tno-notes)\n-\t\t\texport GIT_NOTES_REF=non-existing\n+\t\t\tGIT_NOTES_REF=non-existing; export GIT_NOTES_REF\n \t\t;;\n \t\tnotes)\n \t\t\tunset GIT_NOTES_REF\ndiff --git a/t/t9301-fast-export.sh b/t/t9301-fast-export.sh\nindex 9985721..86c3760 100755\n--- a/t/t9301-fast-export.sh\n+++ b/t/t9301-fast-export.sh\n@@ -185,8 +185,8 @@ test_expect_success 'submodule fast-export | fast-import' '\n \n '\n \n-export GIT_AUTHOR_NAME='A U Thor'\n-export GIT_COMMITTER_NAME='C O Mitter'\n+GIT_AUTHOR_NAME='A U Thor'; export GIT_AUTHOR_NAME\n+GIT_COMMITTER_NAME='C O Mitter'; export GIT_COMMITTER_NAME\n \n test_expect_success 'setup copies' '\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 2021446..7a847ec 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -98,7 +98,7 @@ do\n \t-i|--i|--im|--imm|--imme|--immed|--immedi|--immedia|--immediat|--immediate)\n \t\timmediate=t; shift ;;\n \t-l|--l|--lo|--lon|--long|--long-|--long-t|--long-te|--long-tes|--long-test|--long-tests)\n-\t\texport GIT_TEST_LONG=t; shift ;;\n+\t\tGIT_TEST_LONG=t; export GIT_TEST_LONG; shift ;;\n \t-h|--h|--he|--hel|--help)\n \t\thelp=t; shift ;;\n \t-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)\n-- \n1.6.2.rc1.218.g1b4fab\n"},{"id":"105361","messageId":"20090219003727.GA25808@coredump.intra.peff.net","threadId":"17794","inReplyTo":"alpine.DEB.1.00.0902181251160.6274@intel-tinevez-2-302","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-19T00:37:27Z","receivedAt":"2009-02-19T00:37:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 18, 2009 at 12:53:37PM +0100, Johannes Schindelin wrote:\n\n> > Really? Are they not easily replaced with\n> > \n> > -test cond1 -a cond2 -o cond3\n> > +test cond1 && test cond2 || test cond3\n> \n> ...which is substantially harder to read.\n\nI don't agree that it is harder to read, but that is beside the point.\nWhat is important is whether or not the construct is portable enough to\nmeet git's standards.\n\n\"-a\" and \"-o\" are XSI extensions to POSIX, which is usually a sign that\nthere may be problems. However, besides posh (which at this point I\nthink can be considered a compliance-testing shell and not an actual\nshell in use), I haven't heard of problems in practice. Even FreeBSD's\nash derivative supports it (along with parentheses).\n\nSo I don't think it needs to be changed (which is what I said in my\noriginal message). But I also think saying \"-a and -o are necessary\" is\nnot true; they can be replaced if it turns out to be a problem.\n\n-Peff\n"},{"id":"105363","messageId":"alpine.DEB.1.00.0902190142440.10279@pacific.mpi-cbg.de","threadId":"17794","inReplyTo":"20090219003727.GA25808@coredump.intra.peff.net","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-19T00:46:28Z","receivedAt":"2009-02-19T00:46:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Feb 2009, Jeff King wrote:\n\n> On Wed, Feb 18, 2009 at 12:53:37PM +0100, Johannes Schindelin wrote:\n> \n> > > Really? Are they not easily replaced with\n> > > \n> > > -test cond1 -a cond2 -o cond3\n> > > +test cond1 && test cond2 || test cond3\n> > \n> > ...which is substantially harder to read.\n> \n> I don't agree that it is harder to read, but that is beside the point.\n> What is important is whether or not the construct is portable enough to\n> meet git's standards.\n> \n> \"-a\" and \"-o\" are XSI extensions to POSIX, which is usually a sign that\n> there may be problems. However, besides posh (which at this point I\n> think can be considered a compliance-testing shell and not an actual\n> shell in use), I haven't heard of problems in practice. Even FreeBSD's\n> ash derivative supports it (along with parentheses).\n> \n> So I don't think it needs to be changed (which is what I said in my\n> original message). But I also think saying \"-a and -o are necessary\" is\n> not true; they can be replaced if it turns out to be a problem.\n\nEven if they were in POSIX, I'd suggest not to change the constructs. \nrebase is _bound_ to be built in.\n\nEven if Stephan is not a fan of my recent changes to rebase -i -p (which I \nwill present on this list once they all work as I want them to), I think I \ncan talk him into continuing the sequencer effort: he was not discussing \nthe design in the open, so he should have expected the process to be \ndragged out.\n\nTherefore, any changes to git-rebase--interactive (or for that matter, \ngit-rebase) before the sequencer would be wasted work.\n\nCiao,\nDscho\n"},{"id":"105364","messageId":"20090219004633.GC25808@coredump.intra.peff.net","threadId":"17794","inReplyTo":"alpine.DEB.1.00.0902190142440.10279@pacific.mpi-cbg.de","subject":"Re: [PATCH v2 4/5] t3301: use test_must_fail instead of !","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-19T00:46:33Z","receivedAt":"2009-02-19T00:46:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 19, 2009 at 01:46:28AM +0100, Johannes Schindelin wrote:\n\n> Even if they were in POSIX, I'd suggest not to change the constructs. \n> rebase is _bound_ to be built in.\n> \n> Even if Stephan is not a fan of my recent changes to rebase -i -p (which I \n> will present on this list once they all work as I want them to), I think I \n> can talk him into continuing the sequencer effort: he was not discussing \n> the design in the open, so he should have expected the process to be \n> dragged out.\n\nHaving a sequencer in C would make me even happier. ;)\n\n-Peff\n"}]}