Re: [PATCH 1 of 3] style: avoid an unnecessary line split

[Sean Farley]

Denis Laxalde  writes:

> # HG changeset patch
> # User Denis Laxalde 
> # Date 1480061343 -3600
> #  Fri Nov 25 09:09:03 2016 +0100
> # Node ID cde950264b07eb8626e0aa5aa2fbcd6b27f98c10
> # Parent  8836f13e3c5b8eae765372708b659c55a044cbb4
> # EXP-Topic revert/interactive-remove
> style: avoid an unnecessary line split
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -3255,10 +3255,8 @@ def _performrevert(repo, parents, ctx, a
>  audit_path = pathutil.pathauditor(repo.root)
>  for f in actions['forget'][0]:
>  if interactive:
> -choice = \
> -repo.ui.promptchoice(
> -_("forget added file %s (yn)?$$  $$ ")
> -% f)
> +choice = repo.ui.promptchoice(
> +_("forget added file %s (yn)?$$  $$ ") % f)

If we're going to get rid of a line split, I'd prefer to also fix the
indentation:

forget_str = _("forget added file %s (yn)?$$  $$ ")
for f in actions['forget'][0]:
...
choice = repo.ui.promptchoice(forget_str % f)

Might not be worth sending a V2 and asking a commiter to change that
in-flight.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] revlog: ensure that flags do not overflow 2 bytes

[Jun Wu]

Excerpts from Gregory Szorc's message of 2016-11-25 10:59:42 -0800:
> I think having the check in offset_type() to catch all consumers is the
> right place. (Another source of the flag is changegroup data via
> revlog.addgroup() and cg.deltachunk().)
> 
> To clarify, I'm suggesting that instead of truncating "type" like this
> patch is doing, we should raise ValueError in offset_type(). We should
> never pass in a too large number and IMO this warrants an exception. This
> will actively prevent bad data from buggy code being written to a revlog.

+1. I think raising an Exception is better than dropping the bits silently.
It does not require moving the check elsewhere, but just change "&=" to an
if condition and raise an RuntimeError or so.

> On Fri, Nov 25, 2016 at 10:09 AM, Cotizo Sima  wrote:
> 
> > Gregory, I agree, but in this particular case I chose the “deepest” call
> > which insert flags into the revlog such that I cover all the possible call
> > paths. Although to be fair, another good alternative would be
> > revlog._addrevision. I’m happy to move the operation there, if you guys
> > find it more appropriate.
> >
> >
> >
> > *From: *Gregory Szorc 
> > *Date: *Friday, November 25, 2016 at 5:27 PM
> > *To: *Cotizo Sima 
> > *Subject: *Re: [PATCH] revlog: ensure that flags do not overflow 2 bytes
> >
> >
> >
> > On Fri, Nov 25, 2016 at 5:24 AM, Cotizo Sima  wrote:
> >
> > # HG changeset patch
> > # User Cotizo Sima 
> > # Date 1480079985 28800
> > #  Fri Nov 25 05:19:45 2016 -0800
> > # Node ID 4b311b730614941db6409f560ab9451bec74be75
> > # Parent  a3163433647108b7bec8fc45896db1c20b18ab21
> > revlog: ensure that flags do not overflow 2 bytes
> >
> > This patch adds a line that ensures we are not setting by mistake a set of
> > flags
> > overlfowing the 2 bytes they are allocated. Given the way the data is
> > packed in
> > the revlog header, overflowing 2 bytes will result in setting a wrong
> > offset.
> >
> > diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> > --- a/mercurial/revlog.py
> > +++ b/mercurial/revlog.py
> > @@ -72,6 +72,7 @@
> >  return int(q & 0x)
> >
> >  def offset_type(offset, type):
> > +type &= 0x
> >  return long(long(offset) << 16 | type)
> >
> >  _nullhash = hashlib.sha1(nullid)
> >
> >
> >
> > I'm a believer in failing fast. If this new code comes into play, we've
> > already lost to a bug. I think instead we should raise ValueError if type >
> > 65535.
> >
> >
> >
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] revlog: ensure that flags do not overflow 2 bytes

[Gregory Szorc]

I think having the check in offset_type() to catch all consumers is the
right place. (Another source of the flag is changegroup data via
revlog.addgroup() and cg.deltachunk().)

To clarify, I'm suggesting that instead of truncating "type" like this
patch is doing, we should raise ValueError in offset_type(). We should
never pass in a too large number and IMO this warrants an exception. This
will actively prevent bad data from buggy code being written to a revlog.


On Fri, Nov 25, 2016 at 10:09 AM, Cotizo Sima  wrote:

> Gregory, I agree, but in this particular case I chose the “deepest” call
> which insert flags into the revlog such that I cover all the possible call
> paths. Although to be fair, another good alternative would be
> revlog._addrevision. I’m happy to move the operation there, if you guys
> find it more appropriate.
>
>
>
> *From: *Gregory Szorc 
> *Date: *Friday, November 25, 2016 at 5:27 PM
> *To: *Cotizo Sima 
> *Cc: *mercurial-devel 
> *Subject: *Re: [PATCH] revlog: ensure that flags do not overflow 2 bytes
>
>
>
> On Fri, Nov 25, 2016 at 5:24 AM, Cotizo Sima  wrote:
>
> # HG changeset patch
> # User Cotizo Sima 
> # Date 1480079985 28800
> #  Fri Nov 25 05:19:45 2016 -0800
> # Node ID 4b311b730614941db6409f560ab9451bec74be75
> # Parent  a3163433647108b7bec8fc45896db1c20b18ab21
> revlog: ensure that flags do not overflow 2 bytes
>
> This patch adds a line that ensures we are not setting by mistake a set of
> flags
> overlfowing the 2 bytes they are allocated. Given the way the data is
> packed in
> the revlog header, overflowing 2 bytes will result in setting a wrong
> offset.
>
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -72,6 +72,7 @@
>  return int(q & 0x)
>
>  def offset_type(offset, type):
> +type &= 0x
>  return long(long(offset) << 16 | type)
>
>  _nullhash = hashlib.sha1(nullid)
>
>
>
> I'm a believer in failing fast. If this new code comes into play, we've
> already lost to a bug. I think instead we should raise ValueError if type >
> 65535.
>
>
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 2 of 2] debugcommands: sort command order

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc 
# Date 1480096779 28800
#  Fri Nov 25 09:59:39 2016 -0800
# Node ID 8c73c4648bec7c7cf6f1a2ba6b6adca99da55726
# Parent  7b8444ec642e29b1a8cb3e63909d5caf9e4166d7
debugcommands: sort command order

The diff is a bit large but it is straight code moving without any
logical modifications.

diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -68,6 +68,13 @@ def debugancestor(ui, repo, *args):
 a = r.ancestor(lookup(rev1), lookup(rev2))
 ui.write('%d:%s\n' % (r.rev(a), hex(a)))
 
