Re: [PATCH] match: adding non-recursive directory matching

2016-12-19 Thread Rodrigo Damazio via Mercurial-devel
Unfortunately, while set would match the right files, because of the way
the code is structured, it provides no way to not try visiting the
directories inside the non-recursive match - the set needs to first collect
all the files in all subdirectories (match.py, _expandset) and then filter
that down to the desired ones. In plain hg repos, that's just much slower -
in the context of narrowhg, the repo will simply not have the manifests for
those subdirectories and trying to visit them will crash.

The follow-up change to this one (which I haven't sent yet but is written)
is updating visitdir to allow non-recursiveness, which btw makes something
like "hg files -I rootglob:browser/*" about 4-5x faster in the firefox repo.


On Fri, Dec 16, 2016 at 6:21 AM, Pierre-Yves David <
pierre-yves.da...@ens-lyon.org> wrote:

>
>
> On 12/16/2016 02:19 AM, Augie Fackler wrote:
>
>>
>> On Nov 24, 2016, at 10:28 AM, FUJIWARA Katsunori 
>>> wrote:
>>>
>>> Yes, "files:" was the original version of this patch and the case I
> really
> care about :) I changed it to rootglob after your comments.
> Which way would be preferred to move forward?
>

>>> "files:" is "path:" family, and "rootglob:" is "glob:" family. As we
>>> concluded before, "path:" itself can't control recursion of matching
>>> well.
>>>
>>> Therefore, I think that "files:" should be implemented if needed,
>>> regardless of implementing "rootglob:".
>>>
>>> Of course, we need high point view of this area, at first :-)
>>>
>>>
>>> BTW, it is a little ambiguous (at least, for me) that "files:foo"
>>> matches against both file "foo" and files just under directory
>>> "foo". Name other than "files:" may resolve this ambiguity, but I
>>> don't have any better (and short enough) name :-<
>>>
>>>  ==  === ===
>>>  patternfoo  foo/bar foo/bar/baz
>>>  ==  === ===
>>>  path:fooo o o
>>>
>>>  files:foo   o o x
>>>
>>>  file:fooo x x
>>>  dir:foo x o o
>>>  ==  === ===
>>>
>>>
>> Scanning the plan page, I see that there’s a *lot* of work that could be
>> done and no consensus as yet, but that the only immediate use case seems to
>> be the rootfile/rootglob case. Is there some path forward we could agree on
>> that would unblock those immediate needs for narrowhg and not make things
>> harder in the future?
>>
>> Alternatively, would we be okay with a slight refactor of the matcher so
>> that narrowhg can introduce a custom filesonly: matcher for the time being
>> so we can keep making forward progress there?  I don’t know the matcher
>> code well enough to be able to guess if this is a reasonable path so we can
>> be unblocked.
>>
>> (It’s very hard for to justify the amount of work implied by reaching
>> consensus on FileNamePatternsPlan and then executing the entire thing when
>> what we need is solvable today with a sub-hour patch to existing code, thus
>> my trying to find a solution we can all live with.)
>>
>
> As far as I understand, Foozy finding shows that the feature narrowhg
> needs is already there and nothing new is necessary.
>
> You can add "set:" in front of your glob to make it non recursive in all
> cases "set:your/directory/you/want/to/match/files/in/*"
>
> If this does not fits your needs, this probably mean I got your usecase
> wrong. In that case can you re-explain the issue you are trying to solve
> here?
>
>
> At the project level, it will make sense to clean up the Pattern Matching
> at some point, and Foozy wiki work will help us to do that.
>
> Cheers.
>
> --
> Pierre-Yves David
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH] p4: drop an assignment that is never used

2016-12-19 Thread Jun Wu
# HG changeset patch
# User Jun Wu 
# Date 1482186162 0
#  Mon Dec 19 22:22:42 2016 +
# Node ID a99748ec71eb65b50dabf1d73ab9de0c6b9124b2
# Parent  935092e525b0ee5656d0830162a1c2adf8248de3
# Available At https://bitbucket.org/quark-zju/hg-draft
#  hg pull https://bitbucket.org/quark-zju/hg-draft -r a99748ec71eb
p4: drop an assignment that is never used

Discovered by pyflakes:

  hgext/convert/p4.py:305: local variable 'shortdesc' is assigned to but
  never used

diff --git a/hgext/convert/p4.py b/hgext/convert/p4.py
--- a/hgext/convert/p4.py
+++ b/hgext/convert/p4.py
@@ -303,6 +303,4 @@ class p4_source(common.converter_source)
 """
 desc = self.recode(obj.get("desc", ""))
-shortdesc = desc.split("\n", 1)[0]
-
 date = (int(obj["time"]), 0) # timezone not set
 if parents is None:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 2 of 3 V2] chg: start server at a unique address

2016-12-19 Thread Jun Wu
# HG changeset patch
# User Jun Wu 
# Date 1482185389 0
#  Mon Dec 19 22:09:49 2016 +
# Node ID 24534dee38fe515f2c406e18153f623a074148b3
# Parent  c543f9fd1a5b7657d4114e61e87a8d9bc32f7617
# Available At https://bitbucket.org/quark-zju/hg-draft
#  hg pull https://bitbucket.org/quark-zju/hg-draft -r 24534dee38fe
chg: start server at a unique address

See the previous patch for motivation. Previously, the server is started at
a globally shared address. This patch appends pid to the address so it
becomes unique.

Note: with Linux pid namespace, the address may be non-unique, but it does
not affect correctness of chg - chg client will receive an redirection and
that's it.

diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
--- a/contrib/chg/chg.c
+++ b/contrib/chg/chg.c
@@ -32,4 +32,5 @@
 struct cmdserveropts {
char sockname[UNIX_PATH_MAX];
+   char initsockname[UNIX_PATH_MAX];
char redirectsockname[UNIX_PATH_MAX];
char lockfile[UNIX_PATH_MAX];
@@ -164,4 +165,8 @@ static void setcmdserveropts(struct cmds
if (r < 0 || (size_t)r >= sizeof(opts->lockfile))
abortmsg("too long TMPDIR or CHGSOCKNAME (r = %d)", r);
+   r = snprintf(opts->initsockname, sizeof(opts->initsockname),
+   "%s.%u", opts->sockname, (unsigned)getpid());
+   if (r < 0 || (size_t)r >= sizeof(opts->initsockname))
+   abortmsg("too long TMPDIR or CHGSOCKNAME (r = %d)", r);
 }
 
@@ -224,5 +229,5 @@ static void execcmdserver(const struct c
"serve",
"--cmdserver", "chgunix",
-   "--address", opts->sockname,
+   "--address", opts->initsockname,
"--daemon-postexec", "chdir:/",
};
@@ -248,5 +253,5 @@ static hgclient_t *retryconnectcmdserver
int pst = 0;
 
-   debugmsg("try connect to %s repeatedly", opts->sockname);
+   debugmsg("try connect to %s repeatedly", opts->initsockname);
 
unsigned int timeoutsec = 60;  /* default: 60 seconds */
@@ -256,7 +261,13 @@ static hgclient_t *retryconnectcmdserver
 
for (unsigned int i = 0; !timeoutsec || i < timeoutsec * 100; i++) {
-   hgclient_t *hgc = hgc_open(opts->sockname);
-   if (hgc)
+   hgclient_t *hgc = hgc_open(opts->initsockname);
+   if (hgc) {
+   debugmsg("rename %s to %s", opts->initsockname,
+   opts->sockname);
+   int r = rename(opts->initsockname, opts->sockname);
+   if (r != 0)
+   abortmsgerrno("cannot rename");
return hgc;
+   }
 
if (pid > 0) {
@@ -270,5 +281,5 @@ static hgclient_t *retryconnectcmdserver
}
 
-   abortmsg("timed out waiting for cmdserver %s", opts->sockname);
+   abortmsg("timed out waiting for cmdserver %s", opts->initsockname);
return NULL;
 
@@ -313,5 +324,5 @@ static hgclient_t *connectcmdserver(stru
unlink(opts->sockname);
 
-   debugmsg("start cmdserver at %s", opts->sockname);
+   debugmsg("start cmdserver at %s", opts->initsockname);
 
pid_t pid = fork();
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 1 of 3 V2] chgserver: truncate base address at "." for hash address

2016-12-19 Thread Jun Wu
# HG changeset patch
# User Jun Wu 
# Date 1482185261 0
#  Mon Dec 19 22:07:41 2016 +
# Node ID c543f9fd1a5b7657d4114e61e87a8d9bc32f7617
# Parent  935092e525b0ee5656d0830162a1c2adf8248de3
# Available At https://bitbucket.org/quark-zju/hg-draft
#  hg pull https://bitbucket.org/quark-zju/hg-draft -r c543f9fd1a5b
chgserver: truncate base address at "." for hash address

Previously, the hash address is just appending "-$HASH" to base address.
This patch makes it truncate the basename address at "." before appending
"-$HASH".

This makes it possible to spawn new servers in a racy situation and the
client could be sure the server it connects is the new server just spawned.

This is a step towards removing the lock.

One of the functionalities of the lock is to make sure the connect will
connect to a server it just created:

  1. start server --address foo
  2. connect to foo # wish "foo" is the server just started

With this change, the client could do:

  1. start server --address foo.tmp$PID
  2. connect to foo.tmp$PID # is the serer just started
 (note: if it is not, it does not affect correctness - linux pid
  namespace is not a concern here)
  3. rename foo.tmp$PID to foo

Another functionality of the lock is to avoid starting multiple servers with
a same confighash in parallel. But that also prevents starting multiple
servers with different confighashes in parallel.

diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
--- a/mercurial/chgserver.py
+++ b/mercurial/chgserver.py
@@ -530,5 +530,10 @@ def _tempaddress(address):
 
 def _hashaddress(address, hashstr):
-return '%s-%s' % (address, hashstr)
+# if the basename of address contains '.', use only the left part. this
+# makes it possible for the client to pass 'server.tmp$PID' and follow by
+# an atomic rename to avoid locking when spawning new servers.
+dirname, basename = os.path.split(address)
+basename = basename.split('.', 1)[0]
+return '%s-%s' % (os.path.join(dirname, basename), hashstr)
 
 class chgunixservicehandler(object):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 3 of 3 V2] chg: remove locks

2016-12-19 Thread Jun Wu
# HG changeset patch
# User Jun Wu 
# Date 1482185700 0
#  Mon Dec 19 22:15:00 2016 +
# Node ID 467f4027b8d0162e3e8f09cd0cc0ca627164bce1
# Parent  24534dee38fe515f2c406e18153f623a074148b3
# Available At https://bitbucket.org/quark-zju/hg-draft
#  hg pull https://bitbucket.org/quark-zju/hg-draft -r 467f4027b8d0
chg: remove locks

See the previous two patches for the reason. The advantage is a simplified
code base and better throughput when starting multiple servers with multiple
confighashes. The disadvantage is starting multiple servers in parallel with
a single confighash will waste some CPU time, which is probably fine in
common use-cases.

This makes it easier to switch to relative paths to support long unix domain
socket paths.

diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
--- a/contrib/chg/chg.c
+++ b/contrib/chg/chg.c
@@ -34,8 +34,6 @@ struct cmdserveropts {
char initsockname[UNIX_PATH_MAX];
char redirectsockname[UNIX_PATH_MAX];
-   char lockfile[UNIX_PATH_MAX];
size_t argsize;
const char **args;
-   int lockfd;
int sockdirfd;
 };
@@ -43,5 +41,4 @@ struct cmdserveropts {
 static void initcmdserveropts(struct cmdserveropts *opts) {
memset(opts, 0, sizeof(struct cmdserveropts));
-   opts->lockfd = -1;
opts->sockdirfd = -1;
 }
@@ -51,5 +48,4 @@ static void freecmdserveropts(struct cmd
opts->args = NULL;
opts->argsize = 0;
-   assert(opts->lockfd == -1 && "should be closed by unlockcmdserver()");
if (opts->sockdirfd >= 0) {
close(opts->sockdirfd);
@@ -158,11 +154,7 @@ static void setcmdserveropts(struct cmds
const char *basename = (envsockname) ? envsockname : sockdir;
const char *sockfmt = (envsockname) ? "%s" : "%s/server";
-   const char *lockfmt = (envsockname) ? "%s.lock" : "%s/lock";
r = snprintf(opts->sockname, sizeof(opts->sockname), sockfmt, basename);
if (r < 0 || (size_t)r >= sizeof(opts->sockname))
abortmsg("too long TMPDIR or CHGSOCKNAME (r = %d)", r);
-   r = snprintf(opts->lockfile, sizeof(opts->lockfile), lockfmt, basename);
-   if (r < 0 || (size_t)r >= sizeof(opts->lockfile))
-   abortmsg("too long TMPDIR or CHGSOCKNAME (r = %d)", r);
r = snprintf(opts->initsockname, sizeof(opts->initsockname),
"%s.%u", opts->sockname, (unsigned)getpid());
@@ -171,37 +163,4 @@ static void setcmdserveropts(struct cmds
 }
 
-/*
- * Acquire a file lock that indicates a client is trying to start and connect
- * to a server, before executing a command. The lock is released upon exit or
- * explicit unlock. Will block if the lock is held by another process.
- */
-static void lockcmdserver(struct cmdserveropts *opts)
-{
-   if (opts->lockfd == -1) {
-   opts->lockfd = open(opts->lockfile,
-   O_RDWR | O_CREAT | O_NOFOLLOW, 0600);
-   if (opts->lockfd == -1)
-   abortmsgerrno("cannot create lock file %s",
- opts->lockfile);
-   fsetcloexec(opts->lockfd);
-   }
-   int r = flock(opts->lockfd, LOCK_EX);
-   if (r == -1)
-   abortmsgerrno("cannot acquire lock");
-}
-
-/*
- * Release the file lock held by calling lockcmdserver. Will do nothing if
- * lockcmdserver is not called.
- */
-static void unlockcmdserver(struct cmdserveropts *opts)
-{
-   if (opts->lockfd == -1)
-   return;
-   flock(opts->lockfd, LOCK_UN);
-   close(opts->lockfd);
-   opts->lockfd = -1;
-}
-
 static const char *gethgcmd(void)
 {
@@ -309,12 +268,4 @@ static hgclient_t *connectcmdserver(stru
return hgc;
 
-   lockcmdserver(opts);
-   hgc = hgc_open(sockname);
-   if (hgc) {
-   unlockcmdserver(opts);
-   debugmsg("cmdserver is started by another process");
-   return hgc;
-   }
-
/* prevent us from being connected to an outdated server: we were
 * told by a server to redirect to opts->redirectsockname and that
@@ -335,5 +286,4 @@ static hgclient_t *connectcmdserver(stru
}
 
-   unlockcmdserver(opts);
return hgc;
 }
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] ui: do not translate empty configsource() to 'none' (API)

2016-12-19 Thread Augie Fackler
On Sun, Dec 18, 2016 at 06:54:27PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara 
> # Date 1477212420 -32400
> #  Sun Oct 23 17:47:00 2016 +0900
> # Node ID f2565fc73557e6cbf78119da2953bfae6d457f94
> # Parent  7d505a99f16d0a7faae231432deef5c9bcfd3ad1
> ui: do not translate empty configsource() to 'none' (API)

Sure, queued, thanks.

>
> It should be processed when displaying data, so we can get "source": "" in
> JSON output.
>
> diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
> --- a/mercurial/chgserver.py
> +++ b/mercurial/chgserver.py
> @@ -271,9 +271,6 @@ def _loadnewui(srcui, args):
>  if ':' in source or source == '--config':
>  # path:line or command line
>  continue
> -if source == 'none':
> -# ui.configsource returns 'none' by default
> -source = ''
>  newui.setconfig(section, name, value, source)
>
>  # load wd and repo config, copied from dispatch.py
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -1804,29 +1804,28 @@ def config(ui, repo, *values, **opts):
>  raise error.Abort(_('only one config item permitted'))
>  matched = False
>  for section, name, value in ui.walkconfig(untrusted=untrusted):
> +source = ui.configsource(section, name, untrusted)
>  value = str(value)
>  if fm.isplain():
> +source = source or 'none'
>  value = value.replace('\n', '\\n')
>  entryname = section + '.' + name
>  if values:
>  for v in values:
>  if v == section:
>  fm.startitem()
> -fm.condwrite(ui.debugflag, 'source', '%s: ',
> - ui.configsource(section, name, untrusted))
> +fm.condwrite(ui.debugflag, 'source', '%s: ', source)
>  fm.write('name value', '%s=%s\n', entryname, value)
>  matched = True
>  elif v == entryname:
>  fm.startitem()
> -fm.condwrite(ui.debugflag, 'source', '%s: ',
> - ui.configsource(section, name, untrusted))
> +fm.condwrite(ui.debugflag, 'source', '%s: ', source)
>  fm.write('value', '%s\n', value)
>  fm.data(name=entryname)
>  matched = True
>  else:
>  fm.startitem()
> -fm.condwrite(ui.debugflag, 'source', '%s: ',
> - ui.configsource(section, name, untrusted))
> +fm.condwrite(ui.debugflag, 'source', '%s: ', source)
>  fm.write('name value', '%s=%s\n', entryname, value)
>  matched = True
>  fm.end()
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -249,8 +249,9 @@ class ui(object):
>  if not p:
>  continue
>  if '%%' in p:
> +s = self.configsource('paths', n) or 'none'
>  self.warn(_("(deprecated '%%' in path %s=%s from 
> %s)\n")
> -  % (n, p, self.configsource('paths', n)))
> +  % (n, p, s))
>  p = p.replace('%%', '%')
>  p = util.expandpath(p)
>  if not util.hasscheme(p) and not os.path.isabs(p):
> @@ -291,7 +292,7 @@ class ui(object):
>  return untrusted and self._ucfg or self._tcfg
>
>  def configsource(self, section, name, untrusted=False):
> -return self._data(untrusted).source(section, name) or 'none'
> +return self._data(untrusted).source(section, name)
>
>  def config(self, section, name, default=None, untrusted=False):
>  if isinstance(name, list):
> diff --git a/tests/test-config.t b/tests/test-config.t
> --- a/tests/test-config.t
> +++ b/tests/test-config.t
> @@ -84,6 +84,32 @@ Test case sensitive configuration
> }
>]
>
> +Test empty config source:
> +
> +  $ cat < emptysource.py
> +  > def reposetup(ui, repo):
> +  > ui.setconfig('empty', 'source', 'value')
> +  > EOF
> +  $ cp .hg/hgrc .hg/hgrc.orig
> +  $ cat <> .hg/hgrc
> +  > [extensions]
> +  > emptysource = `pwd`/emptysource.py
> +  > EOF
> +
> +  $ hg config --debug empty.source
> +  read config from: * (glob)
> +  none: value
> +  $ hg config empty.source -Tjson
> +  [
> +   {
> +"name": "empty.source",
> +"source": "",
> +"value": "value"
> +   }
> +  ]
> +
> +  $ cp .hg/hgrc.orig .hg/hgrc
> +
>  Test "%unset"
>
>$ cat >> $HGRCPATH < ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] convert: remove unused-but-set variable introduced in db9e883566e8

2016-12-19 Thread Augie Fackler
On Sun, Dec 18, 2016 at 04:49:11PM +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara 
> # Date 1482045604 -32400
> #  Sun Dec 18 16:20:04 2016 +0900
> # Node ID 7d505a99f16d0a7faae231432deef5c9bcfd3ad1
> # Parent  07025bd744a09dd5622e16ccef11966fda7b86f2
> convert: remove unused-but-set variable introduced in db9e883566e8

Queued, thanks.

>
> Spotted by pyflakes.
>
> diff --git a/hgext/convert/p4.py b/hgext/convert/p4.py
> --- a/hgext/convert/p4.py
> +++ b/hgext/convert/p4.py
> @@ -302,8 +302,6 @@ class p4_source(common.converter_source)
>  `p4 describe` output
>  """
>  desc = self.recode(obj.get("desc", ""))
> -shortdesc = desc.split("\n", 1)[0]
> -
>  date = (int(obj["time"]), 0) # timezone not set
>  if parents is None:
>  parents = []
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


RE: [PATCH] update: introduce config option ui.allowdirtyupdate for dirty nonlinear updates

2016-12-19 Thread Gábor STEFANIK
>


--
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 
> Cc: Jun Wu ; mercurial-devel  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
>  wrote:
> >
> > -Original Message-
> >> On Thu, Dec 8, 2016 at 9:13 AM, Gábor STEFANIK
> >>  wrote:
> >>>
> >>> -Original Message-
>  From: Jun Wu [mailto:qu...@fb.com]
>  Sent: Wednesday, December 7, 2016 6:44 PM
>  To: Gábor STEFANIK 
>  Cc: mercurial-devel 
>  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  # 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 

Re: [PATCH] push: add a message when pushing phases but not changes

2016-12-19 Thread Pierre-Yves David



On 12/06/2016 06:15 PM, Martin von Zweigbergk via Mercurial-devel wrote:

On Fri, Dec 2, 2016 at 11:12 AM, Jeremy Wall (zaphar)
 wrote:

# HG changeset patch
# User Jeremy Wall (zaphar) 
# Date 1480542942 21600
#  Wed Nov 30 15:55:42 2016 -0600
# Node ID 9cb1540e417dc79a55944adffb691a3ada01571c
# Parent  9e29d4e4e08b5996adda49cdd0b497d89e2b16ee
push: add a message when pushing phases but not changes

This is an attempt to fix
https://bz.mercurial-scm.org/show_bug.cgi?id=4232


Thanks, that "no changes found" has struck me as misleading. Question
for others: will it be considered BC-breaking to change it to e.g. "no
changesets found"?


(actual comment coming on V2)

According to my memory of a years old discussion with Matt, yes the 
output of push and pull are covered by BC but there is still room to 
change it. In particular (still from my memory) an overall rework of 
push/pull output were to be eventually expected since the current output 
is not very useful for normal human being (especially the 
changeset/manisfest/file changes) things.


Still from my memory on that discussion the idea was to avoid to large 
change to existing line until we did this overall rework.


(reminder: that discussion was years ago)


diff -r 9e29d4e4e08b -r 9cb1540e417d mercurial/exchange.py
--- a/mercurial/exchange.py Tue Nov 29 04:11:05 2016 -0800
+++ b/mercurial/exchange.py Wed Nov 30 15:55:42 2016 -0600
@@ -14,6 +14,7 @@
 from .node import (
 hex,
 nullid,
+short,
 )
 from . import (
 base85,
@@ -643,9 +644,20 @@
 def _pushcheckoutgoing(pushop):
 outgoing = pushop.outgoing
 unfi = pushop.repo.unfiltered()
+ui = unfi.ui
 if not outgoing.missing:
-# nothing to push
-scmutil.nochangesfound(unfi.ui, unfi, outgoing.excluded)
+# TODO(jeremy): Question? Should I be worrying about
+# fallbackoutdatedphases here too?
+# phases to push
+if pushop.outdatedphases:
+for outphase in pushop.outdatedphases:
+# TODO(jeremy): Is this the right way to report this?
+ui.status(_("sending phase %s for %s\n") %
+  (outphase.phasestr(), short(outphase.node(
+# TODO(jeremy): Do I still return false?
+else:
+# nothing to push
+scmutil.nochangesfound(ui, unfi, outgoing.excluded)


Instead of displaying the message only when no changesets are pushed,
it seems better to do what Pierre-Yves suggested on the bug: display a
message when we're updating phases on changesets not involved in the
push.

On that topic, but off topic for this patch, I'd also like for the
obsmarker exchange to be mentioned in the output. For changeset
discovery, we say "searching for changes", but for obsmarkers, it's
just a progress bar that says "preparing locally". (Phase and bookmark
discover is usually quick enough that I don't think they need to be
mentioned.)


Actually, obsmarker discover have a full an detailled output hidden 
behind a config knob. And most people do not know about that config knob.


experimental.verbose-obsolescence-exchange=1

 Making this more visible has been on my list for some time but I did 
not came to it yet. (anyone feel free to do it)





 return False
 # something to push
 if not pushop.force:
diff -r 9e29d4e4e08b -r 9cb1540e417d tests/test-phases-exchange.t
--- a/tests/test-phases-exchange.t  Tue Nov 29 04:11:05 2016 -0800
+++ b/tests/test-phases-exchange.t  Wed Nov 30 15:55:42 2016 -0600
@@ -384,7 +384,7 @@
   $ hg push ../alpha # from nu
   pushing to ../alpha
   searching for changes
-  no changes found
+  sending phase public for 145e75495359


I think a summary would better than listing every phase root (?).
There could potentially be many. We usually say something like "added
X changesets with Y changes to Z files (+W heads)" for changesets, so
maybe just "updated X phase boundaries" or something like that, or
maybe "updated phases on X commits", since the user is probably not
expected to know what a phase boundary is.


   [1]
   $ cd ..
   $ cd alpha
@@ -600,7 +600,7 @@
   $ hg push ../alpha
   pushing to ../alpha
   searching for changes
-  no changes found
+  sending phase public for b740e3e5c05d
   [1]
   $ hgph
   o  6 public a-F - b740e3e5c05d
@@ -705,7 +705,7 @@
   $ hg push ../alpha
   pushing to ../alpha
   searching for changes
-  no changes found
+  sending phase draft for 967b449fbc94
   [1]
   $ hgph
   o  9 public a-H - 967b449fbc94
diff -r 9e29d4e4e08b -r 9cb1540e417d tests/test-push-warn.t
--- a/tests/test-push-warn.tTue Nov 29 04:11:05 2016 -0800
+++ b/tests/test-push-warn.tWed Nov 30 15:55:42 2016 -0600
@@ -125,7 +125,7 @@
   $ hg push -r 2 ../c
   pushing to ../c
   searching for changes
-  no changes found
+  sending phase public for d9f42cd1a1ec
   [1]

   $ hg push -r 3 ../c
diff -r 9e29d4e4e08b -r 

Re: [PATCH STABLE] demandimport: do not raise ImportError for unknown item in fromlist

2016-12-19 Thread Pierre-Yves David



On 12/19/2016 04:18 PM, Yuya Nishihara wrote:

# HG changeset patch
# User Yuya Nishihara 
# Date 1482155160 -32400
#  Mon Dec 19 22:46:00 2016 +0900
# Branch stable
# Node ID 58ebfc1af9f290638bed3cc9faf9d6f2deb0fc20
# Parent  7817df5585db1d87d3f6c7f496085c86d87e2e9a
demandimport: do not raise ImportError for unknown item in fromlist

This is the behavior of the default __import__() function, which doesn't
validate the existence of the fromlist items. Later on, the missing attribute
is detected while processing the import statement.

https://hg.python.org/cpython/file/v2.7.13/Python/import.c#l2575


O.o …



The comtypes library relies on this (maybe) undocumented behavior, and we
got a bug report to TortoiseHg, sigh.

https://bitbucket.org/tortoisehg/thg/issues/4647/

The test added at 26a4e46af2bc verifies the behavior of the import statement,
so this patch only adds the test of __import__() function and works around
CPython/PyPy difference.

diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
--- a/mercurial/demandimport.py
+++ b/mercurial/demandimport.py
@@ -199,8 +199,11 @@ def _demandimport(name, globals=None, lo
 nonpkg = getattr(mod, '__path__', nothing) is nothing
 if symbol is nothing:
 if nonpkg:
-# do not try relative import, which would raise ValueError
-raise ImportError('cannot import name %s' % attr)
+# do not try relative import, which would raise ValueError,
+# and leave unknown attribute as the default __import__()
+# would do. the missing attribute will be detected later
+# while processing the import statement.
+return
 mn = '%s.%s' % (mod.__name__, attr)
 if mn in ignore:
 importfunc = _origimport
diff --git a/tests/test-demandimport.py b/tests/test-demandimport.py
--- a/tests/test-demandimport.py
+++ b/tests/test-demandimport.py
@@ -70,7 +70,16 @@ try:
 print('no demandmod should be created for attribute of non-package '
   'module:\ncontextlib.unknownattr =', f(unknownattr))
 except ImportError as inst:
-print('contextlib.unknownattr = ImportError: %s' % inst)
+print('contextlib.unknownattr = ImportError: %s'
+  % rsub(r"'", '', str(inst)))
+
+# Unlike the import statement, __import__() function should not raise
+# ImportError even if fromlist has an unknown item
+# (see Python/import.c:import_module_level() and ensure_fromlist())
+contextlibimp = __import__('contextlib', globals(), locals(), ['unknownattr'])
+print("__import__('contextlib', ..., ['unknownattr']) =", f(contextlibimp))
+print("hasattr(__import__('contextlib'), 'unknownattr') =",
+  util.safehasattr(contextlibimp, 'unknownattr'))

 demandimport.disable()
 os.environ['HGDEMANDIMPORT'] = 'disable'
diff --git a/tests/test-demandimport.py.out b/tests/test-demandimport.py.out
--- a/tests/test-demandimport.py.out
+++ b/tests/test-demandimport.py.out
@@ -18,4 +18,6 @@ re.stderr = ', mod
 re = 
 contextlib = 
 contextlib.unknownattr = ImportError: cannot import name unknownattr
+__import__('contextlib', ..., ['unknownattr']) = 
+hasattr(__import__('contextlib'), 'unknownattr') = False


That message is a bit misleading as we are not testing against
"__import__('contextlib')", but
"__import__('contextlib',…,['unknownattr'])"

So the test code is correct but the test output is a bit confusing. One 
way to clear this would be to drop the '__import__()' call around 
contextlib, but I'm open to other idea to make this clearer (including 
insisting the current form is clear).


It also looks like this while file could be transalted to unittest but 
this might be another adventure.



 node = 


--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH STABLE] demandimport: do not raise ImportError for unknown item in fromlist

2016-12-19 Thread Yuya Nishihara
# HG changeset patch
# User Yuya Nishihara 
# Date 1482155160 -32400
#  Mon Dec 19 22:46:00 2016 +0900
# Branch stable
# Node ID 58ebfc1af9f290638bed3cc9faf9d6f2deb0fc20
# Parent  7817df5585db1d87d3f6c7f496085c86d87e2e9a
demandimport: do not raise ImportError for unknown item in fromlist

This is the behavior of the default __import__() function, which doesn't
validate the existence of the fromlist items. Later on, the missing attribute
is detected while processing the import statement.

https://hg.python.org/cpython/file/v2.7.13/Python/import.c#l2575

The comtypes library relies on this (maybe) undocumented behavior, and we
got a bug report to TortoiseHg, sigh.

https://bitbucket.org/tortoisehg/thg/issues/4647/

The test added at 26a4e46af2bc verifies the behavior of the import statement,
so this patch only adds the test of __import__() function and works around
CPython/PyPy difference.

diff --git a/mercurial/demandimport.py b/mercurial/demandimport.py
--- a/mercurial/demandimport.py
+++ b/mercurial/demandimport.py
@@ -199,8 +199,11 @@ def _demandimport(name, globals=None, lo
 nonpkg = getattr(mod, '__path__', nothing) is nothing
 if symbol is nothing:
 if nonpkg:
-# do not try relative import, which would raise ValueError
-raise ImportError('cannot import name %s' % attr)
+# do not try relative import, which would raise ValueError,
+# and leave unknown attribute as the default __import__()
+# would do. the missing attribute will be detected later
+# while processing the import statement.
+return
 mn = '%s.%s' % (mod.__name__, attr)
 if mn in ignore:
 importfunc = _origimport
diff --git a/tests/test-demandimport.py b/tests/test-demandimport.py
--- a/tests/test-demandimport.py
+++ b/tests/test-demandimport.py
@@ -70,7 +70,16 @@ try:
 print('no demandmod should be created for attribute of non-package '
   'module:\ncontextlib.unknownattr =', f(unknownattr))
 except ImportError as inst:
-print('contextlib.unknownattr = ImportError: %s' % inst)
+print('contextlib.unknownattr = ImportError: %s'
+  % rsub(r"'", '', str(inst)))
+
+# Unlike the import statement, __import__() function should not raise
+# ImportError even if fromlist has an unknown item
+# (see Python/import.c:import_module_level() and ensure_fromlist())
+contextlibimp = __import__('contextlib', globals(), locals(), ['unknownattr'])
+print("__import__('contextlib', ..., ['unknownattr']) =", f(contextlibimp))
+print("hasattr(__import__('contextlib'), 'unknownattr') =",
+  util.safehasattr(contextlibimp, 'unknownattr'))
 
 demandimport.disable()
 os.environ['HGDEMANDIMPORT'] = 'disable'
diff --git a/tests/test-demandimport.py.out b/tests/test-demandimport.py.out
--- a/tests/test-demandimport.py.out
+++ b/tests/test-demandimport.py.out
@@ -18,4 +18,6 @@ re.stderr = ', mod
 re = 
 contextlib = 
 contextlib.unknownattr = ImportError: cannot import name unknownattr
+__import__('contextlib', ..., ['unknownattr']) = 
+hasattr(__import__('contextlib'), 'unknownattr') = False
 node = 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3 V2] summary: add evolution "troubles" information to parents header lines

2016-12-19 Thread Pierre-Yves David



On 12/19/2016 09:28 AM, Denis Laxalde wrote:

Pierre-Yves David a écrit :

On 11/08/2016 03:19 PM, Denis Laxalde wrote:

# HG changeset patch
# User Denis Laxalde 
# Date 1475935828 -7200
#  Sat Oct 08 16:10:28 2016 +0200
# Node ID 35c0f05d694cb9541d60bad9a940cb93a39d615d
# Parent  b5d3d230bbc64d44968a9912e8e72aac8236522a
# EXP-Topic evolve-ui
summary: add evolution "troubles" information to parents header lines

Extend labels of the `parent: ` line according to what `hg log`
displays when
coming from changeset_printer. This would make this line appear the
same in
log and summary with custom colors in particular.

Extend that line with "troubles" information in parentheses, when the
parent
is troubled.


I know there have been objection against that, but I cannot remember
which one and why. Can you dig the mailing list a bit to see if you find
something?


I can remember that nobody was in favor of the "trouble" term despite
it's already used here and there (though not part of any API).


I meant "objection about adding trouble information on that first line, 
alongside the node and tags".



From a non-technical perspective, this term arguably feels unsuitable.
But on the other hand, it reminds me the term "troubleshooting" and its
usage in a technical context; so in this respect, it feels quite
appropriate.

(The funny thing is that this word "trouble" comes from the Old French:
https://en.wiktionary.org/wiki/trouble)

Unless I missed something, the only alternative proposal, from Kevin,
was "evolution:". Quoting his message:

Kevin Bullock:

I'm also not keen on 'troubles'. For this purpose, I'd suggest
'evolution:' by analogy to 'bisect:'. That won't work for referring
to the combined set of {divergences, bumps, ...}, but for labelling
"this changeset's evolution status" I think it works.


I'm fine with this proposal. So if it's ok for you, I can send back the
series with this term and other changes you suggested. Just let me know.


There is interesting bit in that proposal but I'm not entirely 
convinced. See my full response here.


https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-November/090920.html

Let's stick to "trouble" for now, the whole thing is gated behind an 
experimental flag anyway. I saw you sent a V3 that is missing from 
patchwork for an unclear reason (and that I missed myself then). I'll 
look at it soon™



Regarding the form, I would probably drop the "troubles:" part. eg:

 parent: 15:73568ab6879d tip (unstable)

Maybe we could use something else that (), but that seems fine so lets
stay with this.


Good idea.


--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] registrar: raise a programming error on duplicated registering

2016-12-19 Thread Pierre-Yves David



On 12/19/2016 03:09 PM, Yuya Nishihara wrote:

On Mon, 19 Dec 2016 08:51:49 +0100, Pierre-Yves David wrote:

On 12/18/2016 08:24 AM, Yuya Nishihara wrote:

On Fri, 16 Dec 2016 20:31:29 +0100, Pierre-Yves David wrote:

# HG changeset patch
# User Pierre-Yves David 
# Date 1481545965 -3600
#  Mon Dec 12 13:32:45 2016 +0100
# Node ID f3479f74af0d2b21c89e906c503f4cddf4b3c39b
# Parent  182cacaa4c32330c0466b7111a75d060830783e8
registrar: raise a programming error on duplicated registering

Previous, registering different object with the same name would silently
overwrite the first value with the second one. We now detect the situation and
raise an error. No extension in test or core had the issues.

diff --git a/mercurial/registrar.py b/mercurial/registrar.py
--- a/mercurial/registrar.py
+++ b/mercurial/registrar.py
@@ -8,6 +8,7 @@
 from __future__ import absolute_import

 from . import (
+error,
 pycompat,
 util,
 )
@@ -55,6 +56,9 @@ class _funcregistrarbase(object):
 func._origdoc = doc
 func.__doc__ = self._formatdoc(decl, doc)

+if name in self._table:
+raise error.ProgrammingError('duplicate registration for name %s',
+ name)


It should be '% name'.


I'm not sure? We usually have the offending entry at the end because it
has variable length. What do you think?
(I made a minor change in the new version,
'duplicate registration for name: "%s"')


I meant s/,/%/.


Ha right, I've apparently fixed this without paying attention in my 
rework :-)



I slightly prefer raising the error before modifying the func object, but that
would make little difference since ProgrammingError isn't a recoverable error.


Ha!, very good catch. I pushed an updated version here, if you want to
look at/take it.

https://www.mercurial-scm.org/repo/users/marmoute/mercurial/rev/b52e8a4f4c0f


I'll take this, thanks.


Thanks!

--
Pierre-Yves David
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 4 of 4] chgserver: move wrapchgui to runcommand

2016-12-19 Thread Yuya Nishihara
On Sun, 18 Dec 2016 18:24:45 +, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2016-12-18 17:30:31 +0900:
> > On Fri, 16 Dec 2016 15:01:21 +, Jun Wu wrote:
> > > # HG changeset patch
> > > # User Jun Wu 
> > > # Date 1481900282 0
> > > #  Fri Dec 16 14:58:02 2016 +
> > > # Node ID 8fe60192f17f6ae99fa66c6bce1ec306772e31df
> > > # Parent  eb3017f14d56dfdc9870b06a684ef9bcf7a030e6
> > > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > > #  hg pull https://bitbucket.org/quark-zju/hg-draft  -r 
> > > 8fe60192f17f
> > > chgserver: move wrapchgui to runcommand
> > > 
> > > The wrapping logic changes ui.system, which should only affect runcommand.
> > > This makes future refactoring a bit cleaner.
> > > 
> > > diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
> > > --- a/mercurial/chgserver.py
> > > +++ b/mercurial/chgserver.py
> > > @@ -330,5 +330,4 @@ class chgcmdserver(commandserver.server)
> > >  def __init__(self, ui, repo, fin, fout, sock, hashstate, 
> > > baseaddress):
> > >  self._csystem = channeledsystem(fin, fout, 'S')
> > > -_wrapchgui(ui, self._csystem)
> > >  super(chgcmdserver, self).__init__(ui, repo, fin, fout)
> > >  self.clientsock = sock
> > > @@ -507,4 +506,5 @@ class chgcmdserver(commandserver.server)
> > >  
> > >  def runcommand(self):
> > > +_wrapchgui(self.ui, self._csystem)
> > >  return super(chgcmdserver, self).runcommand()
> > 
> > This change doesn't make sense to me. The first patch implies we need a 
> > global
> > ui class having the knowledge about chg. This patch delays the introduction 
> > of
> > the wrapped class, but still it lives in global space. (And you know,
> > theoretically "runcommand" can be called more than once.)
> > 
> > If we have to narrow the scope of csystem-ed ui without recreating it, we'll
> > need a flag (or ui._csystem = None|object) to switch the behavior.
> 
> The direction is to eventually lose control on "ui" used in runcommand, and
> let dispatch construct a "ui" object on its own.

I got it.

> And we then use a special
> "uisetup" which calls "wrapui" to modify the ui object created by dispatch.

Who will call this "uisetup"? dispatch or chgserver.runcommand?

I'm slightly afraid of modifying ui class in the middle of the server session
since the ui might be used after runcommand(). That could lead  to a subtle bug.

> Maybe this patch should be moved after adding "uisetup" in dispatch.request.

If it is a temporary code, I don't care much about the cleanliness.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 2 of 3] changegroup: pass 'repo' to allsupportedversions

2016-12-19 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1482118173 -3600
#  Mon Dec 19 04:29:33 2016 +0100
# Node ID fef4a4a522c7ea405d8a036213223dca79fc3f84
# Parent  a03103bb5979f59d82a5fb8a383cc057bd9ceab6
# EXP-Topic cleanup.changegroup
changegroup: pass 'repo' to allsupportedversions

In the next changesets, we will introduce more logic directly related to the
repository to decide what version have to be supported. So we now directly pass
the repo object instead of just ui.

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -874,23 +874,23 @@ class cg3packer(cg2packer):
  '03': (cg3packer, cg3unpacker),
 }
 
-def allsupportedversions(ui):
+def allsupportedversions(repo):
 versions = set(_packermap.keys())
-if not (ui.configbool('experimental', 'changegroup3') or
-ui.configbool('experimental', 'treemanifest')):
+if not (repo.ui.configbool('experimental', 'changegroup3') or
+repo.ui.configbool('experimental', 'treemanifest')):
 versions.discard('03')
 return versions
 
 # Changegroup versions that can be applied to the repo
 def supportedincomingversions(repo):
-versions = allsupportedversions(repo.ui)
+versions = allsupportedversions(repo)
 if 'treemanifest' in repo.requirements:
 versions.add('03')
 return versions
 
 # Changegroup versions that can be created from the repo
 def supportedoutgoingversions(repo):
-versions = allsupportedversions(repo.ui)
+versions = allsupportedversions(repo)
 if 'treemanifest' in repo.requirements:
 # Versions 01 and 02 support only flat manifests and it's just too
 # expensive to convert between the flat manifest and tree manifest on
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 3 of 3] changegroup: simplify logic around enabling changegroup 03

2016-12-19 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1482117918 -3600
#  Mon Dec 19 04:25:18 2016 +0100
# Node ID e62c766c60a19d89c4e0e50881701135c251fb6f
# Parent  fef4a4a522c7ea405d8a036213223dca79fc3f84
# EXP-Topic cleanup.changegroup
changegroup: simplify logic around enabling changegroup 03

There was multiple spot that took care of adding '03' as supported changegroup
version for different condition. We gather them all in one location for
simplicity.

The 'supportedincomingversions' function is now doing nothing, but I kept it
around because it looks like a great hooking point for extension.

(Note that we should probably just get changegroup3 out of experimental now, But
that would be a patch with a much wider scope).

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -877,16 +877,14 @@ class cg3packer(cg2packer):
 def allsupportedversions(repo):
 versions = set(_packermap.keys())
 if not (repo.ui.configbool('experimental', 'changegroup3') or
-repo.ui.configbool('experimental', 'treemanifest')):
+repo.ui.configbool('experimental', 'treemanifest') or
+'treemanifest' in repo.requirements):
 versions.discard('03')
 return versions
 
 # Changegroup versions that can be applied to the repo
 def supportedincomingversions(repo):
-versions = allsupportedversions(repo)
-if 'treemanifest' in repo.requirements:
-versions.add('03')
-return versions
+return allsupportedversions(repo)
 
 # Changegroup versions that can be created from the repo
 def supportedoutgoingversions(repo):
@@ -899,7 +897,6 @@ def supportedoutgoingversions(repo):
 # support versions 01 and 02.
 versions.discard('01')
 versions.discard('02')
-versions.add('03')
 return versions
 
 def safeversion(repo):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 1 of 3] changegroup: simplify 'allsupportedversions' logic

2016-12-19 Thread Pierre-Yves David
# HG changeset patch
# User Pierre-Yves David 
# Date 1482118273 -3600
#  Mon Dec 19 04:31:13 2016 +0100
# Node ID a03103bb5979f59d82a5fb8a383cc057bd9ceab6
# Parent  07025bd744a09dd5622e16ccef11966fda7b86f2
# EXP-Topic cleanup.changegroup
changegroup: simplify 'allsupportedversions' logic

Discarding '03' to add it back is a bit strange. Instead we only discard it when
needed.

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -876,10 +876,9 @@ class cg3packer(cg2packer):
 
 def allsupportedversions(ui):
 versions = set(_packermap.keys())
-versions.discard('03')
-if (ui.configbool('experimental', 'changegroup3') or
-ui.configbool('experimental', 'treemanifest')):
-versions.add('03')
+if not (ui.configbool('experimental', 'changegroup3') or
+ui.configbool('experimental', 'treemanifest')):
+versions.discard('03')
 return versions
 
 # Changegroup versions that can be applied to the repo
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH] registrar: raise a programming error on duplicated registering

