Re: Fwd: Fwd: git-daemon access-hook race condition

2013-09-13 Thread Eugene Sajine

 For now I'm trying to do the following:

 access-hook.bash has:

 delayed-notify.bash $@ 

 delayed-notify.bash has:

 sleep 10
 ...
 curl ...

 I'm expecting access-hook to spawn new process and return without
 waiting for it to finish to let the service to do its job. But when i
 do push - it sleeps for 10 seconds anyway. Am i missing something
 obvious here?

 Any help is much appreciated!

 Thanks,
 Eugene


I found a following solution to make it happen while waiting for
somebody to be generous enough to take on the --post-service-hook
(unfortunately i'm not a C guy):

It is using 'at' command. The access-hook script has:

echo delayed-notify.bash $@ | at now

while delayed-notify.bash has:

sleep 10
curl ...

This is not perfect and in certain situations can still fail because
the delay is not long enough but this will at least resolve 90% of
issues.

I hope that might be helpful for someone.

Thanks,
Eugene
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: Fwd: git-daemon access-hook race condition

2013-09-12 Thread Junio C Hamano
Eugene Sajine eugu...@gmail.com writes:

 So are you really sure that it is a non-starter to have
 --before-service/--after-service options for access-hook?

Given the definition of --access-hook in git help daemon:

--access-hook=path::
Every time a client connects, first run an external command
specified by the path ... The external command can decide
to decline the service by exiting with a non-zero status (or
to allow it by exiting with a zero status)

There is *NO* way in anywhere --after-service makes any sense (and
by definition --before-service is redundant).

What you _could_ propose is to define a *new* hook that is run when
the spawned service has returned, with the same information that is
fed to the access hook (possibly with its exit status).

I do not offhand know if we retain the original service information
that long after the main daemon process has spawned the service
process, though.  With the current system, the only thing it needs
to know is the PID of the service processes that are to be culled by
calls to waitpid().  So you may have to extend existing bookkeeping
data structures a bit to keep those pieces of information around if
you wanted to add such a new hook.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: Fwd: git-daemon access-hook race condition

2013-09-12 Thread Eugene Sajine
Junio,

Thanks for the clarification! Your solution does look better.

For now though i think i will have to delay the notification somehow
and let the service finish first then notify the server.

Thanks again!

Eugene


On Thu, Sep 12, 2013 at 5:08 PM, Junio C Hamano gits...@pobox.com wrote:
 Eugene Sajine eugu...@gmail.com writes:

 So are you really sure that it is a non-starter to have
 --before-service/--after-service options for access-hook?

 Given the definition of --access-hook in git help daemon:

 --access-hook=path::
 Every time a client connects, first run an external command
 specified by the path ... The external command can decide
 to decline the service by exiting with a non-zero status (or
 to allow it by exiting with a zero status)

 There is *NO* way in anywhere --after-service makes any sense (and
 by definition --before-service is redundant).

 What you _could_ propose is to define a *new* hook that is run when
 the spawned service has returned, with the same information that is
 fed to the access hook (possibly with its exit status).

 I do not offhand know if we retain the original service information
 that long after the main daemon process has spawned the service
 process, though.  With the current system, the only thing it needs
 to know is the PID of the service processes that are to be culled by
 calls to waitpid().  So you may have to extend existing bookkeeping
 data structures a bit to keep those pieces of information around if
 you wanted to add such a new hook.


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: Fwd: git-daemon access-hook race condition

2013-09-12 Thread Eugene Sajine
On Thu, Sep 12, 2013 at 5:16 PM, Eugene Sajine eugu...@gmail.com wrote:
 Junio,

 Thanks for the clarification! Your solution does look better.

 For now though i think i will have to delay the notification somehow
 and let the service finish first then notify the server.

 Thanks again!

 Eugene


 On Thu, Sep 12, 2013 at 5:08 PM, Junio C Hamano gits...@pobox.com wrote:
 Eugene Sajine eugu...@gmail.com writes:

 So are you really sure that it is a non-starter to have
 --before-service/--after-service options for access-hook?

 Given the definition of --access-hook in git help daemon:

 --access-hook=path::
 Every time a client connects, first run an external command
 specified by the path ... The external command can decide
 to decline the service by exiting with a non-zero status (or
 to allow it by exiting with a zero status)

 There is *NO* way in anywhere --after-service makes any sense (and
 by definition --before-service is redundant).

 What you _could_ propose is to define a *new* hook that is run when
 the spawned service has returned, with the same information that is
 fed to the access hook (possibly with its exit status).

 I do not offhand know if we retain the original service information
 that long after the main daemon process has spawned the service
 process, though.  With the current system, the only thing it needs
 to know is the PID of the service processes that are to be culled by
 calls to waitpid().  So you may have to extend existing bookkeeping
 data structures a bit to keep those pieces of information around if
 you wanted to add such a new hook.



For now I'm trying to do the following:

access-hook.bash has:

delayed-notify.bash $@ 

delayed-notify.bash has:

sleep 10
...
curl ...

I'm expecting access-hook to spawn new process and return without
waiting for it to finish to let the service to do its job. But when i
do push - it sleeps for 10 seconds anyway. Am i missing something
obvious here?

Any help is much appreciated!

Thanks,
Eugene
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: Fwd: git-daemon access-hook race condition

2013-09-12 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Eugene Sajine eugu...@gmail.com writes:

 So are you really sure that it is a non-starter to have
 --before-service/--after-service options for access-hook?

 Given the definition of --access-hook in git help daemon:

 --access-hook=path::
 Every time a client connects, first run an external command
 specified by the path ... The external command can decide
 to decline the service by exiting with a non-zero status (or
 to allow it by exiting with a zero status)

 There is *NO* way in anywhere --after-service makes any sense (and
 by definition --before-service is redundant).

 What you _could_ propose is to define a *new* hook that is run when
 the spawned service has returned, with the same information that is
 fed to the access hook (possibly with its exit status).

Scratch that exit status part, as I do not think it is useful.

To a receive-pack and a send-pack that is talking to it, if a push
results in a failure, it is a failure.  Likewise for upload-pack and
fetch-pack for a transfer in the reverse direction.

And the way that failure is communicated from the receive-pack to
the end-user via the send-pack is for the receive-pack to send a
protocol message that tells the send-pack about the failure, and the
send-pack showing the error message and signalling the failure with
its exit status.  Likewise for upload-pack and fetch-pack (hence
fetch, which is conceptually a thin wrapper around it).

Between the deamon and the receive-pack (or the fetch-pack) process,
however, such a failed push (or fetch) is still a success.  I
correctly diagnosed and successfully sent a rejection notice to the
other end is signalled by receive-pack to the daemon by exiting
with success (i.e. 0) exit status.

So even if we feed the exit status of the service process to the
hook script specified by the --post-service-hook, it does not tell
the script if the service succeeded in that sense.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: Fwd: git-daemon access-hook race condition

2013-09-12 Thread Eugene Sajine
On Thu, Sep 12, 2013 at 6:20 PM, Junio C Hamano gits...@pobox.com wrote:
 Junio C Hamano gits...@pobox.com writes:

 Eugene Sajine eugu...@gmail.com writes:

 So are you really sure that it is a non-starter to have
 --before-service/--after-service options for access-hook?

 Given the definition of --access-hook in git help daemon:

 --access-hook=path::
 Every time a client connects, first run an external command
 specified by the path ... The external command can decide
 to decline the service by exiting with a non-zero status (or
 to allow it by exiting with a zero status)

 There is *NO* way in anywhere --after-service makes any sense (and
 by definition --before-service is redundant).

 What you _could_ propose is to define a *new* hook that is run when
 the spawned service has returned, with the same information that is
 fed to the access hook (possibly with its exit status).

 Scratch that exit status part, as I do not think it is useful.

 To a receive-pack and a send-pack that is talking to it, if a push
 results in a failure, it is a failure.  Likewise for upload-pack and
 fetch-pack for a transfer in the reverse direction.

 And the way that failure is communicated from the receive-pack to
 the end-user via the send-pack is for the receive-pack to send a
 protocol message that tells the send-pack about the failure, and the
 send-pack showing the error message and signalling the failure with
 its exit status.  Likewise for upload-pack and fetch-pack (hence
 fetch, which is conceptually a thin wrapper around it).

 Between the deamon and the receive-pack (or the fetch-pack) process,
 however, such a failed push (or fetch) is still a success.  I
 correctly diagnosed and successfully sent a rejection notice to the
 other end is signalled by receive-pack to the daemon by exiting
 with success (i.e. 0) exit status.

 So even if we feed the exit status of the service process to the
 hook script specified by the --post-service-hook, it does not tell
 the script if the service succeeded in that sense.


I see what you're saying.
In my particular use case I can work around that service status
because even if it failed it will just trigger Jenkins to poll and in
case of failure to transfer data there will be no new changes for
Jenkins to work with. If we would want the --post-service-hook to know
that data transfer succeeded or failed, then may be there should be
some difference between service status and service process status?
In this case the existing logic works with service process status
while the --post-service-hook is fed with the service status (or
name it data transfer status)

Do i make any sense?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: Fwd: git-daemon access-hook race condition

2013-09-12 Thread Junio C Hamano
Eugene Sajine eugu...@gmail.com writes:

 So even if we feed the exit status of the service process to the
 hook script specified by the --post-service-hook, it does not tell
 the script if the service succeeded in that sense.

 I see what you're saying.
 In my particular use case I can work around that service status
 because even if it failed it will just trigger Jenkins to poll and in
 case of failure to transfer data there will be no new changes for
 Jenkins to work with. If we would want the --post-service-hook to know
 that data transfer succeeded or failed, then may be there should be
 some difference between service status and service process status?
 In this case the existing logic works with service process status
 while the --post-service-hook is fed with the service status (or
 name it data transfer status)

 Do i make any sense?

Almost; you missed that there is no channel to pass data transfer
status from the service back to the daemon.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html