X-Recipient:	archive-cygwin AT delorie DOT com
DomainKey-Signature:	a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id
	:list-unsubscribe:list-subscribe:list-archive:list-post
	:list-help:sender:date:from:to:subject:message-id:references
	:mime-version:content-type:in-reply-to; q=dns; s=default; b=F8xx
	dsugyKcWd9zI+ZS+kyAP2g1Zo/+hEr0vo34SmdPtMZ3hr33OE9oF1brWxlW1PzFh
	P88GlmqeciY017YJnAZoFAJrpOtuyBNtU/snTIpfiMdYtZicLDb9ByxQEpHWyiun
	Hr7zx6zk5i92imeiseK/m7ui+9FYwtHKU9xOBwU=
DKIM-Signature:	v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id
	:list-unsubscribe:list-subscribe:list-archive:list-post
	:list-help:sender:date:from:to:subject:message-id:references
	:mime-version:content-type:in-reply-to; s=default; bh=w4PRjT46/V
	0jMDV64UEAaYZGh4U=; b=Mt0M6YHxY5qHI06zrX4cUwrrM7x54rbjjvYKzAjEmJ
	q2MVBoUvB6umlUjUFAEGOz/+QdE/NFEQYKFdQ6D8s1EMbCXc8Msf3LvpFtZMe0T+
	BOg7USXa87nbWeuGULGhyiCZxc4OLWoOKycbL56Vud4Ipw0liWtpfgEGwPY8i6/Q
	Q=
Mailing-List:	contact cygwin-help AT cygwin DOT com; run by ezmlm
List-Id:	<cygwin.cygwin.com>
List-Subscribe:	<mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive:	<http://sourceware.org/ml/cygwin/>
List-Post:	<mailto:cygwin AT cygwin DOT com>
List-Help:	<mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender:	cygwin-owner AT cygwin DOT com
Mail-Followup-To:	cygwin AT cygwin DOT com
Delivered-To:	mailing list cygwin AT cygwin DOT com
Authentication-Results:	sourceware.org; auth=none
X-Virus-Found:	No
X-Spam-SWARE-Status:	No, score=-0.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2
X-HELO:	tishler.net
Date:	Fri, 17 Jan 2014 12:45:47 -0500
From:	Jason Tishler <jason AT tishler DOT net>
To:	cygwin AT cygwin DOT com
Subject:	Re: Add retry logic to rebaseall
Message-ID:	<20140117174546.GB59772@tishler.net>
Mail-Followup-To:	cygwin AT cygwin DOT com
References:	<CACUHbYNK6e+NrBiNo9RUynqc=F39mLWnizQNFeU-DUSVj0rmeQ AT mail DOT gmail DOT com> <20140116061906 DOT GA1992 AT ednor DOT casa DOT cgf DOT cx> <20140116085431 DOT GB26205 AT calimero DOT vinschen DOT de> <20140117161333 DOT GA59772 AT tishler DOT net> <CACUHbYP9zSQDENbt1tV9WY_jQx5ayxySfTVryt6u6nZAD5u88A AT mail DOT gmail DOT com>
MIME-Version:	1.0
In-Reply-To:	<CACUHbYP9zSQDENbt1tV9WY_jQx5ayxySfTVryt6u6nZAD5u88A@mail.gmail.com>
User-Agent:	Mutt/1.5.21 (2010-09-15)
X-IsSubscribed:	yes

David,

On Fri, Jan 17, 2014 at 11:30:54AM -0500, David Boyce wrote:
> On Fri, Jan 17, 2014 at 11:13 AM, Jason Tishler <jason AT tishler DOT net> wrote:
> > AFAICT, there is a race condition issue with the proposed
> > functionality.  David's build servers could be quiescent when the
> > check for running processes is performed, but they could restart
> > before the rebase is finished.  I realize the window is very small,
> > but it is nevertheless nonzero, so the rebase could still fail.
> 
> I don't think race condition is the right phrase here. This is the
> exact same situation faced by the existing rebaseall functionality; it
> knows there are no Cygwin processes running at start time but any
> process could start between then and end time.

Yes, but rebaseall it typically called manually, so the user wouldn't do
this or would have to deal with the consequences if they did.

> I agree there's a window where things could go wrong, but this feature
> doesn't worsen it. Closing that window is an orthogonal effort, IMHO;
> the new flag isn't called --make-sure-the-rebase-works-dammit.

IMO, if a feature adds automation (i.e., no human intervention required),
then it should try to work under all conditions, if feasible without too
much effort.  Your wait feature could also detect rebase failures and
retry until it succeeds (maybe giving up after N attempts).

> > There are also formatting issues with the patch.  For example, the
> > addition of the while loop requires lines to be shifted to the right.
> > I know rebaseall unfortunately has a mixture of tabs and spaces, but
> > after applying the patch some lines are not indented correctly.
> 
> I had a paragraph about that in the original email which got rejected
> due to "spam score too high" so I cut the text down for the second try
> and ended up losing that part. Yes, the original has a mix of tabs and
> spaces and my editor might be configured differently from yours, so I
> made the patch using "diff -u -w". That may have been a mistake; I'm
> happy to clean it up if asked.

I agree that the formatting issue is minor, but a patch should be
generated to minimize a maintainer's efforts.

> > IMO, the proposed functionality is very specialized and doesn't seem
> > to be generally applicable.  This functionality could also be
> > implemented (by the few who need it) as a very simple wrapper script
> > that calls rebaseall until is succeeds.  This approach would also
> > workaround the race condition.
> 
> How so? The behavior and risk factors would be identical as far as I
> can see.

If you wrap rebaseall in its entirety and retry on failure until
successful, then that will eliminate the "race condition" issue.

If consensus thinks this change is generally useful, then rebaseall
should be changed accordingly.  Otherwise, you can write a few line
wrapper script to meet your special needs.

Jason

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

- Raw text -