2016-12-19 Thread Yuya Nishihara
On Mon, 19 Dec 2016 08:51:49 +0100, Pierre-Yves David wrote:
> On 12/18/2016 08:24 AM, Yuya Nishihara wrote:
> > On Fri, 16 Dec 2016 20:31:29 +0100, Pierre-Yves David wrote:
> >> # HG changeset patch
> >> # User Pierre-Yves David 
> >> # Date 1481545965 -3600
> >> #  Mon Dec 12 13:32:45 2016 +0100
> >> # Node ID f3479f74af0d2b21c89e906c503f4cddf4b3c39b
> >> # Parent  182cacaa4c32330c0466b7111a75d060830783e8
> >> registrar: raise a programming error on duplicated registering
> >>
> >> Previous, registering different object with the same name would silently
> >> overwrite the first value with the second one. We now detect the situation 
> >> and
> >> raise an error. No extension in test or core had the issues.
> >>
> >> diff --git a/mercurial/registrar.py b/mercurial/registrar.py
> >> --- a/mercurial/registrar.py
> >> +++ b/mercurial/registrar.py
> >> @@ -8,6 +8,7 @@
> >>  from __future__ import absolute_import
> >>
> >>  from . import (
> >> +error,
> >>  pycompat,
> >>  util,
> >>  )
> >> @@ -55,6 +56,9 @@ class _funcregistrarbase(object):
> >>  func._origdoc = doc
> >>  func.__doc__ = self._formatdoc(decl, doc)
> >>
> >> +if name in self._table:
> >> +raise error.ProgrammingError('duplicate registration for name 
> >> %s',
> >> + name)
> >
> > It should be '% name'.
> 
> I'm not sure? We usually have the offending entry at the end because it 
> has variable length. What do you think?
> (I made a minor change in the new version,
> 'duplicate registration for name: "%s"')

I meant s/,/%/.

> > I slightly prefer raising the error before modifying the func object, but 
> > that
> > would make little difference since ProgrammingError isn't a recoverable 
> > error.
> 
> Ha!, very good catch. I pushed an updated version here, if you want to 
> look at/take it.
> 
> https://www.mercurial-scm.org/repo/users/marmoute/mercurial/rev/b52e8a4f4c0f

I'll take this, thanks.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 3 of 3 V2] summary: add evolution "troubles" information to parents header lines

2016-12-19 Thread Denis Laxalde

Pierre-Yves David a écrit :

On 11/08/2016 03:19 PM, Denis Laxalde wrote:

# HG changeset patch
# User Denis Laxalde 
# Date 1475935828 -7200
#  Sat Oct 08 16:10:28 2016 +0200
# Node ID 35c0f05d694cb9541d60bad9a940cb93a39d615d
# Parent  b5d3d230bbc64d44968a9912e8e72aac8236522a
# EXP-Topic evolve-ui
summary: add evolution "troubles" information to parents header lines

Extend labels of the `parent: ` line according to what `hg log`
displays when
coming from changeset_printer. This would make this line appear the
same in
log and summary with custom colors in particular.

Extend that line with "troubles" information in parentheses, when the
parent
is troubled.


I know there have been objection against that, but I cannot remember
which one and why. Can you dig the mailing list a bit to see if you find
something?


I can remember that nobody was in favor of the "trouble" term despite
it's already used here and there (though not part of any API).

From a non-technical perspective, this term arguably feels unsuitable.
But on the other hand, it reminds me the term "troubleshooting" and its
usage in a technical context; so in this respect, it feels quite
appropriate.

(The funny thing is that this word "trouble" comes from the Old French:
https://en.wiktionary.org/wiki/trouble)

Unless I missed something, the only alternative proposal, from Kevin,
was "evolution:". Quoting his message:

Kevin Bullock:

I'm also not keen on 'troubles'. For this purpose, I'd suggest
'evolution:' by analogy to 'bisect:'. That won't work for referring
to the combined set of {divergences, bumps, ...}, but for labelling
"this changeset's evolution status" I think it works.


I'm fine with this proposal. So if it's ok for you, I can send back the
series with this term and other changes you suggested. Just let me know.


Regarding the form, I would probably drop the "troubles:" part. eg:

 parent: 15:73568ab6879d tip (unstable)

Maybe we could use something else that (), but that seems fine so lets
stay with this.


Good idea.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel