Mike Kupfer <mike.kupfer at sun.com> writes: > I've looked at 359 and 360 so far. > >> 360 recommit should check for workspace modification in the same >> manner as Mq would > > Looks fine. > >> 359 when recommit fails, cdm needs to make more effort to return the >> workspace to sanity. > [...] >> I'd like input on whether bringing the workspace back to the local >> tip or bringing it back to where it was previously is more correct. >> >> (there's further changes in that area I may end up making, however, >> currently, the local tip is the least confusing place to be) > > I think we should try to return the user to where she was. If we can't > do that, we should provide feedback as to where we left her.
It should be yes, if it's still present. More practically, we can return them to either where they were, or the new tip (another option, which I didn't mention). > Is that possible? If not, can we require that the user always be at the > local tip before running recommit? Would that be a good requirement in > general? That's part of what I was considering with "further changes" above, currently we take an informed guess based on the branch the user is on (which, in the case of recommit, will always lead to a single revision. Whether that remains true depends on what we decide to do about in-workspace branching (allow it? disallow it?) It would make sense, to me, to force them to already be at the tip when they do this, however, we take them there anyway as part of running. > Come to think of it, can the exception handling code assume that the > local tip still exists? (Is there a particular order to what > active.bases() returns?) The order the bases come back in is less important than which of the bases will remove the localtip. Yes, they come back in a well defined order, no, that doesn't allow us to guarantee when the local tip will be removed. My logic was that it would always be removed last, since all bases lead to that one tip, eventually. The more I think about it, the more I'm concerned that's not actually the case however. > Also, what happens to the new tip if we catch an exception from > q.strip()? It remains. I'm unwilling to allow recommit to remove a change that is not going to remain in some other fashion (this is why we construct and commit the conglomerate *then* remove local change), I'm not willing to remove the new tip if there's any possibility of us having removed any of the prior change, and in this case, we have. It would be unsafe. -- Rich