>
-------------------------------------------------------------------------- This message, including its attachments, is confidential. For more information please read NNG's email policy here: http://www.nng.com/emailpolicy/ By responding to this email you accept the email policy. -----Original Message----- > From: Augie Fackler [mailto:r...@durin42.com] > Sent: Friday, December 16, 2016 3:13 AM > To: Gábor STEFANIK <gabor.stefa...@nng.com> > Cc: Jun Wu <qu...@fb.com>; mercurial-devel <mercurial-devel@mercurial- > scm.org> > Subject: Re: [PATCH] update: introduce config option ui.allowdirtyupdate for > dirty nonlinear updates > > This ended up long, because I’m riffing a bit on what it means to be > experimental etc. If you don’t care about those details (most readers of this > list probably don’t?), skip to the “TL;DR:” bit and let’s talk about footguns > now that the bikeshed is painted... > > > On Dec 13, 2016, at 11:48 AM, Gábor STEFANIK > <gabor.stefa...@nng.com> wrote: > > > > -----Original Message----- > >> On Thu, Dec 8, 2016 at 9:13 AM, Gábor STEFANIK > >> <gabor.stefa...@nng.com> wrote: > >>> > >>> -----Original Message----- > >>>> From: Jun Wu [mailto:qu...@fb.com] > >>>> Sent: Wednesday, December 7, 2016 6:44 PM > >>>> To: Gábor STEFANIK <gabor.stefa...@nng.com> > >>>> Cc: mercurial-devel <mercurial-devel@mercurial-scm.org> > >>>> Subject: Re: [PATCH] update: introduce config option > >>>> ui.allowdirtyupdate for dirty nonlinear updates > >>>> > >>>> Excerpts from Gábor Stefanik's message of 2016-12-07 16:56:09 +0100: > >>>>> # HG changeset patch > >>>>> # User Gábor Stefanik <gabor.stefa...@nng.com> # Date 1481126137 > - 3600 > >>>>> # Wed Dec 07 16:55:37 2016 +0100 > >>>>> # Node ID dabbe365b843fcf9b8a0de6c08e9db6100b391e9 > >>>>> # Parent 6472c33e16326b8c817a8bae0e75053b19badb2c > >>>>> update: introduce config option ui.allowdirtyupdate for dirty > >>>>> nonlinear updates > > [...] > > >>> > >>> This is experimental for now, since we need to support "hg update > >>> --abort" to make this safe for users, but eventually I hope to get > >>> this de-experimentalized and maybe even enabled by default. It would > >>> be better not to break backwards compatibility just because this > >>> becomes no longer experimental. > >> > >> Until we're confident that this feature will live forever, it should > >> be in experimental. > > > > I would argue that this will live forever, because: > > a) If we eventually decide to make this the default > > (By this point it wouldn’t be experimental...) > > > , some users will want to > > still have the old behavior - which they can get by explicitly setting > > ui.allowdirtyupdate=False. I don't envision ever dropping the current > > behavior completely. > > Yep! That’s very true. Note that if we were going to try and move the default > (or even the recommended setting), I’d probably bias in the other direction: > making something like --check the default, in the name of greater safety. The > only way I can see that changing would be if we had some fast-but-reliable > way to undo an in-progress update that hit merge conflicts. "Fast-but-reliable way to undo an in-progress-update" is bug 4404. I've outlined the solution on Bugzilla for that one, the only thing needed is a place to store the revision we're updating from. We do save some revision in the merge state files, but I don't know if that's always guaranteed to be the right revision, or if it's instead possibly an older/ancestral revision where the files being merged were last modified. The solution I outlined would look like this: 1. Find the revision "last()" which we have to update back to. 2. For files with saved mergestate, restore the "local" version. 3. Update all "clean" files to the file revisions in "last()". (4. Leave "dirty" files with no mergestate unchanged - if there were any change to undo, the original update would have resulted in the files gaining mergestate.) Always defaulting to --check is another way to fix the UI inconsistency where some updates just work, but in a potentially unsafe way; while others fail with an obscure "not a linear update" error message (the user doesn't know or care what our code considers "linear") - the problem is that it would be a BC break. > > > b) If we decide that nonlinear merge updates shall never be supported > > using the "update" command for whatever reason, we will still need it > > for the same debugging uses that we have now. > > I guess I don’t know what those cases are. Any examples that’d help me > understand the use case? We currently do updates that are technically nonlinear when updating between evolutionary predecessors and successors. The process for this is essentially the same as a graft, but with the "c1" and "c2" changesets role-reversed. Whenever we encounter and fix an issue related to grafting, it's therefore prudent to verify that the fix also works correctly for updating. Right now, the only way to do that is to perform a complicated dance of purges and evolves, even though the functionality being tested isn't really evolve-related. It's much easier to just force a non-linear update using a config option in this case. The recent fix to bug 5436 was an example of such a fix. I originally included code to handle the "update" case, but it ended up not working for other reasons (directory rename detection isn't even run in the "update" version of bug 5436). In the end I removed the "update-side" code entirely, as I couldn't find a way to make it work that wouldn't require a major rewrite in other areas. Bug 4028 similarly had an update-based variant - to test the fix for that, I ended up locally patching my Hg test instances to bypass the linearity checks. > > > This is as likely to live forever as merge.preferancestor, which is > > also marked with "experimental config:", but not in [experimental]. > > It may interest you to know that preferancestor predates the existence of > [experimental]. > > > In terms of backwards compatibility, I would also argue that we > > shouldn't use [experimental] for options that _may_ live forever > > either, only for those that are guaranteed to be temporary (e.g. > > experimental.bundle2-exp, which is to be removed as bundle2 becomes > mandatory for GD->GD clones): > > An "experimental, may live forever" option can have 2 outcomes: > > a) it's dropped before it matures, which is always a BC break > > It’s explicitly *not* a BC break as hg defines BC. That’s the point of > experimental: we can discard the experiment after a long field trial without > having to carry the baggage forever. > > > , or > > b) it matures, and loses its experimental status. > > > > With a), it doesn't matter if we use [experimental] or the appropriate > > regular section - the outcome is always a BC break. With b), going > > straight for the final intended section is a clear BC win - those > > using the experimental-era option won't have to change anything when > > it matures, whereas if we initially use [experimental], then move the > > option to another section once mature, everyone using [experimental] > > in their hgrcs needs to take action (or we must keep the [experimental] > version as alias). > > Yep, that’s a real pain point. We definitely felt it on some things. > > I suppose I could be happy with leaving this in [ui] given that it’s > something I > don’t expect to have on by default as long as it’s prominently labelled in any > documentation as experimental, and that we reserve the right to take it out > if it ends up being too costly to maintain or too much of a footgun for users. > We probably ought to document when to use [experimental] vs > documenting something as experimental. I’m no expert, but this patch has > certainly been an interesting foil for the thought process. > > > I know we don't offer BC guarantees for experimental and debug > > features, but I would still argue against gratuitously breaking BC for no > > gain. > > Nitpick: there’s almost always gain in that we get to throw away code. I was referring to the case when the option graduates from experimental to stable. Having to move from [experimental] to [ui] only complicates things for us, and also for testers/early adopters. If the option is dropped entirely, we throw away code handling it regardless of whether it was in [ui] or [experimental]. > > TL;DR: > > I’m not unsympathetic to your concerns, but this particular feature worries > me in terms of user support load just on its face. Does that make sense? I’m > trying to play a 20-year-horizon usability-and-safety game here, as well as a > 20-year-horizon maintainability game. > > I think in the process of writing this email, I’ve convinced myself that [ui] > with > copious experiment warnings is the right location, but that I’m worried about > the existence of the config knob at all. Not sure what to say, other than to > ask if you can allay my fears that this feature is a massive footgun waiting > to > happen. Regardless of how big a footgun it turns out to be, or how well it can be defootgunized by fixing bug 4404, it's no less a footgun for evolutionarily-linear updates (where Hg already allows this behavior) than for evolutionarily-nonlinear ones. > > Thanks! > Augie _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
