Jason, On Fri, Jan 17, 2014 at 11:13 AM, Jason Tishler <[email protected]> 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. 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. > 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. > 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. David -- 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
