Patchwork localrepo: better error message with obsmarkers but no evolve

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date Sept. 10, 2015, 1:29 p.m.
Message ID <40f503509524d7b487c8.1441891788@Iris>
Download mbox | patch
Permalink /patch/10458/
State Changes Requested
Headers show

Comments

Jordi Gutiérrez Hermoso - Sept. 10, 2015, 1:29 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1441891451 14400
#      Thu Sep 10 09:24:11 2015 -0400
# Node ID 40f503509524d7b487c8d7b0fe939a52ce966cba
# Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
localrepo: better error message with obsmarkers but no evolve

This error message is quite confusing. It seems to be talking about
some feature that is obsolete, instead of talking about what it's
really about: evolve.

Hopefully this wording clarifies matters.
Augie Fackler - Sept. 10, 2015, 1:42 p.m.
On Thu, Sep 10, 2015 at 09:29:48AM -0400, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1441891451 14400
> #      Thu Sep 10 09:24:11 2015 -0400
> # Node ID 40f503509524d7b487c8d7b0fe939a52ce966cba
> # Parent  ea489d94e1dc1fc3dc1dcbef1c86c18c49605ed1
> localrepo: better error message with obsmarkers but no evolve
>
> This error message is quite confusing. It seems to be talking about
> some feature that is obsolete, instead of talking about what it's
> really about: evolve.

Obsolete markers are very useful without evolve - you can enable them
and get nice performance wins for things like histedit and rebase. The
error message in core should talk about obsolete markers, not evolve.

(I agree the error message is awful, but it needs to be carefully
wordsmithed so that it is sensible even for people don't know about or
care about evolve.)

>
> Hopefully this wording clarifies matters.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -452,7 +452,8 @@ class localrepository(object):
>                                    **kwargs)
>          if store and readonly:
>              self.ui.warn(
> -                _('obsolete feature not enabled but %i markers found!\n')
> +                _('changeset evolution not enabled but %i '
> +                  'obsolescence markers found!\n')
>                  % len(list(store)))
>          return store
>
> diff --git a/setup.py b/setup.py
> --- a/setup.py
> +++ b/setup.py
> @@ -164,7 +164,7 @@ def runhg(cmd, env):
>      err = [e for e in err.splitlines()
>             if not e.startswith(b('not trusting file')) \
>                and not e.startswith(b('warning: Not importing')) \
> -              and not e.startswith(b('obsolete feature not enabled'))]
> +              and not e.startswith(b('changeset evolution not enabled'))]
>      if err:
>          printf("stderr from '%s':" % (' '.join(cmd)), file=sys.stderr)
>          printf(b('\n').join([b('  ') + e for e in err]), file=sys.stderr)
> diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
> --- a/tests/test-obsolete.t
> +++ b/tests/test-obsolete.t
> @@ -695,7 +695,7 @@ Checking _enable=False warning if obsole
>    $ echo '[experimental]' >> $HGRCPATH
>    $ echo "evolution=" >> $HGRCPATH
>    $ hg log -r tip
> -  obsolete feature not enabled but 68 markers found!
> +  changeset evolution not enabled but 68 obsolescence markers found!
>    68:c15e9edfca13 (draft) [tip ] add celestine
>
>  reenable for later test
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Jordi Gutiérrez Hermoso - Sept. 10, 2015, 1:53 p.m.
On Thu, 2015-09-10 at 09:42 -0400, Augie Fackler wrote:
> Obsolete markers are very useful without evolve - you can enable them
> and get nice performance wins for things like histedit and rebase. The
> error message in core should talk about obsolete markers, not
> evolve.

Hm... how do you create obsolete commits without evolve? I didn't
know you could do that.

Come to think of it, why is this a warning at all? Obsolete commits
are already hidden without evolve, so what's the problem?
Augie Fackler - Sept. 10, 2015, 1:55 p.m.
On Thu, Sep 10, 2015 at 9:53 AM, Jordi Gutiérrez Hermoso
<jordigh@octave.org> wrote:
> On Thu, 2015-09-10 at 09:42 -0400, Augie Fackler wrote:
>> Obsolete markers are very useful without evolve - you can enable them
>> and get nice performance wins for things like histedit and rebase. The
>> error message in core should talk about obsolete markers, not
>> evolve.
>
> Hm... how do you create obsolete commits without evolve? I didn't
> know you could do that.

...with rebase? or histedit? All you have to do is turn evolve on.

>
> Come to think of it, why is this a warning at all? Obsolete commits
> are already hidden without evolve, so what's the problem?

I think if obsolete is disabled then the markers aren't loaded and
you'll see your dead changesets (and presumably there will be red in
the camera frame somewhere.)
timeless - Sept. 10, 2015, 10:41 p.m.
jordi wrote:
> This error message is quite confusing.

yes, it's confusing.

> -                _('obsolete feature not enabled but %i markers found!\n')
> +                _('changeset evolution not enabled but %i '
> +                  'obsolescence markers found!\n')

Here are my iterations:
"feature 'obsolete' isn't enabled; %i markers found!\n"
"found %i markers for inactive feature 'obsolete'!\n"
"found %i markers for feature 'obsolete' -- but it isn't enabled!\n"

one of these is probably close to as good as we can get, without adding a URL.
Looking through help:

$ hg --config extensions.evolve=! help -k obsolete
Topics:
 revsets Specifying Revision Sets
Commands:
 debugobsolete       create arbitrary obsolete marker
 debugsuccessorssets show set of successors for revision
 log                 show revision history of entire repository or files

We could point to `hg debugobsolete`, since people could at least
*look* at their information that way, and perhaps decide how important
it is.
Pierre-Yves David - Sept. 10, 2015, 11:26 p.m.
On 09/10/2015 06:55 AM, Augie Fackler wrote:
> On Thu, Sep 10, 2015 at 9:53 AM, Jordi Gutiérrez Hermoso
> <jordigh@octave.org> wrote:
>> On Thu, 2015-09-10 at 09:42 -0400, Augie Fackler wrote:
>>> Obsolete markers are very useful without evolve - you can enable them
>>> and get nice performance wins for things like histedit and rebase. The
>>> error message in core should talk about obsolete markers, not
>>> evolve.
>>
>> Hm... how do you create obsolete commits without evolve? I didn't
>> know you could do that.
>
> ...with rebase? or histedit? All you have to do is turn evolve on.

So to create marker without evolve you need evolve? ;-)

Adding this config to a vanillia mercurial would do it:

   [experimental]
   evolution=createmarkers


>> Come to think of it, why is this a warning at all? Obsolete commits
>> are already hidden without evolve, so what's the problem?
>
> I think if obsolete is disabled then the markers aren't loaded and
> you'll see your dead changesets (and presumably there will be red in
> the camera frame somewhere.)

Markers are loaded and eevrything. But this is a very good hint of 
something going wrong with the user config. This already saved 
wheelbarrow of baby seal in the wild.

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -452,7 +452,8 @@  class localrepository(object):
                                   **kwargs)
         if store and readonly:
             self.ui.warn(
-                _('obsolete feature not enabled but %i markers found!\n')
+                _('changeset evolution not enabled but %i '
+                  'obsolescence markers found!\n')
                 % len(list(store)))
         return store
 
diff --git a/setup.py b/setup.py
--- a/setup.py
+++ b/setup.py
@@ -164,7 +164,7 @@  def runhg(cmd, env):
     err = [e for e in err.splitlines()
            if not e.startswith(b('not trusting file')) \
               and not e.startswith(b('warning: Not importing')) \
-              and not e.startswith(b('obsolete feature not enabled'))]
+              and not e.startswith(b('changeset evolution not enabled'))]
     if err:
         printf("stderr from '%s':" % (' '.join(cmd)), file=sys.stderr)
         printf(b('\n').join([b('  ') + e for e in err]), file=sys.stderr)
diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
--- a/tests/test-obsolete.t
+++ b/tests/test-obsolete.t
@@ -695,7 +695,7 @@  Checking _enable=False warning if obsole
   $ echo '[experimental]' >> $HGRCPATH
   $ echo "evolution=" >> $HGRCPATH
   $ hg log -r tip
-  obsolete feature not enabled but 68 markers found!
+  changeset evolution not enabled but 68 obsolescence markers found!
   68:c15e9edfca13 (draft) [tip ] add celestine
 
 reenable for later test