{"thread":{"id":"39068","subject":"Feature Request: provide cmdline args to git hooks","startedAt":"2015-04-14T03:42:45Z","lastAt":"2015-04-15T18:18:56Z","messageCount":6,"participants":["Chris O'Kelly","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"259322","messageId":"CAM-hpQcgOgrAJBAZrH4icDtJMiHUYOz1tynYkoj7qzyVMZ0eVQ@mail.gmail.com","threadId":"39068","inReplyTo":null,"subject":"Feature Request: provide cmdline args to git hooks","fromName":"Chris O'Kelly","fromEmail":"chris@mapcreative.com.au","sentAt":"2015-04-14T03:42:45Z","receivedAt":"2015-04-14T03:42:45Z","isPatch":false,"sender":{"key":"chris@mapcreative.com.au","avatar":null},"body":"Hello,\nJust a brief note about a feature I would find incredibly useful, were\nit available.\n\nA brief background of my use case:\nI am wanting to write a pre-push hook to prevent tags being pushed to\nour production servers. The production servers in our case are --bare\nendpoints, and when we push a tag at them, they always checkout the\ncommit that tag is attached to via some post-receive magic (WPEngine,\nFWIW). This behavior is even present when the tag was already pushed\nto the repo previously, if for instance a normal push is made with the\n--tags argument. In the past we have had problems when a developer is\nusing a GUI like SourceTree and accidentally leaves the 'push all\ntags' option checked, pushing 100s of tags to the server, which then\ndutifully begins checking out each in turn.\n\nCurrently I check for tag refs being pushed with:\n#...SNIP...\nwhile read local_ref local_sha remote_ref remote_sha\ndo\nlocal_type=$(echo \"$local_ref\" | awk -F/ '{print $2}')\nremote_type=$(echo \"$remote_ref\" | awk -F/ '{print $2}')\nif [[ \"$no_tags\" -eq 1 && ($local_type = \"tags\" || $remote_type = \"tags\")]]\nthen\necho \"Detected attempt to push tags to a no_tags repo! Exiting without push...\"\nexit 1\nfi\n#...SNIP...\n\nwhich works fine the first time tags are pushed, but when they are\nalready up to date as of the last fetch, they are not passed to the\nstdin for pre-push, so I cannot detect them.\n\nin a linux environment we can inspect /proc/$PPID/cmdline or ps\n-ocommand= -p $PPID to find the --tags argument (or any manually\nspecified tag refs, etc), however commonly developers are using\nWindows with SourceTree, and the pseudo-nix environment it provides\nlacks a /proc directory and uses the cut down cygwin version of ps. I\nhave considered going down the parsing route with general ps output,\nreading line by line to find the appropriate ppid, then echoing the\noutput, but varying output and column order of ps between SourceTree\nand various linux versions looks to make that infeasible as a portable\nsolution.\n\nIf we could access the original git push commandline inside the hook,\neither through a parameter passed directly to the script or possibly a\nGIT_* environment variable, it would make this possible. My specific\nuse case may not be incredibly common, but this could also be used to\ncheck if, for example, a push is being forced in a portable fashion -\nsomething I can see being useful for a pre-push hook in a variety of\ncircumstances.\n\nAlternatively - am I missing the super easy (and probably super\nobvious) way to do this with the existing tools?\n"},{"id":"259351","messageId":"xmqqwq1e3fy4.fsf@gitster.dls.corp.google.com","threadId":"39068","inReplyTo":"CAM-hpQcgOgrAJBAZrH4icDtJMiHUYOz1tynYkoj7qzyVMZ0eVQ@mail.gmail.com","subject":"Re: Feature Request: provide cmdline args to git hooks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-14T15:08:51Z","receivedAt":"2015-04-14T15:08:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Chris O'Kelly\" <chris@mapcreative.com.au> writes:\n\n> A brief background of my use case:\n> I am wanting to write a pre-push hook to prevent tags being pushed to\n> our production servers. The production servers in our case are --bare\n> endpoints, and when we push a tag at them, they always checkout the\n> commit that tag is attached to via some post-receive magic (WPEngine,\n> FWIW). This behavior is even present when the tag was already pushed\n> to the repo previously, if for instance a normal push is made with the\n> --tags argument.\n\nDo you mean that you want to forbid some people from pushing tags\ninto that repository while allowing others (i.e. those who manage\nproduction deployment)?  If that is the goal, it sounds like that\nthe right place to do this is at the receiving end via pre-receive,\nnot at the sending end via pre-push.\n"},{"id":"259381","messageId":"CAM-hpQcowcRaRMnYcZD8oUDep-nSw1e4Sy3CHjCojpQd=238Ug@mail.gmail.com","threadId":"39068","inReplyTo":"xmqqwq1e3fy4.fsf@gitster.dls.corp.google.com","subject":"Re: Feature Request: provide cmdline args to git hooks","fromName":"Chris O'Kelly","fromEmail":"chris@mapcreative.com.au","sentAt":"2015-04-14T22:28:39Z","receivedAt":"2015-04-14T22:28:39Z","isPatch":false,"sender":{"key":"chris@mapcreative.com.au","avatar":null},"body":"Unfortunately in this case we don't have control over the hooks at the\nreceiving end - we want to prevent tags from being pushed by all users\nto these repo's.\n\nOn Wed, Apr 15, 2015 at 1:08 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> \"Chris O'Kelly\" <chris@mapcreative.com.au> writes:\n>\n>> A brief background of my use case:\n>> I am wanting to write a pre-push hook to prevent tags being pushed to\n>> our production servers. The production servers in our case are --bare\n>> endpoints, and when we push a tag at them, they always checkout the\n>> commit that tag is attached to via some post-receive magic (WPEngine,\n>> FWIW). This behavior is even present when the tag was already pushed\n>> to the repo previously, if for instance a normal push is made with the\n>> --tags argument.\n>\n> Do you mean that you want to forbid some people from pushing tags\n> into that repository while allowing others (i.e. those who manage\n> production deployment)?  If that is the goal, it sounds like that\n> the right place to do this is at the receiving end via pre-receive,\n> not at the sending end via pre-push.\n>\n"},{"id":"259385","messageId":"xmqq4moi1dej.fsf@gitster.dls.corp.google.com","threadId":"39068","inReplyTo":"CAM-hpQcowcRaRMnYcZD8oUDep-nSw1e4Sy3CHjCojpQd=238Ug@mail.gmail.com","subject":"Re: Feature Request: provide cmdline args to git hooks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-14T23:46:44Z","receivedAt":"2015-04-14T23:46:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Chris O'Kelly\" <chris@mapcreative.com.au> writes:\n\n[administrivia: people read from top to bottom; please do not top\npost]\n\n> On Wed, Apr 15, 2015 at 1:08 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> \"Chris O'Kelly\" <chris@mapcreative.com.au> writes:\n>>\n>>> A brief background of my use case:\n>>> I am wanting to write a pre-push hook to prevent tags being pushed to\n>>> our production servers. The production servers in our case are --bare\n>>> endpoints, and when we push a tag at them, they always checkout the\n>>> commit that tag is attached to via some post-receive magic (WPEngine,\n>>> FWIW). This behavior is even present when the tag was already pushed\n>>> to the repo previously, if for instance a normal push is made with the\n>>> --tags argument.\n>>\n>> Do you mean that you want to forbid some people from pushing tags\n>> into that repository while allowing others (i.e. those who manage\n>> production deployment)?  If that is the goal, it sounds like that\n>> the right place to do this is at the receiving end via pre-receive,\n>> not at the sending end via pre-push.\n>>\n\n> Unfortunately in this case we don't have control over the hooks at the\n> receiving end - we want to prevent tags from being pushed by all users\n> to these repo's.\n\nWell, if you refuse to or are unable to use the mechanism that was\nspecifically designed to solve your problem, then there is no way we\ncan offer you a good solution.\n\nLet's set the baseline of the discussion first.\n\nWe agree that any client-side mechanism (like the hook) is *not* a\ngood way to enforce policy or ensure security. It is merely a way to\navoid mistakes---prevent the users to avoid doing something they are\nallowed to do (at the mechanical level) that is not desirable to the\nproject.  After all, the user's hook can be misconfigured (or\ndisabled) by mistake, or the user's workstation may have an older\nversion of Git that does not know about the hook, thereby bypassing\nwhategver you try to do on the client side.  The user can even say\n\"git push --no-verify\".  That is why a true policy-enforcement and\nsecurity must be done on the receiving end.\n\nSo our conversation must start from the shared understanding that\nyou are seeking a client-side \"convention\" that helps users avoid\nmistakes.  You do not have anything more than \"convention\" to force\ncertain behaviour on users.  Are we in agreement?\n\nNow, if a \"convention,\" is an acceptaible solution, you do not even\nnecessarily need a hook. You can give \"git mypush\" to the users,\ntell them to use it instead of \"git push\" and do whatever check in\n\"git mypush\" and you are done.  If your users can be told not to run\n\"git push\" with --no-verify, they can certainly taught not to use\n\"git push\" but to use \"git mypush\". Cf. 5 valid reasons to have hook\n(http://thread.gmane.org/gmane.comp.version-control.git/71069).\n\nThe reason why we added pre-push is primarily because parsing the\ncommand line is too brittle a way to guess what will be pushed.\nWhen the user says \"git push\" and your \"hook\" sees \"ah, there is no\nargument\", that does not mean the user did not try to push any\ntags.  The user may have \"remote.origin.push\" refspec to always push\nthings without typing from the command line, for example.  The\napproach to inspect the command line, whether it is done in \"hook\"\nor \"git mypush\", is unworkable, and that is why pre-push is told\nwhat \"git push\" decided to update on the remote end.\n\nHaving said all that, I am not sure if ec55559f (push: Add support\nfor pre-push hooks, 2013-01-13) implemented the pre-push hook\ncorrectly.\n\nI do not offhand know (I am on a bus with terrible connection so I\nwon't bother checking the source now) if we send \"this ref has to\npoint at that object\" even for STATUS_UPTODATE cases, to cause your\nremote to trigger the receive hook in the frist place, but if that\nis the case, then the code in run_pre_push_hook() that omits such\nrefs from the hook input looks just *wrong*.  On the other hand, I\ndo not offhand think of a valid reason for the push codepath (with\nor without the pre-push hook) to send \"this ref has to point at that\nobject\" when we know the ref is already pointing at that object, so\nI am not sure if your claim (i.e. \"when ... was already pushed\npreviously\") is true for a correctly built receiving side of Git.\n\nOn the other side, if we are telling the other end \"I know your\nrefs/tags/v1.0 is pointing at this object, but I am telling you to\npoint at it again anyway\", then we should be feeding pre-push that,\ntoo.  The code here in transport.c omits UPTODATE ones, and it may\nneed to be disabled to be consistent.\n\n\tfor (r = remote_refs; r; r = r->next) {\n\t\tif (!r->peer_ref) continue;\n\t\tif (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;\n\t\tif (r->status == REF_STATUS_REJECT_STALE) continue;\n\t\tif (r->status == REF_STATUS_UPTODATE) continue;\n\n\t\tstrbuf_reset(&buf);\n\t\tstrbuf_addf( &buf, \"%s %s %s %s\\n\",\n\t\t\t r->peer_ref->name, sha1_to_hex(r->new_sha1),\n\t\t\t r->name, sha1_to_hex(r->old_sha1));\n\n\t\tif (write_in_full(proc.in, buf.buf, buf.len) != buf.len) {\n\t\t\tret = -1;\n\t\t\tbreak;\n\t\t}\n"},{"id":"259387","messageId":"CAM-hpQf2hzjufYxmc2uGUhoqn3JVnwwcoOPOnya4MJbCnjkg3A@mail.gmail.com","threadId":"39068","inReplyTo":"xmqq4moi1dej.fsf@gitster.dls.corp.google.com","subject":"Re: Feature Request: provide cmdline args to git hooks","fromName":"Chris O'Kelly","fromEmail":"chris@mapcreative.com.au","sentAt":"2015-04-15T00:21:39Z","receivedAt":"2015-04-15T00:21:39Z","isPatch":false,"sender":{"key":"chris@mapcreative.com.au","avatar":null},"body":"> I do not offhand know (I am on a bus with terrible connection so I\n> won't bother checking the source now) if we send \"this ref has to\n> point at that object\" even for STATUS_UPTODATE cases, to cause your\n> remote to trigger the receive hook in the frist place, but if that\n> is the case, then the code in run_pre_push_hook() that omits such\n> refs from the hook input looks just *wrong*.  On the other hand, I\n> do not offhand think of a valid reason for the push codepath (with\n> or without the pre-push hook) to send \"this ref has to point at that\n> object\" when we know the ref is already pointing at that object, so\n> I am not sure if your claim (i.e. \"when ... was already pushed\n> previously\") is true for a correctly built receiving side of Git.\n\nNo I totally understand that the pre-receive is the ideal place to do\nit, and I can see that it's not feasible to rework how pre-push was\ndesigned if it was specifically made not to handle this kind of\nsituation. In a perfect world I would just remove/modify the\npost-receive hook causing the undesirable behavior, but I work within\nthe restrictions of my environment. To reiterate and clarify I'm not\nsaying the undesirable behavior is a built in part of git, just a\nfeature of our hosting platform's Git deployment mechanisms, although\nif what you mean is that the post-receive hook on the receiving end\nshouldn't be getting passed pushed tag refs that the local git\nbelieved to be already up to date on the remote (as of most recent\nfetch), then yeah... that is weird because it seems to be the behavior\nin this case.\n\nAnyway it sounds like the answer here is that it really isn't a\nfeasible task in a client side hook, and we should stick with the\ncurrent solution of \"Don't use the GUI to push to Live\" for now, which\nis fine; I should probably stop trying to script around every single\nproblem anyway (https://xkcd.com/1319/).\n"},{"id":"259433","messageId":"xmqqk2xdl0fj.fsf@gitster.dls.corp.google.com","threadId":"39068","inReplyTo":"CAM-hpQf2hzjufYxmc2uGUhoqn3JVnwwcoOPOnya4MJbCnjkg3A@mail.gmail.com","subject":"Re: Feature Request: provide cmdline args to git hooks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-04-15T18:18:56Z","receivedAt":"2015-04-15T18:18:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Chris O'Kelly\" <chris@mapcreative.com.au> writes:\n\n> To reiterate and clarify I'm not saying the undesirable behavior\n> is a built in part of git, just a feature of our hosting\n> platform's Git deployment mechanisms, although if what you mean is\n> that the post-receive hook on the receiving end shouldn't be\n> getting passed pushed tag refs that the local git believed to be\n> already up to date on the remote (as of most recent fetch), then\n> yeah... that is weird because it seems to be the behavior in this\n> case.\n\nI just checked.\n\n    $ rm -fr new && mkdir new && cd new\n    $ git init src && git init --bare dst\n    $ cd src\n    $ echo \"(date;cat) >pushlog\" >.git/hooks/pre-push\n    $ chmod +x .git/hooks/pre-push\n    $ git commit -m 'initial' --allow-empty\n    $ git tag -m 'initial' initial\n\nPush only the branch:\n\n    $ GIT_TRACE_PACKET=1 git push ../dst master 2>&1 | grep 'push>'\n    11:07:26.... packet: push> 0000... 66ba... refs/heads/master\\0report-st...\n    11:07:26.... packet: push> 0000\n    $ cat pushlog\n    Wed Apr 15 11:07:26 PDT 2015\n    refs/heads/master 66ba... refs/heads/master 0000...\n\nIn the packet trace, we can see that we told the remote to update 'master',\nand the pre-push logger also records the same.\n\nThen push with --follow-tags:\n\n    $ GIT_TRACE_PACKET=1 git push --follow-tags ../dst master 2>&1 | grep 'push>'\n    11:09:53.... packet: push> 0000... 30fa... refs/tags/initial\\0report-st...\n    11:09:53.... packet: push> 0000\n    $ cat pushlog\n    Wed Apr 15 11:09:53 PDT 2015\n    refs/tags/initial 30fa... refs/tags/initial 0000...\n\nWe can see that we told the remote to store the tag, which matches\nwhat the pre-push saw.\n\nAnd then an empty push:\n\n    $ GIT_TRACE_PACKET=1 git push --follow-tags ../dst master 2>&1 | grep 'push>'\n    11:11:23.... packet: push> 0000\n    $ cat pushlog\n    Wed Apr 15 11:11:23 PDT 2015\n\nWe tell them to do nothing, and pre-push saw nothing.\n\nFor a good measure, let's advance the branch and push it out:\n\n    $ git commit --allow-empty -m second\n    $ GIT_TRACE_PACKET=1 git push --follow-tags ../dst master 2>&1 | grep 'push>'\n    11:13:43.... packet: push> 66ba... e711... refs/heads/master\\0report-st...\n    11:13:43.... packet: push> 0000\n    $ cat pushlog\n    Wed Apr 15 11:13:43 PDT 2015\n    refs/heads/master e711... refs/heads/master 66ba...\n\nWe notice that the tag is up to date and do not tell them to do\nanything to it, and pre-push did not see the tag either.\n\nAs far as I can see so far, the behaviour of the underlying push\n(i.e. what we decide to tell the remote to update) is sensible,\nand what pre-push is told we are doing by the command is consistent\nwith what is really pushed.\n\nSo....\n\n> Anyway it sounds like the answer here is that it really isn't a\n> feasible task in a client side hook, and we should stick with the\n> current solution of \"Don't use the GUI to push to Live\" for now, which\n> is fine; I should probably stop trying to script around every single\n> problem anyway (https://xkcd.com/1319/).\n\nIt appears that your \"GUI\" is a broken implementation of Git, that\ntells the other side to update what it did not even send, and that\nis what is causing the trouble, perhaps?\n"}]}