[PATCH 3 of 3] dispatch: only check compatibility against major and minor versions

[Augie Fackler]

On Thu, Jan 15, 2015 at 11:38:28PM -0800, Gregory Szorc wrote:
> On Thu, Jan 15, 2015 at 10:52 PM, Ryan McElroy <rm at fb.com> wrote:
>
> > On 1/15/2015 8:36 PM, Gregory Szorc wrote:
> >>   Test version number support in 'hg version':
> >>     $ echo '__version__ = (1, 2, 3)' >> throw.py
> >>     $ rm -f throw.pyc throw.pyo
> >>     $ hg version -v
> >>
> >>
> > I'm happy enough with this series (I'm not for or against this third patch
> > the the other two seem strictly positive). I might argue against this third
> > patch on the basis that major revisions don't seem to have any semantic
> > meaning in mercurial (as I understand it -- but I'm new here -- hg goes
> > from 2.9 to 3.0 on a schedule, not on a set of features). However, I could
> > argue for it in that it reduces the overhead of maintaining extensions to
> > every ten freezes instead of every freeze, which is probably a net win for
> > the world overall.
> >
>
> Unless I did something wrong, I reduced the warn threshold to every 3rd
> freeze, not ten. We should be looking at X.Y versions.
>
>
> > That being said, the whole blaming an extension based on declared
> > compatibility is totally broken. I'd much rather see us try to guess where
> > the break is based on any extension being in the stack trace, otherwise not
> > assigning blame to any particular extension. In my experience at FB, this
> > code *always* blames crecord, which is never actually at fault.
> >
> > I'll probably take a crack at doing something like this eventually if
> > nobody else ever does, but it's not very high on any priority list.
> >
> > Another thing that I think would be good is to write the stack trace to a
> > file (and mention this file in the crash report) but never expose the user
> > to a stack trace. For most users, a stack trace is pretty scary looking and
> > a nice "something went wrong" message with instructions on how to report it
> > without the stack trace would be a lot more approachable. But now I'm just
> > rambling :-)
> >
>
> I completely agree about blaming being almost completely busted. I, too,
> wish it were based on stacks. But even that isn't perfect. For example, you
> can get a TypeError at a call site when passing an unknown argument into a
> function in an extension that doesn't accept that argument. Of course, you
> can also get a TypeError inside a function. How do you attribute which file
> is to blame? I argue that in the current world of relying on
> monkeypatching, you can't. One of the benefits of a more well-defined
> extensibility API (such as the events proposal I sent a few months back) is
> that call sites into extensions are more clearly defined. So, if an error
> occurs, we can look at the module/file being called into an attribute it to
> an extension. That's better but still not perfect. You still have a general
> class of errors around "extension manipulated something wrong and we didn't
> find out about it until later." Oy.

I think we could probably do significantly better by recording a list
of what extension wrapped what things in extensions.wrap*, and then
pattern matching the bottommost stack frame against and see if we can
point a more accurate finger.

>
> I like your idea of writing stacks to files! I'd love to see that in 3.4,
> even if it's behind a feature flag.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel