{"thread":{"id":"23075","subject":"Problem with contrib/hooks/post-receive-email","startedAt":"2010-03-19T06:39:02Z","lastAt":"2010-03-20T03:21:37Z","messageCount":4,"participants":["Eli Barzilay","Andy Parkins","Brandon Casey"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"137185","messageId":"m3vdcsq0hl.fsf@winooski.ccs.neu.edu","threadId":"23075","inReplyTo":null,"subject":"Problem with contrib/hooks/post-receive-email","fromName":"Eli Barzilay","fromEmail":"eli@barzilay.org","sentAt":"2010-03-19T06:39:02Z","receivedAt":"2010-03-19T06:39:02Z","isPatch":false,"sender":{"key":"eli@barzilay.org","avatar":"https://avatars.githubusercontent.com/u/185905?v=4"},"body":"The post-receive-email script goes out of its way to avoid sending\ncommits twice by filtering out commits that are included in existing\nrefs, but if more than one branch changes then some commits can end up\nnot being reported.  For example, I made two commits A and B, made one\nbranch point at A and another at B, and pushed both -- neither of the\nresulting two emails had A.\n-- \n          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:\n                    http://barzilay.org/                   Maze is Life!\n"},{"id":"137239","messageId":"ho09bh$hdh$1@dough.gmane.org","threadId":"23075","inReplyTo":"m3vdcsq0hl.fsf@winooski.ccs.neu.edu","subject":"Re: Problem with contrib/hooks/post-receive-email","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2010-03-19T16:39:12Z","receivedAt":"2010-03-19T16:39:12Z","isPatch":false,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"Eli Barzilay wrote:\n\n> The post-receive-email script goes out of its way to avoid sending\n> commits twice by filtering out commits that are included in existing\n> refs, but if more than one branch changes then some commits can end up\n> not being reported.  For example, I made two commits A and B, made one\n> branch point at A and another at B, and pushed both -- neither of the\n> resulting two emails had A.\n\n<Andy starts crying>\n\nI can't see any way to deal with this case easily with post-receive-email as \nit is.  It inherently processes ref-by-ref.  The relevant bit of script is \nin generate_update_branch_email().  The comments explain how the same \nproblem is addressed for another developer changing the same branch before \npost-receive-email runs, but after the update is performed.  I think the \nsame method could be applied.\n\n  git rev-parse --not --all | grep -v $(git rev-parse $refname)\n\nThis line is where the particular branches are being included and excluded.  \nThe problem you have is that \"--all\" means \"--all-at-the-moment\", and you \nwant \"--all-as-they-were-before-the-update\".\n\nSo, --all will have to go, and a manual list built instead.  The supplied \nchange list includes all the information necessary:\n\n ref1_oldrev ref1_newrev ref1\n ref2_oldrev ref2_newrev ref2\n ref3_oldrev ref3_newrev ref3\n ref4_oldrev ref4_newrev ref4\n\nLet's say there is also a ref5 and ref6 in the repository.  The revision \nlist we want for (say) the ref1 call to generate_email would be:\n\n ref1_newrev\n ^ref2_oldrev\n ^ref3_oldrev\n ^ref4_oldrev\n ^ref5\n ^ref6\n\nAnd similarly for ref2, ref3 and ref4.  It seems to me that it needs a hash \ntable keyed on the refname, but I have no idea how to do that in bash.\n\n %originalreftable{\"ref1\"} = \"^ref1_oldrev\"\n %originalreftable{\"ref2\"} = \"^ref2_oldrev\"\n %originalreftable{\"ref3\"} = \"^ref3_oldrev\"\n %originalreftable{\"ref4\"} = \"^ref4_oldrev\"\n %originalreftable{\"ref5\"} = \"^ref5\"\n %originalreftable{\"ref6\"} = \"^ref6\"\n\nThis table would be sufficient to create the revision list for every \ngenerate_email(), because each generate_email() knows which ref it's being \nupdated for, so could easily do:\n\n %originalreftable{$myref} = \"$mynewrev\"\n\nBefore using the table (and restore it afterwards).\n\nIn short: yuck.  It feels an awful lot like its pushing the boundaries of \nwhat is sensible to do in shell script.\n\n\n\nAndy\n-- \nDr Andy Parkins\nandyparkins@gmail.com\n"},{"id":"137253","messageId":"pT1haZZsyBxOUoBofAkY_SnkAbeYO1Pm-blvjUr1xIzpQgSbduiE4w@cipher.nrlssc.navy.mil","threadId":"23075","inReplyTo":"ho09bh$hdh$1@dough.gmane.org","subject":"Re: Problem with contrib/hooks/post-receive-email","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2010-03-19T18:44:13Z","receivedAt":"2010-03-19T18:44:13Z","isPatch":false,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"On 03/19/2010 11:39 AM, Andy Parkins wrote:\n> Eli Barzilay wrote:\n> \n>> The post-receive-email script goes out of its way to avoid sending\n>> commits twice by filtering out commits that are included in existing\n>> refs, but if more than one branch changes then some commits can end up\n>> not being reported.  For example, I made two commits A and B, made one\n>> branch point at A and another at B, and pushed both -- neither of the\n>> resulting two emails had A.\n> \n> <Andy starts crying>\n> \n> I can't see any way to deal with this case easily with post-receive-email as \n> it is.  It inherently processes ref-by-ref.  The relevant bit of script is \n> in generate_update_branch_email().  The comments explain how the same \n> problem is addressed for another developer changing the same branch before \n> post-receive-email runs, but after the update is performed.  I think the \n> same method could be applied.\n> \n>   git rev-parse --not --all | grep -v $(git rev-parse $refname)\n> \n> This line is where the particular branches are being included and excluded.  \n> The problem you have is that \"--all\" means \"--all-at-the-moment\", and you \n> want \"--all-as-they-were-before-the-update\".\n> \n> So, --all will have to go, and a manual list built instead.  The supplied \n> change list includes all the information necessary:\n> \n>  ref1_oldrev ref1_newrev ref1\n>  ref2_oldrev ref2_newrev ref2\n>  ref3_oldrev ref3_newrev ref3\n>  ref4_oldrev ref4_newrev ref4\n> \n> Let's say there is also a ref5 and ref6 in the repository.  The revision \n> list we want for (say) the ref1 call to generate_email would be:\n> \n>  ref1_newrev\n>  ^ref2_oldrev\n>  ^ref3_oldrev\n>  ^ref4_oldrev\n>  ^ref5\n>  ^ref6\n> \n> And similarly for ref2, ref3 and ref4.  It seems to me that it needs a hash \n> table keyed on the refname, but I have no idea how to do that in bash.\n> \n>  %originalreftable{\"ref1\"} = \"^ref1_oldrev\"\n>  %originalreftable{\"ref2\"} = \"^ref2_oldrev\"\n>  %originalreftable{\"ref3\"} = \"^ref3_oldrev\"\n>  %originalreftable{\"ref4\"} = \"^ref4_oldrev\"\n>  %originalreftable{\"ref5\"} = \"^ref5\"\n>  %originalreftable{\"ref6\"} = \"^ref6\"\n> \n> This table would be sufficient to create the revision list for every \n> generate_email(), because each generate_email() knows which ref it's being \n> updated for, so could easily do:\n> \n>  %originalreftable{$myref} = \"$mynewrev\"\n> \n> Before using the table (and restore it afterwards).\n> \n> In short: yuck.  It feels an awful lot like its pushing the boundaries of \n> what is sensible to do in shell script.\n\nHere's how I did it.  I did this so quickly and so long ago that I\nforgot about giving it a final think-through and submitting it.  Now\nthat I look at it again, and the fact that it has been working for a\ngood long while, it doesn't look as ugly as I remember it.\n\nWhat do you think? Sane enough?\n\nWarning: copy/pasted so beware white space corruption\n\ndiff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email\nindex 58a35c8..42dc4e7 100755\n--- a/contrib/hooks/post-receive-email\n+++ b/contrib/hooks/post-receive-email\n@@ -619,9 +619,8 @@ show_new_revisions()\n                revspec=$oldrev..$newrev\n        fi\n\n-       other_branches=$(git for-each-ref --format='%(refname)' refs/heads/ |\n-           grep -F -v $refname)\n-       git rev-parse --not $other_branches |\n+       git rev-parse --not --branches | grep -v $(git rev-parse $refname) |\n+               sed $sed_rev_script |\n        if [ -z \"$custom_showrev\" ]\n        then\n                git rev-list --pretty --stdin $revspec\n@@ -680,10 +679,22 @@ if [ -n \"$1\" -a -n \"$2\" -a -n \"$3\" ]; then\n        # Output to the terminal in command line mode - if someone wanted to\n        # resend an email; they could redirect the output to sendmail\n        # themselves\n+       sed_rev_script=\"-e s/$3/$2/\"\n        PAGER= generate_email $2 $3 $1\n else\n+       numrevs=0\n        while read oldrev newrev refname\n        do\n-               generate_email $oldrev $newrev $refname | send_mail\n+               sed_rev_script=\"$sed_rev_script -e s/$newrev/$oldrev/\"\n+               oldrevs[$numrevs]=$oldrev\n+               newrevs[$numrevs]=$newrev\n+               refnames[$numrevs]=$refname\n+               numrevs=$(($numrevs + 1))\n+       done\n+       i=0\n+       while [ $i -lt $numrevs ]; do\n+               generate_email ${oldrevs[$i]} ${newrevs[$i]} ${refnames[$i]} |\n+                       send_mail\n+               i=$(($i + 1))\n        done\n fi\n"},{"id":"137301","messageId":"m3r5nfptj2.fsf@winooski.ccs.neu.edu","threadId":"23075","inReplyTo":"ho09bh$hdh$1@dough.gmane.org","subject":"Re: Problem with contrib/hooks/post-receive-email","fromName":"Eli Barzilay","fromEmail":"eli@barzilay.org","sentAt":"2010-03-20T03:21:37Z","receivedAt":"2010-03-20T03:21:37Z","isPatch":false,"sender":{"key":"eli@barzilay.org","avatar":"https://avatars.githubusercontent.com/u/185905?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> Eli Barzilay wrote:\n>\n>> The post-receive-email script goes out of its way to avoid sending\n>> commits twice by filtering out commits that are included in\n>> existing refs, but if more than one branch changes then some\n>> commits can end up not being reported.  For example, I made two\n>> commits A and B, made one branch point at A and another at B, and\n>> pushed both -- neither of the resulting two emails had A.\n>\n> <Andy starts crying>\n>\n> I can't see any way to deal with this case easily with\n> post-receive-email as it is.  It inherently processes ref-by-ref.\n\nWell, one thing that sounded obvious to me is to expicitly say\nsomething about it.  The \"danger\" that I see in this is a central\nrepository setup and people relying on knowing everything that happens\nby reading through these emails -- then get a bad surprise when\nsomething sneaks in past those emails.\n\nAs for a proper solution, I first thought along the lines of what\nBrandon suggested -- but I considered doing that with a hash table\ninstead of accumulating a sed script.\n\n\n> And similarly for ref2, ref3 and ref4.  It seems to me that it needs\n> a hash table keyed on the refname, but I have no idea how to do that\n> in bash.\n> [...]\n\n(Isn't that just associative arrays?)\n\n\n> In short: yuck.  It feels an awful lot like its pushing the\n> boundaries of what is sensible to do in shell script.\n\nYeah, my conclusion was similar...  But after considering it for a\nwhile, I think that a saner approach is to choose a main branch (eg,\n`master' or `devel') where all commits are always reported, then for\nbranches show only commits that are not on this main branch.  This\nmeans that if you only read the emails on this branch you know\npractically everything that happens, and if you're interested in what\nhappens on a branch you will see some commits again in the future when\nthey're added to the main branch -- but that seems even better to me:\neven I read some commit when it happened on a branch, I'd still want\nto know when it's added to the main branch.\n\nThis seems to me both more predictable in the sense of the\nnotifications and the code.  But I'm in the process of converting a\nproject to git so I might not be experienced enough...\n\n-- \n          ((lambda (x) (x x)) (lambda (x) (x x)))          Eli Barzilay:\n                    http://barzilay.org/                   Maze is Life!\n"}]}