{"thread":{"id":"34919","subject":"Fwd: git-daemon access-hook race condition","startedAt":"2013-09-12T18:51:41Z","lastAt":"2013-09-13T17:17:52Z","messageCount":10,"participants":["Eugene Sajine","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"227553","messageId":"CAPZPVFbJqbRGQZ+m3-EfahcYegPvVcS-jNTsCXxBqWUsLqyHkg@mail.gmail.com","threadId":"34919","inReplyTo":"CAPZPVFa=gqJ26iA6eQ1B6pcbTcQmmnXHYz6OQLtMORnAa5ec2w@mail.gmail.com","subject":"Fwd: git-daemon access-hook race condition","fromName":"Eugene Sajine","fromEmail":"euguess@gmail.com","sentAt":"2013-09-12T18:51:41Z","receivedAt":"2013-09-12T18:51:41Z","isPatch":false,"sender":{"key":"euguess@gmail.com","avatar":null},"body":"Hi,\n\n\nWe are serving repos in closed netwrok via git protocol. We are using\ngit-daemon access hook (thank you very much for such a great feature)\nin order to create push notifications for Jenkins.\nI.e. upon the push the access-hook is called and then the curl command\nis created and executed. As we have several instances of Jenkins, that\nwe need to notify (three), the execution of the access-hook can take\nsome time.\n\nSometimes we have a situation when the whole chain works fine but\nJenkins git plugin doesn't recognize the changes. I think it happens\nbecause we hit a kind of race condition:\n\n1. Incoming push triggers access-hook\n2. notify jenkins 1\n3. notify jenkins 2\n4. jenkins 1 polls repo but sees no changes\n5. notify Jenkins 3\n6. the push data transfer finishes - consequent pushes will find\nchanges w/o any problem\n\nThe question is:\n\nIs there a way to avoid that?\nIs it possible to have access-hook to be executed after receive?\nIs it possible to introduce a parameter that would specify if it needs\nto be executed before receive or after?\n\nThanks,\nEugene\n"},{"id":"227556","messageId":"xmqq4n9pq2av.fsf@gitster.dls.corp.google.com","threadId":"34919","inReplyTo":"CAPZPVFbJqbRGQZ+m3-EfahcYegPvVcS-jNTsCXxBqWUsLqyHkg@mail.gmail.com","subject":"Re: Fwd: git-daemon access-hook race condition","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-12T19:15:36Z","receivedAt":"2013-09-12T19:15:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eugene Sajine writes:\n\n> Is it possible to have access-hook to be executed after receive?\n\nThe whole point of access-hook is to allow it to decide whether the\naccess is allowed or not, so that is a non-starter.\n\nA notification _after_ successful push update is usually done via\nthe post-receive hook in the receiving repository, I think.\n"},{"id":"227575","messageId":"CAPZPVFZpYJnQY4BpjaPxU8NnBmPZ9Fp6UpovoQEvkKzRnLa=KA@mail.gmail.com","threadId":"34919","inReplyTo":"CAPZPVFZLPV=JVR+SSqfX-=aLyFWZBkof+yCkivcLoKNnv6f__Q@mail.gmail.com","subject":"Fwd: Fwd: git-daemon access-hook race condition","fromName":"Eugene Sajine","fromEmail":"euguess@gmail.com","sentAt":"2013-09-12T20:30:06Z","receivedAt":"2013-09-12T20:30:06Z","isPatch":false,"sender":{"key":"euguess@gmail.com","avatar":null},"body":"On Thu, Sep 12, 2013 at 3:15 PM, Junio C Hamano wrote:\n> Eugene Sajine writes:\n>\n>> Is it possible to have access-hook to be executed after receive?\n>\n> The whole point of access-hook is to allow it to decide whether the\n> access is allowed or not, so that is a non-starter.\n>\n> A notification _after_ successful push update is usually done via\n> the post-receive hook in the receiving repository, I think.\n\n\nJunio,\n\nThanks for the reply!\n\nThis is interesting: i always thought about the access-hook as\nsomething to be executed when the repo is accessed, not just\nverification if access is allowed - your definition is much more\nlimiting.\n\nwe have about 1400 bare repos - so i would like to avoid the\nconfiguration of each one of them. I could probably find a way to\nautomate it, but already having access-hook in current implementation\nmakes me reluctant to go this way, because it is so much easier to use\ncentralized manager.\n\nSo are you really sure that it is a non-starter to have\n--before-service/--after-service options for access-hook?\n\nThanks,\nEugene\n"},{"id":"227578","messageId":"xmqqioy5oiif.fsf@gitster.dls.corp.google.com","threadId":"34919","inReplyTo":"CAPZPVFZpYJnQY4BpjaPxU8NnBmPZ9Fp6UpovoQEvkKzRnLa=KA@mail.gmail.com","subject":"Re: Fwd: Fwd: git-daemon access-hook race condition","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-12T21:08:24Z","receivedAt":"2013-09-12T21:08:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eugene Sajine writes:\n\n> So are you really sure that it is a non-starter to have\n> --before-service/--after-service options for access-hook?\n\nGiven the definition of \"--access-hook\" in \"git help daemon\":\n\n --access-hook=::\n Every time a client connects, first run an external command\n specified by the ... The external command can decide\n to decline the service by exiting with a non-zero status (or\n to allow it by exiting with a zero status)....\n\nThere is *NO* way in anywhere --after-service makes any sense (and\nby definition --before-service is redundant).\n\nWhat you _could_ propose is to define a *new* hook that is run when\nthe spawned service has returned, with the same information that is\nfed to the access hook (possibly with its exit status).\n\nI do not offhand know if we retain the original service information\nthat long after the main daemon process has spawned the service\nprocess, though. With the current system, the only thing it needs\nto know is the PID of the service processes that are to be culled by\ncalls to waitpid(). So you may have to extend existing bookkeeping\ndata structures a bit to keep those pieces of information around if\nyou wanted to add such a new hook.\n"},{"id":"227580","messageId":"CAPZPVFYtVsPrFEMcx9UBQzJffi9tRduDqLfUF1vGR=WSKL95aQ@mail.gmail.com","threadId":"34919","inReplyTo":"xmqqioy5oiif.fsf@gitster.dls.corp.google.com","subject":"Re: Fwd: Fwd: git-daemon access-hook race condition","fromName":"Eugene Sajine","fromEmail":"euguess@gmail.com","sentAt":"2013-09-12T21:16:26Z","receivedAt":"2013-09-12T21:16:26Z","isPatch":false,"sender":{"key":"euguess@gmail.com","avatar":null},"body":"Junio,\n\nThanks for the clarification! Your solution does look better.\n\nFor now though i think i will have to delay the notification somehow\nand let the service finish first then notify the server.\n\nThanks again!\n\nEugene\n\n\nOn Thu, Sep 12, 2013 at 5:08 PM, Junio C Hamano wrote:\n> Eugene Sajine writes:\n>\n>> So are you really sure that it is a non-starter to have\n>> --before-service/--after-service options for access-hook?\n>\n> Given the definition of \"--access-hook\" in \"git help daemon\":\n>\n> --access-hook=::\n> Every time a client connects, first run an external command\n> specified by the ... The external command can decide\n> to decline the service by exiting with a non-zero status (or\n> to allow it by exiting with a zero status)....\n>\n> There is *NO* way in anywhere --after-service makes any sense (and\n> by definition --before-service is redundant).\n>\n> What you _could_ propose is to define a *new* hook that is run when\n> the spawned service has returned, with the same information that is\n> fed to the access hook (possibly with its exit status).\n>\n> I do not offhand know if we retain the original service information\n> that long after the main daemon process has spawned the service\n> process, though. With the current system, the only thing it needs\n> to know is the PID of the service processes that are to be culled by\n> calls to waitpid(). So you may have to extend existing bookkeeping\n> data structures a bit to keep those pieces of information around if\n> you wanted to add such a new hook.\n>\n>\n"},{"id":"227583","messageId":"CAPZPVFZHyzPZnyg5gjhL0tP6Kgd=XFLc6o8ZKsmPXLUSdcc5mA@mail.gmail.com","threadId":"34919","inReplyTo":"CAPZPVFYtVsPrFEMcx9UBQzJffi9tRduDqLfUF1vGR=WSKL95aQ@mail.gmail.com","subject":"Re: Fwd: Fwd: git-daemon access-hook race condition","fromName":"Eugene Sajine","fromEmail":"euguess@gmail.com","sentAt":"2013-09-12T22:01:03Z","receivedAt":"2013-09-12T22:01:03Z","isPatch":false,"sender":{"key":"euguess@gmail.com","avatar":null},"body":"On Thu, Sep 12, 2013 at 5:16 PM, Eugene Sajine wrote:\n> Junio,\n>\n> Thanks for the clarification! Your solution does look better.\n>\n> For now though i think i will have to delay the notification somehow\n> and let the service finish first then notify the server.\n>\n> Thanks again!\n>\n> Eugene\n>\n>\n> On Thu, Sep 12, 2013 at 5:08 PM, Junio C Hamano wrote:\n>> Eugene Sajine writes:\n>>\n>>> So are you really sure that it is a non-starter to have\n>>> --before-service/--after-service options for access-hook?\n>>\n>> Given the definition of \"--access-hook\" in \"git help daemon\":\n>>\n>> --access-hook=::\n>> Every time a client connects, first run an external command\n>> specified by the ... The external command can decide\n>> to decline the service by exiting with a non-zero status (or\n>> to allow it by exiting with a zero status)....\n>>\n>> There is *NO* way in anywhere --after-service makes any sense (and\n>> by definition --before-service is redundant).\n>>\n>> What you _could_ propose is to define a *new* hook that is run when\n>> the spawned service has returned, with the same information that is\n>> fed to the access hook (possibly with its exit status).\n>>\n>> I do not offhand know if we retain the original service information\n>> that long after the main daemon process has spawned the service\n>> process, though. With the current system, the only thing it needs\n>> to know is the PID of the service processes that are to be culled by\n>> calls to waitpid(). So you may have to extend existing bookkeeping\n>> data structures a bit to keep those pieces of information around if\n>> you wanted to add such a new hook.\n>>\n>>\n\nFor now I'm trying to do the following:\n\naccess-hook.bash has:\n\ndelayed-notify.bash $@ &\n\ndelayed-notify.bash has:\n\nsleep 10\n...\ncurl ...\n\nI'm expecting access-hook to spawn new process and return without\nwaiting for it to finish to let the service to do its job. But when i\ndo push - it sleeps for 10 seconds anyway. Am i missing something\nobvious here?\n\nAny help is much appreciated!\n\nThanks,\nEugene\n"},{"id":"227587","messageId":"xmqqa9jhof6k.fsf@gitster.dls.corp.google.com","threadId":"34919","inReplyTo":"xmqqioy5oiif.fsf@gitster.dls.corp.google.com","subject":"Re: Fwd: Fwd: git-daemon access-hook race condition","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-12T22:20:19Z","receivedAt":"2013-09-12T22:20:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Eugene Sajine writes:\n>\n>> So are you really sure that it is a non-starter to have\n>> --before-service/--after-service options for access-hook?\n>\n> Given the definition of \"--access-hook\" in \"git help daemon\":\n>\n> --access-hook=::\n> Every time a client connects, first run an external command\n> specified by the ... The external command can decide\n> to decline the service by exiting with a non-zero status (or\n> to allow it by exiting with a zero status)....\n>\n> There is *NO* way in anywhere --after-service makes any sense (and\n> by definition --before-service is redundant).\n>\n> What you _could_ propose is to define a *new* hook that is run when\n> the spawned service has returned, with the same information that is\n> fed to the access hook (possibly with its exit status).\n\nScratch that \"exit status\" part, as I do not think it is useful.\n\nTo a receive-pack and a send-pack that is talking to it, if a push\nresults in a failure, it is a failure. Likewise for upload-pack and\nfetch-pack for a transfer in the reverse direction.\n\nAnd the way that failure is communicated from the receive-pack to\nthe end-user via the send-pack is for the receive-pack to send a\nprotocol message that tells the send-pack about the failure, and the\nsend-pack showing the error message and signalling the failure with\nits exit status. Likewise for upload-pack and fetch-pack (hence\n\"fetch\", which is conceptually a thin wrapper around it).\n\nBetween the deamon and the receive-pack (or the fetch-pack) process,\nhowever, such a failed push (or fetch) is still a success. \"I\ncorrectly diagnosed and successfully sent a rejection notice to the\nother end\" is signalled by receive-pack to the daemon by exiting\nwith success (i.e. 0) exit status.\n\nSo even if we feed the exit status of the service process to the\nhook script specified by the --post-service-hook, it does not tell\nthe script if the service \"succeeded\" in that sense.\n"},{"id":"227592","messageId":"CAPZPVFYY6Q=zZ62mY6uGi6O7KaOO8NhUEH0YUgcQE3NBvHQEpg@mail.gmail.com","threadId":"34919","inReplyTo":"xmqqa9jhof6k.fsf@gitster.dls.corp.google.com","subject":"Re: Fwd: Fwd: git-daemon access-hook race condition","fromName":"Eugene Sajine","fromEmail":"euguess@gmail.com","sentAt":"2013-09-12T23:17:08Z","receivedAt":"2013-09-12T23:17:08Z","isPatch":false,"sender":{"key":"euguess@gmail.com","avatar":null},"body":"On Thu, Sep 12, 2013 at 6:20 PM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Eugene Sajine writes:\n>>\n>>> So are you really sure that it is a non-starter to have\n>>> --before-service/--after-service options for access-hook?\n>>\n>> Given the definition of \"--access-hook\" in \"git help daemon\":\n>>\n>> --access-hook=::\n>> Every time a client connects, first run an external command\n>> specified by the ... The external command can decide\n>> to decline the service by exiting with a non-zero status (or\n>> to allow it by exiting with a zero status)....\n>>\n>> There is *NO* way in anywhere --after-service makes any sense (and\n>> by definition --before-service is redundant).\n>>\n>> What you _could_ propose is to define a *new* hook that is run when\n>> the spawned service has returned, with the same information that is\n>> fed to the access hook (possibly with its exit status).\n>\n> Scratch that \"exit status\" part, as I do not think it is useful.\n>\n> To a receive-pack and a send-pack that is talking to it, if a push\n> results in a failure, it is a failure. Likewise for upload-pack and\n> fetch-pack for a transfer in the reverse direction.\n>\n> And the way that failure is communicated from the receive-pack to\n> the end-user via the send-pack is for the receive-pack to send a\n> protocol message that tells the send-pack about the failure, and the\n> send-pack showing the error message and signalling the failure with\n> its exit status. Likewise for upload-pack and fetch-pack (hence\n> \"fetch\", which is conceptually a thin wrapper around it).\n>\n> Between the deamon and the receive-pack (or the fetch-pack) process,\n> however, such a failed push (or fetch) is still a success. \"I\n> correctly diagnosed and successfully sent a rejection notice to the\n> other end\" is signalled by receive-pack to the daemon by exiting\n> with success (i.e. 0) exit status.\n>\n> So even if we feed the exit status of the service process to the\n> hook script specified by the --post-service-hook, it does not tell\n> the script if the service \"succeeded\" in that sense.\n\n\nI see what you're saying.\nIn my particular use case I can work around that service status\nbecause even if it failed it will just trigger Jenkins to poll and in\ncase of failure to transfer data there will be no new changes for\nJenkins to work with. If we would want the --post-service-hook to know\nthat data transfer succeeded or failed, then may be there should be\nsome difference between \"service status\" and \"service process status\"?\nIn this case the existing logic works with \"service process status\"\nwhile the --post-service-hook is fed with the \"service status\" (or\nname it \"data transfer status\")\n\nDo i make any sense?\n"},{"id":"227595","messageId":"xmqqppsdmxig.fsf@gitster.dls.corp.google.com","threadId":"34919","inReplyTo":"CAPZPVFYY6Q=zZ62mY6uGi6O7KaOO8NhUEH0YUgcQE3NBvHQEpg@mail.gmail.com","subject":"Re: Fwd: Fwd: git-daemon access-hook race condition","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-12T23:27:19Z","receivedAt":"2013-09-12T23:27:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eugene Sajine writes:\n\n>> So even if we feed the exit status of the service process to the\n>> hook script specified by the --post-service-hook, it does not tell\n>> the script if the service \"succeeded\" in that sense.\n>\n> I see what you're saying.\n> In my particular use case I can work around that service status\n> because even if it failed it will just trigger Jenkins to poll and in\n> case of failure to transfer data there will be no new changes for\n> Jenkins to work with. If we would want the --post-service-hook to know\n> that data transfer succeeded or failed, then may be there should be\n> some difference between \"service status\" and \"service process status\"?\n> In this case the existing logic works with \"service process status\"\n> while the --post-service-hook is fed with the \"service status\" (or\n> name it \"data transfer status\")\n>\n> Do i make any sense?\n\nAlmost; you missed that there is no channel to pass \"data transfer\nstatus\" from the service back to the daemon.\n"},{"id":"227638","messageId":"CAPZPVFY_AD+dg2C3TSDQW3fKXW9qiSMX70MuMtMsMxmnKBoVWA@mail.gmail.com","threadId":"34919","inReplyTo":"CAPZPVFZHyzPZnyg5gjhL0tP6Kgd=XFLc6o8ZKsmPXLUSdcc5mA@mail.gmail.com","subject":"Re: Fwd: Fwd: git-daemon access-hook race condition","fromName":"Eugene Sajine","fromEmail":"euguess@gmail.com","sentAt":"2013-09-13T17:17:52Z","receivedAt":"2013-09-13T17:17:52Z","isPatch":false,"sender":{"key":"euguess@gmail.com","avatar":null},"body":">\n> For now I'm trying to do the following:\n>\n> access-hook.bash has:\n>\n> delayed-notify.bash $@ &\n>\n> delayed-notify.bash has:\n>\n> sleep 10\n> ...\n> curl ...\n>\n> I'm expecting access-hook to spawn new process and return without\n> waiting for it to finish to let the service to do its job. But when i\n> do push - it sleeps for 10 seconds anyway. Am i missing something\n> obvious here?\n>\n> Any help is much appreciated!\n>\n> Thanks,\n> Eugene\n\n\nI found a following solution to make it happen while waiting for\nsomebody to be generous enough to take on the --post-service-hook\n(unfortunately i'm not a C guy):\n\nIt is using 'at' command. The access-hook script has:\n\necho \"delayed-notify.bash $@\" | at now\n\nwhile delayed-notify.bash has:\n\nsleep 10\ncurl ...\n\nThis is not perfect and in certain situations can still fail because\nthe delay is not long enough but this will at least resolve 90% of\nissues.\n\nI hope that might be helpful for someone.\n\nThanks,\nEugene\n"}]}