You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Hudson <gh...@MIT.EDU> on 2004/07/04 15:30:25 UTC
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
This commit seems a little rough, and it seems like an odd enough design
that perhaps it should have been hashed out on the dev list. (Perhaps
it was, and I missed it.)
On Sun, 2004-07-04 at 01:06, breser@tigris.org wrote:
> +static apr_status_t wait_for_input_or_timeout(apr_file_t *f,
> + apr_interval_time_t timeout,
> + apr_pool_t *pool)
> +{
> + srv = apr_poll(&pollset, 1, &n, 10);
You appear to be ignoring the timeout parameter.
> + status = wait_for_input_or_timeout(fp,10,pool);
Spaces after commas?
> + if (status != APR_SUCCESS && status != APR_ENOTIMPL)
> + continue;
Shouldn't you be testing specifically for a timeout result? What's so
special about APR_ENOTIMPL?
The timeout should really be unnecessary. Unless I'm missing something,
apr_poll() should terminate with EINTR if a signal occurs. (On Unix,
anyway. I don't know how this all works under Windows.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Klaus Rennecke <kr...@tigris.org>.
kfogel@collab.net wrote:
> [...]
> Why is Windows different from other platforms?
Now there's a funny question ;-p
Seriously, I believe there is no real provision for OOB signals in
Windows keyboard input handling. So, down in the depths of the OS
libraries, someone will have to poll for keystrokes that are supposed to
break things out of the current contemplation.
For a demonstration, create a java process on XP that populates half a
gig heap of Lots Of Small Things, then System.gc() in a tight loop. Try
to get control of your computer again - good luck!
/Klaus
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Ben Reser <be...@reser.org>.
On Fri, Jul 09, 2004 at 01:25:19PM +0200, Branko ??ibej wrote:
> Not my fault, ask Microsoft. :-)
>
> There are no signals on Windows. They're simulated in the C runtime
> library, and if that library happens to miss (or can't catch) a ^C while
> waiting for input, then that's just tough luck. It's even possible that
> the apr_poll code doesn't work on stdin -- wouldn't be surprised at all
> if it didn't.
Should we then be using the poll with timeout code instead that I
original used? This would give Windows a chance to pickup a Ctrl+C but
it would of course still be unreliable...
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Branko Čibej <br...@xbc.nu>.
kfogel@collab.net wrote:
>Branko Čibej <br...@xbc.nu> writes:
>
>
>>It's just that I'm not quite sure if the cancellation code is supposed
>>to leave the WC in a consistent state, or do we expect the user to
>>have to "svn cleanup" afterwards? Because that's what you have to do
>>if you cancel in a prompt on Windows.
>>
>>
>
>The cancellation code is supposed to make it less likely that the WC
>will be left in an inconsistent state. I think we established at the
>time that it wasn't really possible to *guarantee* we'd always leave
>it in a consistent state, that's why we still have the cleanup
>command.
>
>Why is Windows different from other platforms?
>
>
Not my fault, ask Microsoft. :-)
There are no signals on Windows. They're simulated in the C runtime
library, and if that library happens to miss (or can't catch) a ^C while
waiting for input, then that's just tough luck. It's even possible that
the apr_poll code doesn't work on stdin -- wouldn't be surprised at all
if it didn't.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by kf...@collab.net.
Branko Čibej <br...@xbc.nu> writes:
> It's just that I'm not quite sure if the cancellation code is supposed
> to leave the WC in a consistent state, or do we expect the user to
> have to "svn cleanup" afterwards? Because that's what you have to do
> if you cancel in a prompt on Windows.
The cancellation code is supposed to make it less likely that the WC
will be left in an inconsistent state. I think we established at the
time that it wasn't really possible to *guarantee* we'd always leave
it in a consistent state, that's why we still have the cleanup
command.
Why is Windows different from other platforms?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Branko Čibej <br...@xbc.nu>.
Ben Reser wrote:
>On Mon, Jul 05, 2004 at 01:03:28AM +0200, Branko ??ibej wrote:
>
>
>>Grrrr. I meant "password", of course.
>>
>>
>
>Yeah, I should have been more specific. Some people on some platforms
>(Linux) may not be able to Ctrl+C out of the password prompt. It's
>platform specific depending upon their implementation of getpass() or
>getpassphrase() if they have one, or if they don't then it depends upon
>how APR does it on its own. I didn't pay any attention to what they do.
>Maybe I should look.
>
>
It's just that I'm not quite sure if the cancellation code is supposed
to leave the WC in a consistent state, or do we expect the user to have
to "svn cleanup" afterwards? Because that's what you have to do if you
cancel in a prompt on Windows.
>P.S. Not sure if you meant to send this only to me. If you meant to
>send it to the list. Resend and I'll resend this reply.
>
>
Oops...
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Branko Čibej <br...@xbc.nu>.
Ben Reser wrote:
>On Sun, Jul 04, 2004 at 06:00:09PM +0200, Branko ??ibej wrote:
>
>
>>It probably doesn't. The apr_poll function is notoriously tricky.
>>
>>
>
>I was afraid of that. Which is why I put that ENOTIMPL bit in, even if
>the logic is just wrong. Can you try getting a prompt on r10136 on
>Windows and see if it still works. If the poll stuff works right then
>you should be able to Ctrl+C out of any prompt except a password prompt.
>
>
I can ^C out of a username prompt, but the cleanups don't run (e.g., the
WC remains locked). Oh, on Windows, I can cancel a username prompt, too.
I don't know if any signal handlers actually run.
Anyway, I don't think we open the FS until after all prompting is done,
so a hard kill during a prompt won't damage the repos anyway, will it?
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Ben Reser <be...@reser.org>.
On Sun, Jul 04, 2004 at 06:00:09PM +0200, Branko ??ibej wrote:
> It probably doesn't. The apr_poll function is notoriously tricky.
I was afraid of that. Which is why I put that ENOTIMPL bit in, even if
the logic is just wrong. Can you try getting a prompt on r10136 on
Windows and see if it still works. If the poll stuff works right then
you should be able to Ctrl+C out of any prompt except a password prompt.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:
>This commit seems a little rough, and it seems like an odd enough design
>that perhaps it should have been hashed out on the dev list. (Perhaps
>it was, and I missed it.)
>
>On Sun, 2004-07-04 at 01:06, breser@tigris.org wrote:
>
>
>>+static apr_status_t wait_for_input_or_timeout(apr_file_t *f,
>>+ apr_interval_time_t timeout,
>>+ apr_pool_t *pool)
>>+{
>>
>>
>
>
>
>>+ srv = apr_poll(&pollset, 1, &n, 10);
>>
>>
>
>You appear to be ignoring the timeout parameter.
>
>
>
>>+ status = wait_for_input_or_timeout(fp,10,pool);
>>
>>
>
>Spaces after commas?
>
>
>
>>+ if (status != APR_SUCCESS && status != APR_ENOTIMPL)
>>+ continue;
>>
>>
>
>Shouldn't you be testing specifically for a timeout result? What's so
>special about APR_ENOTIMPL?
>
>The timeout should really be unnecessary. Unless I'm missing something,
>apr_poll() should terminate with EINTR if a signal occurs. (On Unix,
>anyway. I don't know how this all works under Windows.)
>
>
It probably doesn't. The apr_poll function is notoriously tricky.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Ben Reser <be...@reser.org>.
On Sun, Jul 04, 2004 at 01:51:43PM -0400, Greg Hudson wrote:
> It occurred to me after writing that that a cancellation doesn't
> necessarily come from a signal. But since this is a text prompt
> function, I think it's fair to only worry about cancellations from
> signals.
Yeah but the only time we set cancelled = TRUE right now is when we got
a signal. So yeah it's safe. I can't imagine any other reason for us
to be canceling.
> > Does that logic look right:
> > if (ctx)
> > SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
> > status = wait_for_input (fp, pool);
> > if (APR_STATUS_IS_EINTR (status))
> > {
> > if (ctx)
> > SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
> > continue;
>
> Just continue. You'll be calling the cancel function again at the top
> of the loop.
err yeah.
> > }
> > else if (status && status != APR_ENOTIMPL)
> > return svn_error_wrap_apr (status, _("Can't read stdin"));
>
> Otherwise looks fine to me.
>
> It would be better if we could just read the input and get APR to tell
> us about EINTRs, so that you could hit ^C at a prompt even after you've
> typed someting. But I don't see any way of doing that. (Oddly, it
> looks like APR might do what I want for buffered files only, but that
> doesn't seem like something to rely on, and I would assume stdin is
> unbuffered anyway.)
Ohh I whole heartily agreeded. I looked for a way to just get APR to do
this without me having to do the hoop jumping. But couldn't find one.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-07-04 at 13:25, Ben Reser wrote:
>> I did not miss the point of the exercise. If a handled signal is
>> invoked, apr_poll() will (on Unix) terminate with EINTR.
> Ohhh. Didn't realize that.
It occurred to me after writing that that a cancellation doesn't
necessarily come from a signal. But since this is a text prompt
function, I think it's fair to only worry about cancellations from
signals.
> Does that logic look right:
> if (ctx)
> SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
> status = wait_for_input (fp, pool);
> if (APR_STATUS_IS_EINTR (status))
> {
> if (ctx)
> SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
> continue;
Just continue. You'll be calling the cancel function again at the top
of the loop.
> }
> else if (status && status != APR_ENOTIMPL)
> return svn_error_wrap_apr (status, _("Can't read stdin"));
Otherwise looks fine to me.
It would be better if we could just read the input and get APR to tell
us about EINTRs, so that you could hit ^C at a prompt even after you've
typed someting. But I don't see any way of doing that. (Oddly, it
looks like APR might do what I want for buffered files only, but that
doesn't seem like something to rely on, and I would assume stdin is
unbuffered anyway.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Ben Reser <be...@reser.org>.
On Sun, Jul 04, 2004 at 01:06:53PM -0400, Greg Hudson wrote:
> I did not miss the point of the exercise. If a handled signal is
> invoked, apr_poll() will (on Unix) terminate with EINTR.
Ohhh. Didn't realize that.
> (I don't even see a separate Windows implementation of apr_poll, so
> I'm still confused as to what happens there.)
Yup, I couldn't find it either, but there's no constant to specify that
it's there. So according to APR rules it's supposed to work or return
ENOTIMPL. I'm hoping one of those is true. But until someone with
access to Windows tries, I can't say for sure...
> Your retry loop will of course catch the
> EINTR and retry, but that's not really necessary.
*nod*
> Here's a test program if you want to verify. Notice that even though
> SIGINT is handled, you can still interrupt the program with ^C.
>
> #include <apr_file_io.h>
> #include <apr_poll.h>
> #include <apr_signal.h>
>
> static apr_status_t wait_for_input(apr_file_t *f, apr_pool_t *pool)
> {
> apr_pollfd_t pollset;
> int srv, n;
>
> pollset.desc_type = APR_POLL_FILE;
> pollset.desc.f = f;
> pollset.p = pool;
> pollset.reqevents = APR_POLLIN;
>
> srv = apr_poll(&pollset, 1, &n, -1);
>
> if (n == 1 && pollset.rtnevents & APR_POLLIN)
> return APR_SUCCESS;
>
> return srv;
> }
>
> static void handler(int signum)
> {
> }
>
> int main()
> {
> apr_pool_t *pool;
> apr_file_t *file;
>
> apr_initialize();
> apr_pool_create(&pool, NULL);
> apr_file_open_stdin(&file, pool);
> apr_signal(SIGINT, handler);
> wait_for_input(file, pool);
> return 0;
> }
Okay this was very helpful...
Does that logic look right:
if (ctx)
SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
status = wait_for_input (fp, pool);
if (APR_STATUS_IS_EINTR (status))
{
if (ctx)
SVN_ERR (ctx->cancel_func (ctx->cancel_baton));
continue;
}
else if (status && status != APR_ENOTIMPL)
return svn_error_wrap_apr (status, _("Can't read stdin"));
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-07-04 at 12:47, Ben Reser wrote:
> > The timeout should really be unnecessary. Unless I'm missing something,
> > apr_poll() should terminate with EINTR if a signal occurs. (On Unix,
> > anyway. I don't know how this all works under Windows.)
>
> Umm the timeout is the whole point. I'm trying to get an opportunity to
> call the cancel function. I think you missed the entire point of this
> exercise. If all I wanted to do was wait until someone typed something
> I wouldn't need any of this at all. But I want to wait for people to
> type something *AND* periodically poll for a cancel.
I did not miss the point of the exercise. If a handled signal is
invoked, apr_poll() will (on Unix) terminate with EINTR. (I don't even
see a separate Windows implementation of apr_poll, so I'm still confused
as to what happens there.) Your retry loop will of course catch the
EINTR and retry, but that's not really necessary.
Here's a test program if you want to verify. Notice that even though
SIGINT is handled, you can still interrupt the program with ^C.
#include <apr_file_io.h>
#include <apr_poll.h>
#include <apr_signal.h>
static apr_status_t wait_for_input(apr_file_t *f, apr_pool_t *pool)
{
apr_pollfd_t pollset;
int srv, n;
pollset.desc_type = APR_POLL_FILE;
pollset.desc.f = f;
pollset.p = pool;
pollset.reqevents = APR_POLLIN;
srv = apr_poll(&pollset, 1, &n, -1);
if (n == 1 && pollset.rtnevents & APR_POLLIN)
return APR_SUCCESS;
return srv;
}
static void handler(int signum)
{
}
int main()
{
apr_pool_t *pool;
apr_file_t *file;
apr_initialize();
apr_pool_create(&pool, NULL);
apr_file_open_stdin(&file, pool);
apr_signal(SIGINT, handler);
wait_for_input(file, pool);
return 0;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r10132 - trunk/subversion/clients/cmdline
Posted by Ben Reser <be...@reser.org>.
On Sun, Jul 04, 2004 at 11:30:25AM -0400, Greg Hudson wrote:
> This commit seems a little rough, and it seems like an odd enough design
> that perhaps it should have been hashed out on the dev list. (Perhaps
> it was, and I missed it.)
It wasn't. The odd design is a result of APR just flat out not giving
me anyway of doing non-blocking IO on stdin without doing this. APR
actually includes code for non-blocking IO. But it will only use it on
a pipe. *sigh*
You'll note there's a comment in there saying that if and when APR gives
us better way to do this use it. Unfortunately, I don't think that'll
happen for a long time because as far as I can tell even APR 1.0 is
missing support for turning on non-blocking IO for stdin.
Incidentally this is pretty much identical to how APR does non-blocking
IO on pipes.
> On Sun, 2004-07-04 at 01:06, breser@tigris.org wrote:
> > +static apr_status_t wait_for_input_or_timeout(apr_file_t *f,
> > + apr_interval_time_t timeout,
> > + apr_pool_t *pool)
> > +{
>
> > + srv = apr_poll(&pollset, 1, &n, 10);
>
> You appear to be ignoring the timeout parameter.
Yup forgot to change that.
> > + status = wait_for_input_or_timeout(fp,10,pool);
>
> Spaces after commas?
Yup.
> > + if (status != APR_SUCCESS && status != APR_ENOTIMPL)
> > + continue;
>
> Shouldn't you be testing specifically for a timeout result? What's so
> special about APR_ENOTIMPL?
Yup you're right. The above will just loop forever on APR_ENOTIMPL or
an error.
> The timeout should really be unnecessary. Unless I'm missing something,
> apr_poll() should terminate with EINTR if a signal occurs. (On Unix,
> anyway. I don't know how this all works under Windows.)
Umm the timeout is the whole point. I'm trying to get an opportunity to
call the cancel function. I think you missed the entire point of this
exercise. If all I wanted to do was wait until someone typed something
I wouldn't need any of this at all. But I want to wait for people to
type something *AND* periodically poll for a cancel.
If someone can see a better way of achieving this then by all means
please commit it.
The fixes to your comments are committed in r10136. Thanks for catching
them.
--
Ben Reser <be...@reser.org>
http://ben.reser.org
"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org