+@command('debugapplystreamclonebundle', [], 'FILE')
+def debugapplystreamclonebundle(ui, repo, fname):
+"""apply a stream clone bundle file"""
+f = hg.openpath(ui, fname)
+gen = exchange.readbundle(ui, f, fname)
+gen.apply(repo)
+
 @command('debugbuilddag',
 [('m', 'mergeable-file', None, _('add single file mergeable changes')),
 ('o', 'overwritten-file', None, _('add single file all revs overwrite')),
@@ -220,24 +227,6 @@ def debugbuilddag(ui, repo, text=None,
 ui.progress(_('building'), None)
 release(tr, lock, wlock)
 
-@command('debugbundle',
-[('a', 'all', None, _('show all details')),
- ('', 'spec', None, _('print the bundlespec of the bundle'))],
-_('FILE'),
-norepo=True)
-def debugbundle(ui, bundlepath, all=None, spec=None, **opts):
-"""lists the contents of a bundle"""
-with hg.openpath(ui, bundlepath) as f:
-if spec:
-spec = exchange.getbundlespec(ui, f)
-ui.write('%s\n' % spec)
-return
-
-gen = exchange.readbundle(ui, f, bundlepath)
-if isinstance(gen, bundle2.unbundle20):
-return _debugbundle2(ui, gen, all=all, **opts)
-_debugchangegroup(ui, gen, all=all, **opts)
-
 def _debugchangegroup(ui, gen, all=None, indent=0, **opts):
 indent_string = ' ' * indent
 if all:
@@ -288,24 +277,23 @@ def _debugbundle2(ui, gen, all=None, **o
 cg = changegroup.getunbundler(version, part, 'UN')
 _debugchangegroup(ui, cg, all=all, indent=4, **opts)
 
-@command('debugcreatestreamclonebundle', [], 'FILE')
-def debugcreatestreamclonebundle(ui, repo, fname):
-"""create a stream clone bundle file
+@command('debugbundle',
+[('a', 'all', None, _('show all details')),
+ ('', 'spec', None, _('print the bundlespec of the bundle'))],
+_('FILE'),
+norepo=True)
+def debugbundle(ui, bundlepath, all=None, spec=None, **opts):
+"""lists the contents of a bundle"""
+with hg.openpath(ui, bundlepath) as f:
+if spec:
+spec = exchange.getbundlespec(ui, f)
+ui.write('%s\n' % spec)
+return
 
-Stream bundles are special bundles that are essentially archives of
-revlog files. They are commonly used for cloning very quickly.
-"""
-requirements, gen = streamclone.generatebundlev1(repo)
-changegroup.writechunks(ui, gen, fname)
-
-ui.write(_('bundle requirements: %s\n') % ', '.join(sorted(requirements)))
-
-@command('debugapplystreamclonebundle', [], 'FILE')
-def debugapplystreamclonebundle(ui, repo, fname):
-"""apply a stream clone bundle file"""
-f = hg.openpath(ui, fname)
-gen = exchange.readbundle(ui, f, fname)
-gen.apply(repo)
+gen = exchange.readbundle(ui, f, bundlepath)
+if isinstance(gen, bundle2.unbundle20):
+return _debugbundle2(ui, gen, all=all, **opts)
+_debugchangegroup(ui, gen, all=all, **opts)
 
 @command('debugcheckstate', [], '')
 def debugcheckstate(ui, repo):
@@ -371,6 +359,18 @@ def debugcomplete(ui, cmd='', **opts):
 cmdlist = [' '.join(c[0]) for c in cmdlist.values()]
 ui.write("%s\n" % "\n".join(sorted(cmdlist)))
 
+@command('debugcreatestreamclonebundle', [], 'FILE')
+def debugcreatestreamclonebundle(ui, repo, fname):
+"""create a stream clone bundle file
+
+Stream bundles are special bundles that are essentially archives of
+revlog files. They are commonly used for cloning very quickly.
+"""
+requirements, gen = streamclone.generatebundlev1(repo)
+changegroup.writechunks(ui, gen, fname)
+
+ui.write(_('bundle requirements: %s\n') % ', '.join(sorted(requirements)))
+
 @command('debugdag',
 [('t', 'tags', None, _('use tags as labels')),
 ('b', 'branches', None, _('annotate with branch names')),
@@ -465,6 +465,107 @@ def debugdate(ui, date, range=None, **op
 m = util.matchdate(range)
 ui.write(("match: %s\n") % m(d[0]))
 
+@command('debugdeltachain',
+commands.debugrevlogopts + commands.formatteropts,
+_('-c|-m|FILE'),
+optionalrepo=True)
+def debugdeltachain(ui, repo, file_=None, **opts):
+"""dump information about delta chains in a revlog
+
+Output can be templatized. Available template keywords are:
+
+:``rev``:   revision number
+

[PATCH 1 of 2] tests: add test that @commands in debugcommands.py are sorted

[Gregory Szorc]

# HG changeset patch
# User Gregory Szorc 
# Date 1480096505 28800
#  Fri Nov 25 09:55:05 2016 -0800
# Node ID 7b8444ec642e29b1a8cb3e63909d5caf9e4166d7
# Parent  906a7d8e969552536fffe0df7a5e63bf5d79b34b
tests: add test that @commands in debugcommands.py are sorted

I felt like inline Python in test-check-code.t was the most
appropriate place for this, as other linters in contrib/ seem to
be source file agnostic.

The test currently fails.

diff --git a/tests/test-check-code.t b/tests/test-check-code.t
--- a/tests/test-check-code.t
+++ b/tests/test-check-code.t
@@ -18,3 +18,20 @@ New errors are not allowed. Warnings are
   Skipping mercurial/httpclient/__init__.py it has no-che?k-code (glob)
   Skipping mercurial/httpclient/_readers.py it has no-che?k-code (glob)
   Skipping mercurial/statprof.py it has no-che?k-code (glob)
+
+@commands in debugcommands.py should be in alphabetical order.
+
+  >>> import re
+  >>> commands = []
+  >>> with open('mercurial/debugcommands.py', 'rb') as fh:
+  ... for line in fh:
+  ... m = re.match("^@command\('([a-z]+)", line)
+  ... if m:
+  ... commands.append(m.group(1))
+  >>> scommands = list(sorted(commands))
+  >>> for i, command in enumerate(scommands):
+  ... if command != commands[i]:
+  ... print('commands in debugcommands.py not sorted; first differing '
+  ...   'command is %s; expected %s' % (commands[i], command))
+  ... break
+  commands in debugcommands.py not sorted; first differing command is 
debugbuilddag; expected debugapplystreamclonebundle
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3 V3] memctx: allow the memlightctx thats reusing the manifest node

[Gregory Szorc]

On Tue, Nov 22, 2016 at 2:18 PM, Jun Wu  wrote:

> Excerpts from Augie Fackler's message of 2016-11-21 18:26:16 -0500:
> > Also, could histedit be updated to use this for 'mess' actions
> > perhaps? Probably not easy, but if it is I'd love to see an in-tree
> > client of this class. Can you take a look?
>
> It does not work if "mess" happens after other csets creating new
> manifests.
>
> Instead of making it a special for histedit. I'm always more interested in
> a general purpose lower-level function doing some kind of "rebase" in core,
> that smartly deals with this case and handles bookmark and dirstate parent
> movements (and obsmarker creation) automatically.
>
> There are currently multiple places reinventing the logic over and over:
>   - absorb / collate
>   - rebase
>   - histedit
>   - evolve
>
> And once we have this API in core, we can migrate the above commands to use
> it.
>
>
This is a good idea. And it has been discussed before! Often in the context
of https://www.mercurial-scm.org/wiki/PlanHistoryRewritePlan.

I once showed Pierre-Yves some code (
https://hg.mozilla.org/hgcustom/version-control-tools/file/6298a2195598/pylib/mozhg/mozhg/rewrite.py)
Mozilla wrote to do generic changeset rewriting. He was receptive to
introducing a rewrite.py or rewriting.py in the core project and
consolidating low-level history rewriting code there. Whether Mozilla's
code could be used - probably not, as it was designed for a very narrow
focus.


> I'm thinking about something like:
>
>   relocate(origctx, newparents, fileoverrides, metaoverrides, obsoleted,
>transaction)
>   # origctx: context
>   # newparents: [node]
>   # fileoverrides: abstract dict, keys: path, value: (content, mode,
> renamed)
>   # metaoverrides: dict with defined set of keys. to override metadata like
>   #commit message etc.
>   # obsoleted: bool, could also be a non-bool to provide more metadata
>
> Then relocate will choose what *ctx to use internally. For absorb,
> metaoverrides is empty, for metaedit, fileoverrides is empty. For rebase,
> both are empty. For histedit, depends.
>
> Two things I'd like to make sure they are considered while developing the
> API:
>
>   1. Giant files friendly - if we need to commit two giant files, we don't
>  need to keep both of them in memory. "fileoverrides" being an abstract
>  dict should solve this.
>   2. Commit hooks friendly - if any commit hook is requiring a working
> copy,
>  fallback to the slow path that writs the disk. (this could be the most
>  complex part of the implementation, but I guess crecord-alike already
>  has similar things). We also need to have ways to mark a commit hook
> as
>  "do not require on-disk working copy" - could be a config option, or
>  checking some magic string in the hook itself.
>
> If we decide to go this way, I could help start some more formal discussion
> (a plan page or so) - hopefully we can collect enough corner cases that
> need support, have a final decision about the API soon.
>
> CC smf because it could be related to your memctx work.
> ___
> 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] revlog: ensure that flags do not overflow 2 bytes

[Gregory Szorc]

On Fri, Nov 25, 2016 at 5:24 AM, Cotizo Sima  wrote:

> # HG changeset patch
> # User Cotizo Sima 
> # Date 1480079985 28800
> #  Fri Nov 25 05:19:45 2016 -0800
> # Node ID 4b311b730614941db6409f560ab9451bec74be75
> # Parent  a3163433647108b7bec8fc45896db1c20b18ab21
> revlog: ensure that flags do not overflow 2 bytes
>
> This patch adds a line that ensures we are not setting by mistake a set of
> flags
> overlfowing the 2 bytes they are allocated. Given the way the data is
> packed in
> the revlog header, overflowing 2 bytes will result in setting a wrong
> offset.
>
> diff --git a/mercurial/revlog.py b/mercurial/revlog.py
> --- a/mercurial/revlog.py
> +++ b/mercurial/revlog.py
> @@ -72,6 +72,7 @@
>  return int(q & 0x)
>
>  def offset_type(offset, type):
> +type &= 0x
>  return long(long(offset) << 16 | type)
>
>  _nullhash = hashlib.sha1(nullid)
>
>
I'm a believer in failing fast. If this new code comes into play, we've
already lost to a bug. I think instead we should raise ValueError if type >
65535.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] fsmonitor: be robust in the face of bad state

[Martijn Pieters]

On 25 November 2016 at 15:30, Simon Farnsworth  wrote:

> # HG changeset patch
> # User Simon Farnsworth 
> # Date 1480087846 28800
> #  Fri Nov 25 07:30:46 2016 -0800
> # Node ID 0ca34f1b83da754246ee33e01c4f7d6652061f5d
> # Parent  a3163433647108b7bec8fc45896db1c20b18ab21
> fsmonitor: be robust in the face of bad state
>
> fsmonitor could write out bad state if interrupted part way through, and
> would then crash when it tried to read it back in.
>
> Make both sides of the operation more robust - reading state should fail
> cleanly, and we can use atomictemp to write out cleanly as the file is
> small. Between the two, we shouldn't crash with an IndexError any more.
>

I stepped through this with Simon; specifically using the atomic file
object as a context manager ensures that it never replaces the old state
file if an exception occurs; the previous version would close the new file
no matter what happened (and so leaving an incomplete file).

I traced what would happen if there are exactly 3 elements (so notefiles
would be an empty list); the code that uses state.get() can set notefiles
to an empty list in various places *anyway*, so the list being empty is a
valid edgecase.

In other words, looks great to me.



> diff --git a/hgext/fsmonitor/state.py b/hgext/fsmonitor/state.py
> --- a/hgext/fsmonitor/state.py
> +++ b/hgext/fsmonitor/state.py
> @@ -59,6 +59,12 @@
>  state = file.read().split('\0')
>  # state = hostname\0clock\0ignorehash\0 + list of files, each
>  # followed by a \0
> +if len(state) < 3:
> +self._ui.log(
> +'fsmonitor', 'fsmonitor: state file truncated
> (expected '
> +'3 chunks, found %d), nuking state\n' % len(state))
> +self.invalidate()
> +return None, None, None
>  diskhostname = state[0]
>  hostname = socket.gethostname()
>  if diskhostname != hostname:
> @@ -85,12 +91,12 @@
>  return
>
>  try:
> -file = self._opener('fsmonitor.state', 'wb')
> +file = self._opener('fsmonitor.state', 'wb', atomictemp=True)
>  except (IOError, OSError):
>  self._ui.warn(_("warning: unable to write out fsmonitor
> state\n"))
>  return
>
> -try:
> +with file:
>  file.write(struct.pack(_versionformat, _version))
>  file.write(socket.gethostname() + '\0')
>  file.write(clock + '\0')
> @@ -98,8 +104,6 @@
>  if notefiles:
>  file.write('\0'.join(notefiles))
>  file.write('\0')
> -finally:
> -file.close()
>
>  def invalidate(self):
>  try:
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>



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

Re: [PATCH] fsmonitor: be robust in the face of bad state

[Mateusz Kwapich]

The logic looks correct. It definitely makes the fsmonitor more robust.

Excerpts from Simon Farnsworth's message of 2016-11-25 07:30:53 -0800:
> # HG changeset patch
> # User Simon Farnsworth 
> # Date 1480087846 28800
> #  Fri Nov 25 07:30:46 2016 -0800
> # Node ID 0ca34f1b83da754246ee33e01c4f7d6652061f5d
> # Parent  a3163433647108b7bec8fc45896db1c20b18ab21
> fsmonitor: be robust in the face of bad state
> 
> fsmonitor could write out bad state if interrupted part way through, and
> would then crash when it tried to read it back in.
> 
> Make both sides of the operation more robust - reading state should fail
> cleanly, and we can use atomictemp to write out cleanly as the file is
> small. Between the two, we shouldn't crash with an IndexError any more.
> 
> diff --git a/hgext/fsmonitor/state.py b/hgext/fsmonitor/state.py
> --- a/hgext/fsmonitor/state.py
> +++ b/hgext/fsmonitor/state.py
> @@ -59,6 +59,12 @@
>  state = file.read().split('\0')
>  # state = hostname\0clock\0ignorehash\0 + list of files, each
>  # followed by a \0
> +if len(state) < 3:
> +self._ui.log(
> +'fsmonitor', 'fsmonitor: state file truncated (expected '
> +'3 chunks, found %d), nuking state\n' % len(state))
> +self.invalidate()
> +return None, None, None
>  diskhostname = state[0]
>  hostname = socket.gethostname()
>  if diskhostname != hostname:
> @@ -85,12 +91,12 @@
>  return
>  
>  try:
> -file = self._opener('fsmonitor.state', 'wb')
> +file = self._opener('fsmonitor.state', 'wb', atomictemp=True)
>  except (IOError, OSError):
>  self._ui.warn(_("warning: unable to write out fsmonitor 
> state\n"))
>  return
>  
> -try:
> +with file:
>  file.write(struct.pack(_versionformat, _version))
>  file.write(socket.gethostname() + '\0')
>  file.write(clock + '\0')
> @@ -98,8 +104,6 @@
>  if notefiles:
>  file.write('\0'.join(notefiles))
>  file.write('\0')
> -finally:
> -file.close()
>  
>  def invalidate(self):
>  try:

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

[PATCH] fsmonitor: be robust in the face of bad state

[Simon Farnsworth]

# HG changeset patch
# User Simon Farnsworth 
# Date 1480087846 28800
#  Fri Nov 25 07:30:46 2016 -0800
# Node ID 0ca34f1b83da754246ee33e01c4f7d6652061f5d
# Parent  a3163433647108b7bec8fc45896db1c20b18ab21
fsmonitor: be robust in the face of bad state

fsmonitor could write out bad state if interrupted part way through, and
would then crash when it tried to read it back in.

Make both sides of the operation more robust - reading state should fail
cleanly, and we can use atomictemp to write out cleanly as the file is
small. Between the two, we shouldn't crash with an IndexError any more.

diff --git a/hgext/fsmonitor/state.py b/hgext/fsmonitor/state.py
--- a/hgext/fsmonitor/state.py
+++ b/hgext/fsmonitor/state.py
@@ -59,6 +59,12 @@
 state = file.read().split('\0')
 # state = hostname\0clock\0ignorehash\0 + list of files, each
 # followed by a \0
+if len(state) < 3:
+self._ui.log(
+'fsmonitor', 'fsmonitor: state file truncated (expected '
+'3 chunks, found %d), nuking state\n' % len(state))
+self.invalidate()
+return None, None, None
 diskhostname = state[0]
 hostname = socket.gethostname()
 if diskhostname != hostname:
@@ -85,12 +91,12 @@
 return
 
 try:
-file = self._opener('fsmonitor.state', 'wb')
+file = self._opener('fsmonitor.state', 'wb', atomictemp=True)
 except (IOError, OSError):
 self._ui.warn(_("warning: unable to write out fsmonitor state\n"))
 return
 
-try:
+with file:
 file.write(struct.pack(_versionformat, _version))
 file.write(socket.gethostname() + '\0')
 file.write(clock + '\0')
@@ -98,8 +104,6 @@
 if notefiles:
 file.write('\0'.join(notefiles))
 file.write('\0')
-finally:
-file.close()
 
 def invalidate(self):
 try:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 7 of 7 v2] bdiff: give slight preference to removing trailing lines

[Jun Wu]

Excerpts from Mads Kiilerich's message of 2016-11-25 15:58:28 +0100:
> On 11/24/2016 06:52 PM, Jun Wu wrote:
> > Excerpts from Augie Fackler's message of 2016-11-17 12:42:26 -0500:
> >> My own cursory perfbdiff runs suggest this is a perf wash (using
> >> `perfbdiff -m 3041e4d59df2` in the mozilla repo). Queued. Thanks!
> > I'd mention this series changes the behavior of the diff output. The
> > difference was caught by fastannotate test.
> >
> > See the below table (old: e1d6aa0e4c3a, new: 8836f13e3c5b):
> >
> > a | b | old | new
> >
> > a | a |  a  | -a
> > a | z | +z  |  a
> > a | a |  a  | +z
> >   |   | -a  |  a
> >
> > a | a | a
> > a | a | a
> > a |   |-a
> >
> > I think we would always prefer putting deletions at the end, to be 
> > consistent.
> 
> 
> I agree that this end result would be nice.
> 
> (My patches "bdiff: early pruning of common suffix before doing 
> expensive computations" and "bdiff: early pruning of common prefix 
> before doing expensive computations" happens to give the obvious "good" 
> result in this case ... but is no general solution to the problem.)

If you have no easy solution for this. I'd suggest ignore it for now and
probably postpone other ideas about bdiff. I also have some "directional"
ideas on diff algrotihm. I'll try to start the discussion in a few weeks.

> Stating the obvious context:
> The algorithms and tweaks for making "good" diffs are just heuristics 
> and can never be perfect. A counter example doesn't necessarily prove 
> anything. But also, it might be worth it to add a tweak...
> 
> An idea, described informally: While the current recursive algorithm 
> works "best" by selecting middle-most longest common substrings, I guess 
> it could make sense to have a post processing step that shifts common 
> substrings to the first occurrence in the previous diff chunk. Also, if 
> a previous or later non-common range is empty (because it is an 
> add/remove), the matching range can be "shifted", perhaps with 
> preference of not starting with an "empty" line but prefering the lowest 
> amount of leading spaces.
> 
> /Mads
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 7 of 7 v2] bdiff: give slight preference to removing trailing lines

[Mads Kiilerich]

On 11/24/2016 06:52 PM, Jun Wu wrote:

Excerpts from Augie Fackler's message of 2016-11-17 12:42:26 -0500:

My own cursory perfbdiff runs suggest this is a perf wash (using
`perfbdiff -m 3041e4d59df2` in the mozilla repo). Queued. Thanks!

I'd mention this series changes the behavior of the diff output. The
difference was caught by fastannotate test.

See the below table (old: e1d6aa0e4c3a, new: 8836f13e3c5b):

a | b | old | new
   
a | a |  a  | -a
a | z | +z  |  a
a | a |  a  | +z
  |   | -a  |  a
   
a | a | a
a | a | a
a |   |-a

I think we would always prefer putting deletions at the end, to be consistent.



I agree that this end result would be nice.

(My patches "bdiff: early pruning of common suffix before doing 
expensive computations" and "bdiff: early pruning of common prefix 
before doing expensive computations" happens to give the obvious "good" 
result in this case ... but is no general solution to the problem.)


Stating the obvious context:
The algorithms and tweaks for making "good" diffs are just heuristics 
and can never be perfect. A counter example doesn't necessarily prove 
anything. But also, it might be worth it to add a tweak...


An idea, described informally: While the current recursive algorithm 
works "best" by selecting middle-most longest common substrings, I guess 
it could make sense to have a post processing step that shifts common 
substrings to the first occurrence in the previous diff chunk. Also, if 
a previous or later non-common range is empty (because it is an 
add/remove), the matching range can be "shifted", perhaps with 
preference of not starting with an "empty" line but prefering the lowest 
amount of leading spaces.


/Mads

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

Re: [PATCH] test-rebase-base: add test cases about multiple branches with merges

[Mateusz Kwapich]

Please, forgive my email client (or my misuse of it). The proper
contents of my last reply are following:

LGTM

Excerpts from Jun Wu's message of 2016-11-25 12:49:01 +:
> # HG changeset patch
> # User Jun Wu 
> # Date 1480077830 0
> #  Fri Nov 25 12:43:50 2016 +
> # Node ID c0a9c4c2c6ae2a779c060dc2424942099d7c984d
> # Parent  fd4175ec0f4e9bd68f4bfdcd601e11d77499d486
> # Available At https://bitbucket.org/quark-zju/hg-draft 
> #  hg pull https://bitbucket.org/quark-zju/hg-draft  -r 
> c0a9c4c2c6ae
> test-rebase-base: add test cases about multiple branches with merges
> 
> This helps clarify the current behavior. When a merge changeset is selected
> in --base directly, only one path will be chosen. The behavior remains the
> same before and after "rebase: calculate ancestors for --base separately
> (issue5420)".
> 
> diff --git a/tests/test-rebase-base.t b/tests/test-rebase-base.t
> --- a/tests/test-rebase-base.t
> +++ b/tests/test-rebase-base.t
> @@ -93,2 +93,169 @@ Mixed rebasable and non-rebasable bases 
>nothing to rebase
>[1]
> +
> +  $ cd ..
> +
> +Multiple branches with merges:
> +
> +  $ hg init b
> +  $ cd b
> +
> +  $ hg debugdrawdag < +  > h   g
> +  > |  /|
> +  > | f |
> +  > |/ /
> +  > | e
> +  > |/  d
> +  > |  /|
> +  > | c |
> +  > |/ /
> +  > | b
> +  > |/
> +  > a
> +  > EOS
> +
> +  $ hg rebase -b b+f -d h
> +  rebasing 1:488e1b7e7341 "b" (b)
> +  rebasing 6:0c088b72e768 "d" (d)
> +  rebasing 4:0e9bbb7dd767 "f" (f)
> +  rebasing 7:9bdc802fd225 "g" (g tip)
> +  saved backup bundle to 
> $TESTTMP/b/.hg/strip-backup/0e9bbb7dd767-ff8b132b-backup.hg (glob)
> +  $ hg tglog
> +  o7: g
> +  |\
> +  | o  6: f
> +  | |
> +  | | o5: d
> +  | | |\
> +  | +---o  4: b
> +  | | |
> +  | o |  3: h
> +  | | |
> +  o | |  2: e
> +  |/ /
> +  | o  1: c
> +  |/
> +  o  0: a
> +  
> +  $ cd ..
> +
> +Multiple branches with multiple merges:
> +
> +  $ hg init c
> +  $ cd c
> +
> +  $ hg debugdrawdag <<'EOS'
> +  > j i
> +  > | |
> +  > | h
> +  > |/|
> +  > |   g | k
> +  > |  /| |/
> +  > | f | |
> +  > |/ /  |
> +  > | e  /
> +  > |/  d
> +  > |  /|
> +  > | c |
> +  > |/ /
> +  > | b
> +  > |/
> +  > a
> +  > EOS
> +  $ hg rebase -b b+f -d j
> +  rebasing 1:488e1b7e7341 "b" (b)
> +  rebasing 4:0e9bbb7dd767 "f" (f)
> +  rebasing 6:0c088b72e768 "d" (d)
> +  rebasing 9:91358dadd39b "k" (k)
> +  rebasing 7:9bdc802fd225 "g" (g)
> +  rebasing 8:91a34cc1f2a7 "h" (h)
> +  rebasing 10:f0d5af4cc88e "i" (i tip)
> +  saved backup bundle to 
> $TESTTMP/c/.hg/strip-backup/0e9bbb7dd767-6f921be4-backup.hg (glob)
> +  $ hg tglog
> +  o  10: i
> +  |
> +  o9: h
> +  |\
> +  | o8: g
> +  | |\
> +  +-o  7: k
> +  | | |
> +  o | |6: d
> +  |\ \ \
> +  | | | o  5: f
> +  | | | |
> +  | o---+  4: b
> +  |  / /
> +  | | o  3: j
> +  | | |
> +  | o |  2: e
> +  | |/
> +  o /  1: c
> +  |/
> +  o  0: a
> +  
> +
> +  $ cd ..
> +
> +Pick merge changesets in -b, only one of the two parents is selected:
> +
> +  $ hg init d
> +  $ cd d
> +
> +  $ hg debugdrawdag <<'EOS'
> +  >   dest  n
> +  > |   |\
> +  > m   |   g d
> +  > |\  |  /|
> +  > | l | f |
> +  >  \ \|/ /
> +  >   k | e
> +  > i  \|/  d
> +  > |\  |  /|
> +  > | h | c |
> +  >  \ \|/ /
> +  >   j | b
> +  >\|/
> +  > a
> +  > EOS
> +
> +  $ hg rebase -b n+i -d dest
> +  rebasing 6:e22eece29d69 "h" (h)
> +  rebasing 12:edd90b885ce0 "i" (i)
> +  rebasing 1:488e1b7e7341 "b" (b)
> +  rebasing 10:0c088b72e768 "d" (d)
> +  rebasing 14:f188fc87af23 "n" (n tip)
> +  saved backup bundle to 
> $TESTTMP/d/.hg/strip-backup/e22eece29d69-0eeca84a-backup.hg (glob)
> +  $ hg tglog
> +  o14: n
> +  |\
> +  | o13: d
> +  | |\
> +  | | o  12: b
> +  | | |
> +  | | | o11: i
> +  | | | |\
> +  | | +---o  10: h
> +  | | | |
> +  | | | | o9: m
> +  | | | | |\
> +  o \ \ \ \ \8: g
> +  |\ \ \ \ \ \
> +  | | | | | | o  7: l
> +  | | | | | | |
> +  | | | | | o |  6: k
> +  | | | | | |/
> +  | | | | o /  5: j
> +  | | | | |/
> +  | o-+  4: f
> +  |  / / /
> +  o-+  3: e
> +   / / /
> +  | o /  2: dest
> +  | |/
> +  o /  1: c
> +  |/
> +  o  0: a
> +  
> +  $ cd ..

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

Re: [PATCH] test-rebase-base: add test cases about multiple branches with merges

[Mateusz Kwapich]

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

mercurial@30449: new changeset

[Mercurial Commits]

New changeset in mercurial:

http://selenic.com/repo/hg//rev/a31634336471
changeset:   30449:a31634336471
bookmark:@
tag: tip
user:Jun Wu 
date:Wed Nov 09 16:01:34 2016 +
summary: drawdag: update test repos by drawing the changelog DAG in ASCII

-- 
Repository URL: http://selenic.com/repo/hg/
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 2 of 2 V2] rebase: calculate ancestors for --base separately (issue5420)

[Jun Wu]

I have added "pathological" cases I can think of in
patchwork.mercurial-scm.org/patch/17754/

I believe the current "--base" implementation is bug-free regarding to merges,
otherwise "-s" will have bugs, too.

Excerpts from Pierre-Yves David's message of 2016-11-23 16:54:51 +0100:
> 
> On 11/19/2016 02:40 AM, Augie Fackler wrote:
> > On Thu, Nov 17, 2016 at 11:49:58PM +, Jun Wu wrote:
> >> # HG changeset patch
> >> # User Jun Wu 
> >> # Date 1479426495 0
> >> #  Thu Nov 17 23:48:15 2016 +
> >> # Node ID e5451a607d1ca53b2446ab049375e5dd5a055bb8
> >> # Parent  87e0edc93d87b88e2925877469d8c142e01737fc
> >> # Available At https://bitbucket.org/quark-zju/hg-draft 
> >> #  hg pull https://bitbucket.org/quark-zju/hg-draft  -r 
> >> e5451a607d1c
> >> rebase: calculate ancestors for --base separately (issue5420)
> >
> > These are queued, thanks.
> 
> I'm a bit concerned about the lack of test covering situation with merge 
> commit. There could be pathological case lurking there. I'll try to a 
> have a deeper look at what this change imply is that case before giving 
> it a second accept stamp.
> 
> >> Previously, the --base option only works with a single "branch" - if there
> >> are multiple branching points, "rebase" will error out with:
> >>
> >>   abort: source is ancestor of destination
> >>
> >> This happens if the user has multiple draft branches with different
> >> branching points, and uses "hg rebase -b 'draft()' -d master". The error
> >> message looks cryptic to users who don't know the implementation detail.
> >>
> >> This patch changes the logic to calculate ancestors for each "base" branch
> >> so it would work in the multiple branching points case.
> >>
> >> Note: if there are multiple bases where some of them are rebasable, while
> >> some of them aren't because the branching point is the destination, the
> >> current behavior is to abort with "nothing to rebase", which seems wrong.
> >> However, that issue is not introduced by this patch - it exists for "-s" as
> >> well. I have reported it as issue5422 and should be solved separately.
> >>
> >> diff --git a/hgext/rebase.py b/hgext/rebase.py
> >> --- a/hgext/rebase.py
> >> +++ b/hgext/rebase.py
> >> @@ -722,10 +722,17 @@ def _definesets(ui, repo, destf=None, sr
> >>  destf = str(dest)
> >>
> >> -commonanc = repo.revs('ancestor(%ld, %d)', base, dest).first()
> >> -if commonanc is not None:
> >> -rebaseset = repo.revs('(%d::(%ld) - %d)::',
> >> -  commonanc, base, commonanc)
> >> -else:
> >> -rebaseset = []
> >> +# calculate ancestors for individual bases
> >> +realbases = []
> >> +for b in repo.revs('roots(%ld)', base):
> >> +# branching point
> >> +bp = repo.revs('ancestor(%d, %d)', b, dest).first()
> >> +if bp is None:
> >> +continue
> >> +# move b to be the direct child of the branching point
> >> +b = repo.revs('%d::%d - %d', bp, b, bp).first()
> >> +if b is not None:
> >> +realbases.append(b)
> >> +
> >> +rebaseset = repo.revs('%ld::', realbases)
> >>
> >>  if not rebaseset:
> >> diff --git a/tests/test-rebase-base.t b/tests/test-rebase-base.t
> >> new file mode 100644
> >> --- /dev/null
> >> +++ b/tests/test-rebase-base.t
> >> @@ -0,0 +1,94 @@
> >> +  $ cat >> $HGRCPATH < >> +  > [extensions]
> >> +  > rebase=
> >> +  > drawdag=$TESTDIR/drawdag.py
> >> +  >
> >> +  > [phases]
> >> +  > publish=False
> >> +  >
> >> +  > [alias]
> >> +  > tglog = log -G --template "{rev}: {desc}"
> >> +  > EOF
> >> +
> >> +  $ hg init a
> >> +  $ cd a
> >> +
> >> +  $ hg debugdrawdag < >> +  > g f
> >> +  > |/
> >> +  > e c d
> >> +  > | |/
> >> +  > | b
> >> +  > |/
> >> +  > a
> >> +  > EOS
> >> +
> >> +  $ cd ..
> >> +
> >> +Pick a single base:
> >> +
> >> +  $ cp -a a a1 && cd a1
> >> +  $ hg rebase -b c -d g -q
> >> +  $ hg tglog
> >> +  o  6: d
> >> +  |
> >> +  | o  5: c
> >> +  |/
> >> +  o  4: b
> >> +  |
> >> +  o  3: g
> >> +  |
> >> +  | o  2: f
> >> +  |/
> >> +  o  1: e
> >> +  |
> >> +  o  0: a
> >> +
> >> +  $ cd ..
> >> +
> >> +Pick a base that is already an descendant of dest:
> >> +
> >> +  $ cp -a a a2 && cd a2
> >> +  $ hg rebase -b g -d e
> >> +  nothing to rebase
> >> +  [1]
> >> +  $ hg rebase -b d -d a
> >> +  nothing to rebase
> >> +  [1]
> >> +  $ hg rebase -b d+c+f+e -d a
> >> +  nothing to rebase
> >> +  [1]
> >> +  $ cd ..
> >> +
> >> +Pick multiple bases (issue5420):
> >> +
> >> +  $ cp -a a a3 && cd a3
> >> +  $ hg rebase -b d+f -d g -q
> >> +  $ hg tglog
> >> +  o  6: f
> >> +  |
> >> +  | o  5: d
> >> +  | |
> >> +  | | o  4: c
> >> +  | |/
> >> +  | o  3: b
> >> +  |/
> >> +  o  2: g
> >> +  |
> >> +  o  1: e
> >> +  |
> >> +  o  0: a
> >> +
> >> +  $ cd ..
> >> +
> >> +Mixed rebasable and non-rebasable bases (unresolved, issue5422):
> >> +
> >> +  $ cp 

Re: [PATCH V2] evolve: improve error message if unstable changes are disallowed

[Mateusz Kwapich]

LGTM, I suppose that timeless may also want to review it before it's
queued.

Excerpts from Pulkit Goyal's message of 2016-11-24 23:13:48 +0530:
> # HG changeset patch
> # User Pulkit Goyal <7895pul...@gmail.com>
> # Date 1479915042 -19800
> #  Wed Nov 23 21:00:42 2016 +0530
> # Node ID 920d5946d13339d9cf4828f678fb55063cd8
> # Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
> evolve: improve error message if unstable changes are disallowed
> 
> I saw a question on stackoverflow why evolve reports something like cannot
> fold chain not ending with head. Even I was confused the first time about the
> behavior. The error message can be improved to avoid confusion to people who
> are unaware about the config in future.
> 
> diff -r cb2bac3253fb -r 920d5946d133 hgext/evolve.py
> --- a/hgext/evolve.pyWed Nov 02 18:56:44 2016 +0100
> +++ b/hgext/evolve.pyWed Nov 23 21:00:42 2016 +0530
> @@ -2514,7 +2514,8 @@
>  raise error.Abort('nothing to prune')
>  
>  if _disallowednewunstable(repo, revs):
> -raise error.Abort(_("cannot prune in the middle of a stack"))
> +raise error.Abort(_("cannot prune in the middle of a stack"),
> +hint = _("new unstable changesets are not allowed"))
>  
>  # defines successors changesets
>  sucs = scmutil.revrange(repo, succs)
> @@ -3234,8 +3235,9 @@
>  newunstable = _disallowednewunstable(repo, revs)
>  if newunstable:
>  raise error.Abort(
> -_('cannot edit commit information in the middle of a 
> stack'),
> -hint=_('%s will be affected') % 
> repo[newunstable.first()])
> +_('cannot edit commit information in the middle of a '\
> +'stack'), hint=_('%s will become unstable and new 
> unstable'\
> +' changes are not allowed') % repo[newunstable.first()])
>  root = head = repo[revs.first()]
>  
>  wctx = repo[None]
> @@ -3299,7 +3301,8 @@
>  head = repo[heads.first()]
>  if _disallowednewunstable(repo, revs):
>  raise error.Abort(_("cannot fold chain not ending with a head "\
> -"or with branching"))
> +"or with branching"), hint = _("new unstable"\
> +" changesets are not allowed"))
>  return root, head
>  
>  def _disallowednewunstable(repo, revs):
> diff -r cb2bac3253fb -r 920d5946d133 tests/test-evolve.t
> --- a/tests/test-evolve.tWed Nov 02 18:56:44 2016 +0100
> +++ b/tests/test-evolve.tWed Nov 23 21:00:42 2016 +0530
> @@ -1301,9 +1301,11 @@
>created new head
>$ hg prune '26 + 27'
>abort: cannot prune in the middle of a stack
> +  (new unstable changesets are not allowed)
>[255]
>$ hg prune '19::28'
>abort: cannot prune in the middle of a stack
> +  (new unstable changesets are not allowed)
>[255]
>$ hg prune '26::'
>3 changesets pruned
> @@ -1338,9 +1340,11 @@
>  
>$ hg fold --exact "19 + 18"
>abort: cannot fold chain not ending with a head or with branching
> +  (new unstable changesets are not allowed)
>[255]
>$ hg fold --exact "18::29"
>abort: cannot fold chain not ending with a head or with branching
> +  (new unstable changesets are not allowed)
>[255]
>$ hg fold --exact "19::"
>2 changesets folded
> @@ -1483,10 +1487,11 @@
>  check that metaedit respects allowunstable
>$ hg metaedit '.^' --config 'experimental.evolution=createmarkers, 
> allnewcommands'
>abort: cannot edit commit information in the middle of a stack
> -  (c904da5245b0 will be affected)
> +  (c904da5245b0 will become unstable and new unstable changes are not 
> allowed)
>[255]
>$ hg metaedit '18::20' --fold --config 
> 'experimental.evolution=createmarkers, allnewcommands'
>abort: cannot fold chain not ending with a head or with branching
> +  (new unstable changesets are not allowed)
>[255]
>$ hg metaedit --user foobar
>0 files updated, 0 files merged, 0 files removed, 0 files unresolved

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

Re: [PATCH 3 of 3 V3] memctx: allow the memlightctx thats reusing the manifest node

[Mateusz Kwapich]

I like the relocate idea. It would be an useful abstractions - we have a
lot of things that are rewriting one commit at a time.

Excerpts from Jun Wu's message of 2016-11-22 22:18:59 +:
> Excerpts from Augie Fackler's message of 2016-11-21 18:26:16 -0500:
> > Also, could histedit be updated to use this for 'mess' actions
> > perhaps? Probably not easy, but if it is I'd love to see an in-tree
> > client of this class. Can you take a look?
> 
> It does not work if "mess" happens after other csets creating new manifests.
> 
> Instead of making it a special for histedit. I'm always more interested in
> a general purpose lower-level function doing some kind of "rebase" in core,
> that smartly deals with this case and handles bookmark and dirstate parent
> movements (and obsmarker creation) automatically.
> 
> There are currently multiple places reinventing the logic over and over:
>   - absorb / collate
>   - rebase
>   - histedit
>   - evolve
> 
> And once we have this API in core, we can migrate the above commands to use
> it.
> 
> I'm thinking about something like:
> 
>   relocate(origctx, newparents, fileoverrides, metaoverrides, obsoleted,
>transaction)
>   # origctx: context
>   # newparents: [node]
>   # fileoverrides: abstract dict, keys: path, value: (content, mode, renamed)
>   # metaoverrides: dict with defined set of keys. to override metadata like
>   #commit message etc. 
>   # obsoleted: bool, could also be a non-bool to provide more metadata
> 
> Then relocate will choose what *ctx to use internally. For absorb,
> metaoverrides is empty, for metaedit, fileoverrides is empty. For rebase,
> both are empty. For histedit, depends.
> 
> Two things I'd like to make sure they are considered while developing the
> API:
>   
>   1. Giant files friendly - if we need to commit two giant files, we don't
>  need to keep both of them in memory. "fileoverrides" being an abstract
>  dict should solve this.
>   2. Commit hooks friendly - if any commit hook is requiring a working copy,
>  fallback to the slow path that writs the disk. (this could be the most
>  complex part of the implementation, but I guess crecord-alike already
>  has similar things). We also need to have ways to mark a commit hook as
>  "do not require on-disk working copy" - could be a config option, or
>  checking some magic string in the hook itself.
> 
> If we decide to go this way, I could help start some more formal discussion
> (a plan page or so) - hopefully we can collect enough corner cases that
> need support, have a final decision about the API soon.
> 
> CC smf because it could be related to your memctx work.

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

[PATCH 1 of 3] style: avoid an unnecessary line split

[Denis Laxalde]

# HG changeset patch
# User Denis Laxalde 
# Date 1480061343 -3600
#  Fri Nov 25 09:09:03 2016 +0100
# Node ID cde950264b07eb8626e0aa5aa2fbcd6b27f98c10
# Parent  8836f13e3c5b8eae765372708b659c55a044cbb4
# EXP-Topic revert/interactive-remove
style: avoid an unnecessary line split

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -3255,10 +3255,8 @@ def _performrevert(repo, parents, ctx, a
 audit_path = pathutil.pathauditor(repo.root)
 for f in actions['forget'][0]:
 if interactive:
-choice = \
-repo.ui.promptchoice(
-_("forget added file %s (yn)?$$  $$ ")
-% f)
+choice = repo.ui.promptchoice(
+_("forget added file %s (yn)?$$  $$ ") % f)
 if choice == 0:
 repo.dirstate.drop(f)
 else:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 2 of 3] revert: indicate the default choice when prompting to forget files

[Denis Laxalde]

# HG changeset patch
# User Denis Laxalde 
# Date 1480061371 -3600
#  Fri Nov 25 09:09:31 2016 +0100
# Node ID 0df7774c49eda9f95f255066cfddae296c13a33a
# Parent  cde950264b07eb8626e0aa5aa2fbcd6b27f98c10
# EXP-Topic revert/interactive-remove
revert: indicate the default choice when prompting to forget files

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -3256,7 +3256,7 @@ def _performrevert(repo, parents, ctx, a
 for f in actions['forget'][0]:
 if interactive:
 choice = repo.ui.promptchoice(
-_("forget added file %s (yn)?$$  $$ ") % f)
+_("forget added file %s (Yn)?$$  $$ ") % f)
 if choice == 0:
 repo.dirstate.drop(f)
 else:
diff --git a/tests/test-revert-interactive.t b/tests/test-revert-interactive.t
--- a/tests/test-revert-interactive.t
+++ b/tests/test-revert-interactive.t
@@ -424,14 +424,14 @@ Check the experimental config to invert 
   > n
   > EOF
   forgetting newfile
-  forget added file newfile (yn)? n
+  forget added file newfile (Yn)? n
   $ hg status
   A newfile
   $ hg revert -i < y
   > EOF
   forgetting newfile
-  forget added file newfile (yn)? y
+  forget added file newfile (Yn)? y
   $ hg status
   ? newfile
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 3 of 3] revert: prompt before removing files in interactive mode

[Denis Laxalde]

# HG changeset patch
# User Denis Laxalde 
# Date 1480061430 -3600
#  Fri Nov 25 09:10:30 2016 +0100
# Node ID ea1c62be7e0782d32ddbee782ace3efee7ccdfdd
# Parent  0df7774c49eda9f95f255066cfddae296c13a33a
# EXP-Topic revert/interactive-remove
revert: prompt before removing files in interactive mode

Prior to this change, files to be removed (i.e. files added since the revision
to revert to) were unconditionally removed despite the interactive mode. Now
prompt before actually removing the files, as this is done for other actions
(e.g. forget).

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -3252,6 +3252,13 @@ def _performrevert(repo, parents, ctx, a
 fc = ctx[f]
 repo.wwrite(f, fc.data(), fc.flags())
 
+def doremove(f):
+try:
+util.unlinkpath(repo.wjoin(f))
+except OSError:
+pass
+repo.dirstate.remove(f)
+
 audit_path = pathutil.pathauditor(repo.root)
 for f in actions['forget'][0]:
 if interactive:
@@ -3265,11 +3272,15 @@ def _performrevert(repo, parents, ctx, a
 repo.dirstate.drop(f)
 for f in actions['remove'][0]:
 audit_path(f)
-try:
-util.unlinkpath(repo.wjoin(f))
-except OSError:
-pass
-repo.dirstate.remove(f)
+if interactive:
+choice = repo.ui.promptchoice(
+_("remove added file %s (Yn)?$$  $$ ") % f)
+if choice == 0:
+doremove(f)
+else:
+excluded_files.append(repo.wjoin(f))
+else:
+doremove(f)
 for f in actions['drop'][0]:
 audit_path(f)
 repo.dirstate.remove(f)
diff --git a/tests/test-revert-interactive.t b/tests/test-revert-interactive.t
--- a/tests/test-revert-interactive.t
+++ b/tests/test-revert-interactive.t
@@ -46,6 +46,7 @@ 10 run the same test than 8 from within 
   > y
   > y
   > y
+  > y
   > n
   > n
   > EOF
@@ -53,6 +54,7 @@ 10 run the same test than 8 from within 
   reverting folder1/g (glob)
   removing folder1/i (glob)
   reverting folder2/h (glob)
+  remove added file folder1/i (Yn)? y
   diff --git a/f b/f
   2 hunks, 2 lines changed
   examine changes to 'f'? [Ynesfdaq?] y
@@ -174,6 +176,7 @@ Test --no-backup
   $ hg update -C 6
   3 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg revert -i -r 2 --all -- << EOF
+  > n
   > y
   > y
   > y
@@ -186,6 +189,7 @@ Test --no-backup
   reverting folder1/g (glob)
   removing folder1/i (glob)
   reverting folder2/h (glob)
+  remove added file folder1/i (Yn)? n
   diff --git a/f b/f
   2 hunks, 2 lines changed
   examine changes to 'f'? [Ynesfdaq?] y
@@ -258,7 +262,6 @@ Test --no-backup
   $ hg st
   M f
   M folder1/g
-  R folder1/i
   $ hg revert --interactive f << EOF
   > y
   > y
@@ -290,7 +293,6 @@ Test --no-backup
   $ hg st
   M f
   M folder1/g
-  R folder1/i
   ? f.orig
   $ cat f
   a
@@ -307,7 +309,7 @@ Test --no-backup
   5
   $ rm f.orig
   $ hg update -C .
-  3 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
 
 Check editing files newly added by a revert
 
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel