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

2016-12-18 Thread Pierre-Yves David



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 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


 self._table[name] = func
 self._extrasetup(name, func, *args, **kwargs)


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


Re: buildbot config change idea - move test-check-*.t to their own target

2016-12-18 Thread Pierre-Yves David



On 12/16/2016 02:45 AM, Augie Fackler wrote:

Right now test-check-commit.t et al can mark all the builds as broken, even 
when it’s just a commit that we’ve decided to take despite some sanity 
checker’s objections. Usually it’s around vendored code, though today it’s 
around my unittest-ification of test-bdiff.py.

Given that these results are fairly stable across platforms, and they’re not 
platform-specific, how would people feel about moving those builds to a 
separate builder so we can see “lint results” separately from “correctness 
results”?

If nobody is opposed, I’ll work on the buildbot config change with Kevin next 
week.


It is true that check-commit complaining about thing we don't care a bit 
too often. I'm really not thrilled at having a new builder target 
intended to be broken on a regular basis. Instead, we should fix the 
problem rather than the symptoms. I would rather have a way to mark 
commit as "skipped-by-check-commit" and the tests to stay green.


For example, we having test-check-commit skip commit with a description
containing "#nocheckcommit" is simple to have.


The idea of an independent target has some other merits, running all the 
tests on the builder is quite long (1h30), having a small target with 
just to test-check-*.t can give use feedback in less than one minutes. 
But I'm not sure it is worth putting effort in that since I expect all 
reviewers to run the tests before they push so that would not be too 
useful (sometime people forgot, but it is rare and sometime pyflakes is 
not around, but they will install it once). In all case we should 
implement check-commit exception first to keep such target happy and 
passing. (And we should keep then in plateform specific tests).


Cheers,

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


Re: [PATCH 1 of 4 v5] revlog: add ability to apply transforms on flags

2016-12-18 Thread Pierre-Yves David



On 12/17/2016 09:09 AM, Pierre-Yves David wrote:



On 12/14/2016 12:58 PM, Remi Chaintron wrote:

# HG changeset patch
# User Remi Chaintron 
# Date 1481716603 0
#  Wed Dec 14 11:56:43 2016 +
# Branch stable
# Node ID 7599256f2800bbacc3b478ac8d2c4a5746eb4cd1
# Parent  d34c04da52a8f2eb61b51bb0aa9cedf1e4b43474
revlog: add ability to apply transforms on flags

Add a mechanism for extensions/subclasses of revlog to register
transforms on
read/write operations on revlog, based on revision flags.

Due to some operations being non-commutative, transforms applied in a
specific
order when writing the contents of the revlog need to be applied on
the reverse
order when reading.
In order to allow the composition of such operation, the
'processflags' method
applies transforms on flags in the order defined by REVIDX_FLAGS_ORDER.

Extensions can use the `localrepository.registerflagtransform()`
method to
register default transforms on flags in `reposetup()` in order to
ensure their
transform will not override/be overridden by other extensions'
transforms.


As a general principle, we don't add new methods on localrepo. Its
tempting to add stuff to that central object but this lead to horrible
horrible class bloat in the past. In addition, your method does not
seems to make any use of the repo itself. (if we did not wrote that down
somewhere, we should)

Instead, you should go for one of these two options:
- a small function defined next to the dictionary
- our shiny and nice "registrar" mechanism. See
https://www.mercurial-scm.org/repo/hg/file/tip/hgext/rebase.py#l109 for
an example.

I would probably pick registrar because the code exists, but given the
low amount of registrees it might be overkill.

Couple of feedback inlined



diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
--- a/mercurial/bundlerepo.py
+++ b/mercurial/bundlerepo.py
@@ -147,7 +147,9 @@
 delta = self._chunk(chain.pop())
 text = mdiff.patches(text, [delta])

-self.checkhash(text, node, rev=rev)
+text, validatehash = self.processflags(text, self.flags(rev))
+if validatehash is True:


Should be:

  if validatehash:


+self.checkhash(text, node, rev=rev)
 self._cache = (node, rev, text)
 return text

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -48,6 +48,7 @@
 phases,
 pushkey,
 repoview,
+revlog,
 revset,
 scmutil,
 store,
@@ -1959,6 +1960,21 @@
 fp.close()
 return self.pathto(fp.name[len(self.root) + 1:])

+def registerflagtransform(self, flag, transform):
+"""Register transform on revlog flags.
+
+Extensions can rely on this method to set default transforms
on revlog
+flags, and ensure they are not overwritten by/overwriting other
+extensions'.
+"""
+if not flag & revlog.REVIDX_KNOWN_FLAGS:
+raise error.Abort(_("Cannot register transform on unknown
flag."))


note that we neither capitalize nor punctuate our error messages.


+transform = revlog.REVIDX_FLAGS_TRANSFORMS.get(flag, None)
+if transform is not None:
+raise error.Abort(
+_("Cannot register multiple transforms on flags."))


ditto


+revlog.REVIDX_FLAGS_TRANSFORMS[flag] = transform
+
 # used to avoid circular references so destructors work
 def aftertrans(files):
 renamefiles = [tuple(t) for t in files]
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -56,6 +56,12 @@
 REVIDX_ISCENSORED = (1 << 15) # revision has censor metadata, must be
verified
 REVIDX_DEFAULT_FLAGS = 0
 REVIDX_KNOWN_FLAGS = REVIDX_ISCENSORED
+# stable order in which flags need to be processed and their
transforms applied
+REVIDX_FLAGS_ORDER = [
+REVIDX_ISCENSORED,
+]
+REVIDX_FLAGS_TRANSFORMS = { }


I've just tough of something else, those are not constants, because they 
are designed for other extension to be able to extend them (especially 
the transform bits). Can you move them to our standard (lowercase, no '_')


(also, our constant are usually lowercase anyway, but this file is 
inconsistency for the rest of the code base)



Small nits: as we have no "public" accessors of this, we could add a '_'
in front of it to discourage people to access it directly.


+

 # max size of revlog with inline data
 _maxinline = 131072
@@ -1203,11 +1209,6 @@
 if rev is None:
 rev = self.rev(node)

-# check rev flags
-if self.flags(rev) & ~REVIDX_KNOWN_FLAGS:
-raise RevlogError(_('incompatible revision flag %x') %
-  (self.flags(rev) & ~REVIDX_KNOWN_FLAGS))
-
 chain, stopped = self._deltachain(rev, stoprev=cachedrev)
 if stopped:
 text = self._cache[2]
@@ -1221,7 +1222,11 @@
 bins = bins[1:]

 

Re: [PATCH 4 of 4 v5] changegroup: enable version 03 on 'lfs' requirement

2016-12-18 Thread Pierre-Yves David



On 12/16/2016 09:46 PM, Augie Fackler wrote:

On Wed, Dec 14, 2016 at 11:58:10AM +, Remi Chaintron wrote:

# HG changeset patch
# User Remi Chaintron 
# Date 1481639041 0
#  Tue Dec 13 14:24:01 2016 +
# Branch stable
# Node ID 8120778471f01b41d01f8d13b4176b32d2b8482c
# Parent  217f7db4dae3309bec78e27c85cc90e924b109be
changegroup: enable version 03 on 'lfs' requirement


This also needs to disable 01 and 02 if it spots lfs (or treemanifests
for that matter).


TLDR; there is issue, but not were we though they were. However, we can 
just drop that patch and have 'lfs' set 'experimental.changegroup3' in 
'reposetup'




Looks at bit deeper, the change regarding changegroup version is 
actually fine¹. The part that only adds '03' is 
'supportedincomingversions'. receiving '01' and '02' version is fine as 
long as their don't contains 'lfs' entries. But the peer generating the 
bundle is responsible to ensure it emit correct ones.


The second side of the coin 'supportedoutgoingversions' is already 
discarding '01' and '02' and is re-used for on disk bundle.


(to be honest, there is some confusing bit in the core implementation 
here, I've sent a small series to clarify that)



However, I've found another problematic bug in last part of the 
changesets. adding 'lfs' to "supportedformats" will make core pretend it 
can handle 'lfs' respository while it can't. Only the 'lfs' extension can.


Overall having to mention 'lfs' anywhere in core look suspicious and I 
don't think it is necessary. We already have "experimental.changegroup3" 
config. The 'lfs' extensions just need to set it to True at reposetup time.




(also, it would be nice if someone (ideally from Google) could look into 
making cg3 enable by default).


(And ideally, we would have a low level change in changegroup building 
that make sure we are not bundling anything with a flag in a changegroup 
version that don't support it. )



Version 03 is required by the `lfs` extension in order to send flags for
revlog objects over the wire.

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -879,14 +879,16 @@
 # Changegroup versions that can be applied to the repo
 def supportedincomingversions(repo):
 versions = allsupportedversions(repo.ui)
-if 'treemanifest' in repo.requirements:
+if ('treemanifest' in repo.requirements or
+'lfs' in repo.requirements):
 versions.add('03')
 return versions

 # Changegroup versions that can be created from the repo
 def supportedoutgoingversions(repo):
 versions = allsupportedversions(repo.ui)
-if 'treemanifest' in repo.requirements:
+if ('treemanifest' in repo.requirements or
+'lfs' 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
 # the fly. Since tree manifests are hashed differently, all of history
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -239,7 +239,7 @@
 class localrepository(object):

 supportedformats = set(('revlogv1', 'generaldelta', 'treemanifest',
-'manifestv2'))
+'manifestv2', 'lfs'))
 _basesupported = supportedformats | set(('store', 'fncache', 'shared',
  'dotencode'))
 openerreqs = set(('revlogv1', 'generaldelta', 'treemanifest', 
'manifestv2'))


This is the bit of code I was talking about earlier. If I understand 
this correctly, this new code seems to indicate core support 'lfs' 
natively, which is not the case.


So this code will let a vanillia mercurial open 'lfs' repository without 
actual 'lfs' support.


Cheers,

--
Pierre-Yves David

[1] mea culpa for making the initial flagging as that "error" in an IRC 
conversation with Augie. Sorry about that false positive.

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


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

2016-12-18 Thread Jun Wu
Excerpts from timeless's message of 2016-12-18 23:52:05 -0500:
> Jun Wu wrote:
> > Pid namespace breaks mercurial lock, which is a symlink with the content
> > "host:pid".
> 
> Would adjusting the format to:
> host:pid:starttime:boottime fix this?

No. A pid number in a pid namespace is highly likely to be invalid in
another pid ns. So hg just thinks the process who held the lock is dead,
and ignores the lock.

> 
> Sadly, it seems like process start time is actually stored as jiffies
> since system boot on Linux, and NTP can make canonicalizing that value
> /somewhat/ flaky [1]. Hopefully, there'd be some way to make this work
> 
> [1] 
> http://linuxcommando.blogspot.ca/2008/09/how-to-get-process-start-date-and-time.html
>  
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


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

2016-12-18 Thread timeless
Jun Wu wrote:
> Pid namespace breaks mercurial lock, which is a symlink with the content
> "host:pid".

Would adjusting the format to:
host:pid:starttime:boottime fix this?

Sadly, it seems like process start time is actually stored as jiffies
since system boot on Linux, and NTP can make canonicalizing that value
/somewhat/ flaky [1]. Hopefully, there'd be some way to make this work

[1] 
http://linuxcommando.blogspot.ca/2008/09/how-to-get-process-start-date-and-time.html
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[Bug 5452] New: "Interrupted system call" with sshpeer.py when resizing terminal

2016-12-18 Thread mercurial-bugs
https://bz.mercurial-scm.org/show_bug.cgi?id=5452

Bug ID: 5452
   Summary: "Interrupted system call" with sshpeer.py when
resizing terminal
   Product: Mercurial
   Version: 4.0.1
  Hardware: PC
OS: Linux
Status: UNCONFIRMED
  Severity: feature
  Priority: wish
 Component: Mercurial
  Assignee: bugzi...@mercurial-scm.org
  Reporter: ar...@tunes.org
CC: mercurial-de...@selenic.com

Running Mercurial across ssh, at least on Linux: when it is waiting for more
data (sshpeer.py:84, "act = util.poll(fds)"), if the terminal window is
resized, then the select call is interrupted with "select.error: (4,
'Interrupted system call')".  The problem is that the error propagates and
crashes the whole mercurial run with a standard traceback.  Instead it should
be caught and ignored.

The problem can be reproduced by running "hg pull ssh://..." or "hg push
ssh://..." and resizing the terminal window while the operation is in progress.
 I guess that the same problem would occur by sending other Posix signals to
the application, but signals are rare nowadays.  I think that resizing the
terminal window sends SIGWINCH to the process, which is normally ignored, but
still interrupts some system calls like select().

It doesn't seem to occur for https:// connections.  I guess they are handled
differently.

Ran with Python 2.7.12 on Arch Linux.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 8 of 8 upgraderepo V3] repair: clean up stale lock file from store backup

2016-12-18 Thread Gregory Szorc
# HG changeset patch
# User Gregory Szorc 
# Date 1480041929 28800
#  Thu Nov 24 18:45:29 2016 -0800
# Node ID 1efcda54217b590bd076bffaaf18a98fe9096dc7
# Parent  96b6f747358116643b039d9c91157cde27106662
repair: clean up stale lock file from store backup

Since we did a directory rename on the stores, the source
repository's lock path now references the dest repository's
lock path and the dest repository's lock path now references
a non-existent filename.

So releasing the lock on the source will unlock the dest and
releasing the lock on the dest will no-op because it fails due
to file not found. So we clean up the dest's lock manually.

diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -920,6 +920,12 @@ def _upgraderepo(ui, srcrepo, dstrepo, r
'again\n'))
 scmutil.writerequires(srcrepo.vfs, requirements)
 
+# The lock file from the old store won't be removed because nothing has a
+# reference to its new location. So clean it up manually. Alternatively, we
+# could update srcrepo.svfs and other variables to point to the new
+# location. This is simpler.
+backupvfs.unlink('store/lock')
+
 return backuppath
 
 def upgraderepo(ui, repo, run=False, optimize=None):
diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t
--- a/tests/test-upgrade-repo.t
+++ b/tests/test-upgrade-repo.t
@@ -303,7 +303,6 @@ old store should be backed up
   00manifest.i
   data
   fncache
-  lock
   phaseroots
   undo
   undo.backup.fncache
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 3 of 8 upgraderepo V3] repair: determine what upgrade will do

2016-12-18 Thread Gregory Szorc
# HG changeset patch
# User Gregory Szorc 
# Date 1482108669 28800
#  Sun Dec 18 16:51:09 2016 -0800
# Node ID d38d8742be98cde337410be7703822eb4412f3d1
# Parent  8c7cd8a43f9e9b230bc125e57b80de73eb0649b5
repair: determine what upgrade will do

This commit introduces code for determining what actions/improvements
an upgrade should perform.

The "upgradefindimprovements" function introduces a mechanism to
return a list of improvements that can be made to a repository.
Each improvement is effectively an action that an upgrade will
perform. Associated with each of these improvements is metadata
that will be used to inform users what's wrong and what an
upgrade will do.

Each "improvement" is categorized as a "deficiency" or an
"optimization." TBH, I'm not thrilled about the terminology and
am receptive to constructive bikeshedding. The main difference
between a "deficiency" and an "optimization" is a deficiency
is always corrected (if it deviates from the current config) and
an "optimization" is an optional action that goes above and beyond
to improve the state of the repository (usually by requiring more
CPU during upgrade).

Our initial set of improvements identifies missing repository
requirements, a single, easily correctable problem with
changelog storage, and a set of "optimizations" related to delta
recalculation.

The main "upgraderepo" function has been expanded to handle
improvements. It queries for the list of improvements and determines
which of them will run based on the current repository state and user

I went through numerous iterations of the output format before
settling on a ReST-inspired definition list format. (I used
bulleted lists in the first submission of this commit and could
not get it to format just right.) Even with the various iterations,
I'm still not super thrilled with the format. But, this is a debug*
command, so that should mean we can refine the output without BC
concerns.

diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -433,11 +433,218 @@ def upgradeallowednewrequirements(repo):
 'generaldelta',
 ])
 
+DEFICIENCY = 'deficiency'
+OPTIMIZATION = 'optimization'
+
+class upgradeimprovement(object):
+"""Represents an improvement that can be made as part of an upgrade.
+
+The following attributes are defined on each instance:
+
+name
+   Machine-readable string uniquely identifying this improvement. It
+   will be mapped to an action later in the upgrade process.
+
+type
+   Either ``DEFICIENCY`` or ``OPTIMIZATION``. A deficiency is an obvious
+   problem. An optimization is an action (sometimes optional) that
+   can be taken to further improve the state of the repository.
+
+description
+   Message intended for humans explaining the improvement in more detail,
+   including the implications of it. For ``DEFICIENCY`` types, should be
+   worded in the present tense. For ``OPTIMIZATION`` types, should be
+   worded in the future tense.
+
+upgrademessage
+   Message intended for humans explaining what an upgrade addressing this
+   issue will do. Should be worded in the future tense.
+
+fromdefault (``DEFICIENCY`` types only)
+   Boolean indicating whether the current (deficient) state deviates
+   from Mercurial's default configuration.
+
+fromconfig (``DEFICIENCY`` types only)
+   Boolean indicating whether the current (deficient) state deviates
+   from the current Mercurial configuration.
+"""
+def __init__(self, name, type, description, upgrademessage, **kwargs):
+self.name = name
+self.type = type
+self.description = description
+self.upgrademessage = upgrademessage
+
+for k, v in kwargs.items():
+setattr(self, k, v)
+
+def upgradefindimprovements(repo):
+"""Determine improvements that can be made to the repo during upgrade.
+
+Returns a list of ``upgradeimprovement`` describing repository deficiencies
+and optimizations.
+"""
+# Avoid cycle: cmdutil -> repair -> localrepo -> cmdutil
+from . import localrepo
+
+newreporeqs = localrepo.newreporequirements(repo)
+
+improvements = []
+
+# We could detect lack of revlogv1 and store here, but they were added
+# in 0.9.2 and we don't support upgrading repos without these
+# requirements, so let's not bother.
+
+if 'fncache' not in repo.requirements:
+improvements.append(upgradeimprovement(
+name='fncache',
+type=DEFICIENCY,
+description=_('long and reserved filenames may not work correctly; 
'
+  'repository performance is sub-optimal'),
+upgrademessage=_('repository will be more resilient to storing '
+ 'certain paths and performance of certain '
+ 'operations should be improved'),
+

[PATCH 7 of 8 upgraderepo V3] repair: copy non-revlog store files during upgrade

2016-12-18 Thread Gregory Szorc
# HG changeset patch
# User Gregory Szorc 
# Date 1480041290 28800
#  Thu Nov 24 18:34:50 2016 -0800
# Node ID 96b6f747358116643b039d9c91157cde27106662
# Parent  01fe6742b8df6a5d832096291267366cf5ae9da2
repair: copy non-revlog store files during upgrade

The store contains more than just revlogs. This patch teaches the
upgrade code to copy regular files as well.

As the test changes demonstrate, the phaseroots file is now copied.

diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -10,6 +10,7 @@ from __future__ import absolute_import
 
 import errno
 import hashlib
+import stat
 import tempfile
 import time
 
@@ -801,6 +802,44 @@ def _copyrevlogs(ui, srcrepo, dstrepo, t
 ui.write(_('finished migrating %d total revisions; total change in store '
'size: %s\n') % (revcount, util.bytecount(dstsize - srcsize)))
 
+def _upgradefilterstorefile(srcrepo, dstrepo, requirements, path, mode, st):
+"""Determine whether to copy a store file during upgrade.
+
+This function is called when migrating store files from ``srcrepo`` to
+``dstrepo`` as part of upgrading a repository.
+
+Args:
+  srcrepo: repo we are copying from
+  dstrepo: repo we are copying to
+  requirements: set of requirements for ``dstrepo``
+  path: store file being examined
+  mode: the ``ST_MODE`` file type of ``path``
+  st: ``stat`` data structure for ``path``
+
+Function should return ``True`` if the file is to be copied.
+"""
+# Skip revlogs.
+if path.endswith(('.i', '.d')):
+return False
+# Skip transaction related files.
+if path.startswith('undo'):
+return False
+# Only copy regular files.
+if mode != stat.S_IFREG:
+return False
+# Skip other skipped files.
+if path in ('lock', 'fncache'):
+return False
+
+return True
+
+def _upgradefinishdatamigration(ui, srcrepo, dstrepo, requirements):
+"""Hook point for extensions to perform additional actions during upgrade.
+
+This function is called after revlogs and store files have been copied but
+before the new store is swapped into the original location.
+"""
+
 def _upgraderepo(ui, srcrepo, dstrepo, requirements, actions):
 """Do the low-level work of upgrading a repository.
 
@@ -830,7 +869,18 @@ def _upgraderepo(ui, srcrepo, dstrepo, r
 _copyrevlogs(ui, srcrepo, dstrepo, tr, deltareuse,
  'redeltamultibase' in actions)
 
-# TODO copy non-revlog store files
+# Now copy other files in the store directory.
+for p, kind, st in srcrepo.store.vfs.readdir('', stat=True):
+if not _upgradefilterstorefile(srcrepo, dstrepo, requirements,
+   p, kind, st):
+continue
+
+srcrepo.ui.write(_('copying %s\n') % p)
+src = srcrepo.store.vfs.join(p)
+dst = dstrepo.store.vfs.join(p)
+util.copyfile(src, dst, copystat=True)
+
+_upgradefinishdatamigration(ui, srcrepo, dstrepo, requirements)
 
 ui.write(_('data fully migrated to temporary repository\n'))
 
diff --git a/tests/test-upgrade-repo.t b/tests/test-upgrade-repo.t
--- a/tests/test-upgrade-repo.t
+++ b/tests/test-upgrade-repo.t
@@ -241,6 +241,7 @@ Upgrading a repository to generaldelta w
   migrating changelog containing 3 revisions (184 bytes in store; 181 bytes 
tracked data)
   finished migrating 3 changelog revisions; change in size: 0 bytes
   finished migrating 9 total revisions; total change in store size: 0 bytes
+  copying phaseroots
   data fully migrated to temporary repository
   marking source repository as being upgraded; clients will be unable to read 
from repository
   starting in-place swap of repository data
@@ -276,6 +277,7 @@ store directory has files we expect
   00manifest.i
   data
   fncache
+  phaseroots
   undo
   undo.backupfiles
   undo.phaseroots
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 4 of 8 upgraderepo V3] repair: begin implementation of in-place upgrading

2016-12-18 Thread Gregory Szorc
# HG changeset patch
# User Gregory Szorc 
# Date 1482109144 28800
#  Sun Dec 18 16:59:04 2016 -0800
# Node ID dce3bdbc3fafa7d7c4c6500fbe2bbdab1fdc2b19
# Parent  d38d8742be98cde337410be7703822eb4412f3d1
repair: begin implementation of in-place upgrading

Now that all the upgrade planning work is in place, we can start
doing the real work: actually upgrading a repository.

The main goal of this commit is to get the "framework" for running
in-place upgrade actions in place.

Rather than get too clever and low-level with regards to in-place
upgrades, our strategy is to create a new, temporary repository,
copy data to it, then replace the old data with the new. This allows
us to reuse a lot of code in localrepo.py around store interaction,
which will eventually consume the bulk of the upgrade code.

But we have to start small. This patch implements adding new
repository requirements. But it still sets up a temporary
repository and locks it and the source repo before performing the
requirements file swap. This means all the plumbing is in place
to implement store copying in subsequent commits.

diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -10,6 +10,7 @@ from __future__ import absolute_import
 
 import errno
 import hashlib
+import tempfile
 
 from .i18n import _
 from .node import short
@@ -19,6 +20,7 @@ from . import (
 error,
 exchange,
 obsolete,
+scmutil,
 util,
 )
 
@@ -639,6 +641,50 @@ def upgradedetermineactions(repo, improv
 
 return newactions
 
+def _upgraderepo(ui, srcrepo, dstrepo, requirements, actions):
+"""Do the low-level work of upgrading a repository.
+
+The upgrade is effectively performed as a copy between a source
+repository and a temporary destination repository.
+
+The source repository is unmodified for as long as possible so the
+upgrade can abort at any time without causing loss of service for
+readers and without corrupting the source repository.
+"""
+assert srcrepo.currentwlock()
+assert dstrepo.currentwlock()
+
+# TODO copy store
+
+backuppath = tempfile.mkdtemp(prefix='upgradebackup.', dir=srcrepo.path)
+backupvfs = scmutil.vfs(backuppath)
+
+# Make a backup of requires file first, as it is the first to be modified.
+util.copyfile(srcrepo.join('requires'), backupvfs.join('requires'))
+
+# We install an arbitrary requirement that clients must not support
+# as a mechanism to lock out new clients during the data swap. This is
+# better than allowing a client to continue while the repository is in
+# an inconsistent state.
+ui.write(_('marking source repository as being upgraded; clients will be '
+   'unable to read from repository\n'))
+scmutil.writerequires(srcrepo.vfs,
+  srcrepo.requirements | set(['upgradeinprogress']))
+
+ui.write(_('starting in-place swap of repository data\n'))
+ui.write(_('replaced files will be backed up at %s\n') %
+ backuppath)
+
+# TODO do the store swap here.
+
+# We first write the requirements file. Any new requirements will lock
+# out legacy clients.
+ui.write(_('finalizing requirements file and making repository readable '
+   'again\n'))
+scmutil.writerequires(srcrepo.vfs, requirements)
+
+return backuppath
+
 def upgraderepo(ui, repo, run=False, optimize=None):
 """Upgrade a repository in place."""
 # Avoid cycle: cmdutil -> repair -> localrepo -> cmdutil
@@ -773,3 +819,43 @@ def upgraderepo(ui, repo, run=False, opt
  '"--optimize ":\n\n'))
 for i in unusedoptimize:
 ui.write(_('%s\n   %s\n\n') % (i.name, i.description))
+return
+
+# Else we're in the run=true case.
+ui.write(_('upgrade will perform the following actions:\n\n'))
+printrequirements()
+printupgradeactions()
+
+ui.write(_('beginning upgrade...\n'))
+with repo.wlock():
+with repo.lock():
+ui.write(_('repository locked and read-only\n'))
+# Our strategy for upgrading the repository is to create a new,
+# temporary repository, write data to it, then do a swap of the
+# data. There are less heavyweight ways to do this, but it is 
easier
+# to create a new repo object than to instantiate all the 
components
+# (like the store) separately.
+tmppath = tempfile.mkdtemp(prefix='upgrade.', dir=repo.path)
+backuppath = None
+try:
+ui.write(_('creating temporary repository to stage migrated '
+   'data: %s\n') % tmppath)
+dstrepo = localrepo.localrepository(repo.baseui,
+path=tmppath,
+create=True)
+
+with dstrepo.wlock():
+   

Re: Future of copy metadata

2016-12-18 Thread Augie Fackler
(bcc: mpm to hopefully get pointers to anything he might have handy in notes on 
this subject)
> On Dec 18, 2016, at 4:31 PM, Gregory Szorc  wrote:
> 
> Mercurial currently stores file copy/rename metadata as a "header" in filelog 
> revision data. Furthermore, there is some wonkiness with p1 and p2 in the 
> filelog when copies are at play (see _filecommit() in localrepo.py). This 
> metadata means copies/renames can be followed without expensive run-time 
> "similarity" detection, which is great, especially for large repositories.
> 
> However, people or automated processes don't always perform the necessary 
> invocations of `hg copy` or `hg rename` to record copy/rename metadata. And 
> historically there have been a number of bugs or feature deficiencies where 
> copy/rename metadata is lost or not recorded where it should have been. 
> Coupled with the design of having copy metadata in the filelog data (which is 
> part of the hash and the merkle tree contributing to the changeset node), 
> this means that if copy metadata isn't correct from the beginning, it is 
> wrong forever. That's a pretty painful constraint.
> 
> The subject of copy/rename inaccuracy is a frequent complaint among Mozilla 
> developers doing lots of code archeology - in short they can't trust it and 
> they fall back to a Git conversion of the repo when they know copies/renames 
> are in play (Git performs copy/rename detection at operation run-time).
> 
> I recall a very informal conversation with mpm at the 3.8 Sprint in March 
> about this topic and he seemed to express a desire to move copy/rename 
> detection/metadata out of filelogs. I vaguely recall him suggesting it be 
> computed at run-time and cached if performance dictates. I also recall him 
> saying something about modern research in the area of copy detection has 
> enabled better solutions than "measure the percentage of identical lines."
> 
> I was wondering if there have been any formal discussions or proposals on the 
> future of copy metadata. I am most interested in:
> 
> * Whether there are plans for (or even an extension implementation of) a 
> supplemental copy metadata "database." The goal would be to correct 
> deficiencies in the set-in-stone filelog-based metadata.
> * Whether there are plans to move copy metadata out of filelog revisions 
> completely. (This would make the filelogs simpler and more clearly separate 
> file content from metadata.)
> * If we're talking about new designs for copy/rename metadata, should 
> improvements to linkrev be discussed at the same time?

I’m also interested in this, because it’d be nice for remotefilelog et al to be 
able to transmit copy information as a separate step on some setups.

> 
> ___
> 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


Future of copy metadata

2016-12-18 Thread Gregory Szorc
Mercurial currently stores file copy/rename metadata as a "header" in
filelog revision data. Furthermore, there is some wonkiness with p1 and p2
in the filelog when copies are at play (see _filecommit() in localrepo.py).
This metadata means copies/renames can be followed without expensive
run-time "similarity" detection, which is great, especially for large
repositories.

However, people or automated processes don't always perform the necessary
invocations of `hg copy` or `hg rename` to record copy/rename metadata. And
historically there have been a number of bugs or feature deficiencies where
copy/rename metadata is lost or not recorded where it should have been.
Coupled with the design of having copy metadata in the filelog data (which
is part of the hash and the merkle tree contributing to the changeset
node), this means that if copy metadata isn't correct from the beginning,
it is wrong forever. That's a pretty painful constraint.

The subject of copy/rename inaccuracy is a frequent complaint among Mozilla
developers doing lots of code archeology - in short they can't trust it and
they fall back to a Git conversion of the repo when they know
copies/renames are in play (Git performs copy/rename detection at operation
run-time).

I recall a very informal conversation with mpm at the 3.8 Sprint in March
about this topic and he seemed to express a desire to move copy/rename
detection/metadata out of filelogs. I vaguely recall him suggesting it be
computed at run-time and cached if performance dictates. I also recall him
saying something about modern research in the area of copy detection has
enabled better solutions than "measure the percentage of identical lines."

I was wondering if there have been any formal discussions or proposals on
the future of copy metadata. I am most interested in:

* Whether there are plans for (or even an extension implementation of) a
supplemental copy metadata "database." The goal would be to correct
deficiencies in the set-in-stone filelog-based metadata.
* Whether there are plans to move copy metadata out of filelog revisions
completely. (This would make the filelogs simpler and more clearly separate
file content from metadata.)
* If we're talking about new designs for copy/rename metadata, should
improvements to linkrev be discussed at the same time?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH] convert: config option for git rename limit

2016-12-18 Thread Gregory Szorc
# HG changeset patch
# User Gregory Szorc 
# Date 1482094400 28800
#  Sun Dec 18 12:53:20 2016 -0800
# Node ID 34f99a6983df082fc56f50e8aab9dd5f1bbe6676
# Parent  935092e525b0ee5656d0830162a1c2adf8248de3
convert: config option for git rename limit

By default, Git applies rename and copy detection to 400 files. The
diff.renamelimit config option and -l argument to diff commands can
override this.

As part of converting some repositories in the wild, I was hitting
the default limit. Unfortunately, the warnings that Git prints in this
scenario are swallowed because the process running functionality in
common.py redirects stderr to /dev/null by default. This seems like
a bug, but a bug for another day.

This commit establishes a config option to send the rename limit
through to `git diff-tree`. The added tests demonstrate a too-low
rename limit doesn't result in copy metadata being recorded.

diff --git a/hgext/convert/__init__.py b/hgext/convert/__init__.py
--- a/hgext/convert/__init__.py
+++ b/hgext/convert/__init__.py
@@ -320,6 +320,13 @@ def convert(ui, src, dest=None, revmapfi
 is very expensive for large projects, and is only effective when
 ``convert.git.similarity`` is greater than 0. The default is False.
 
+:convert.git.renamelimit: perform rename and copy detection up to this
+many changed files in a commit. Increasing this will make rename
+and copy detection more accurate but will significantly slow down
+computation on large projects. The option is only relevant if
+``convert.git.similarity`` is greater than 0. The default is
+``400``.
+
 :convert.git.remoteprefix: remote refs are converted as bookmarks with
 ``convert.git.remoteprefix`` as a prefix followed by a /. The default
 is 'remote'.
diff --git a/hgext/convert/git.py b/hgext/convert/git.py
--- a/hgext/convert/git.py
+++ b/hgext/convert/git.py
@@ -78,6 +78,10 @@ class convert_git(common.converter_sourc
  False)
 if findcopiesharder:
 self.simopt.append('--find-copies-harder')
+
+renamelimit = ui.configint('convert', 'git.renamelimit',
+   default=400)
+self.simopt.append('-l%d' % renamelimit)
 else:
 self.simopt = []
 
diff --git a/tests/test-convert-git.t b/tests/test-convert-git.t
--- a/tests/test-convert-git.t
+++ b/tests/test-convert-git.t
@@ -374,6 +374,31 @@ source, the copy source took the content
   A bar-copied2
 bar
 
+renamelimit config option works
+
+  $ cd git-repo2
+  $ cp bar bar-copy0
+  $ echo 0 >> bar-copy0
+  $ cp bar bar-copy1
+  $ echo 1 >> bar-copy1
+  $ git add bar-copy0 bar-copy1
+  $ commit -a -m 'copy bar 2 times'
+  $ cd ..
+
+  $ hg -q convert --config convert.git.renamelimit=1 \
+  > --config convert.git.findcopiesharder=true --datesort git-repo2 fullrepo2
+  $ hg -R fullrepo2 status -C --change master
+  A bar-copy0
+  A bar-copy1
+
+  $ hg -q convert --config convert.git.renamelimit=100 \
+  > --config convert.git.findcopiesharder=true --datesort git-repo2 fullrepo3
+  $ hg -R fullrepo3 status -C --change master
+  A bar-copy0
+bar
+  A bar-copy1
+bar
+
 test binary conversion (issue1359)
 
   $ count=19
diff --git a/tests/test-convert.t b/tests/test-convert.t
--- a/tests/test-convert.t
+++ b/tests/test-convert.t
@@ -261,6 +261,13 @@
 for large projects, and is only effective when
 "convert.git.similarity" is greater than 0. The default is
 False.
+  convert.git.renamelimit
+perform rename and copy detection up to this many changed
+files in a commit. Increasing this will make rename and 
copy
+detection more accurate but will significantly slow down
+computation on large projects. The option is only relevant
+if "convert.git.similarity" is greater than 0. The default
+is "400".
   convert.git.remoteprefix
 remote refs are converted as bookmarks with
 "convert.git.remoteprefix" as a prefix followed by a /. The
___
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-18 Thread Jun Wu
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. And we then use a special
"uisetup" which calls "wrapui" to modify the ui object created by dispatch.

Maybe this patch should be moved after adding "uisetup" in dispatch.request.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


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

2016-12-18 Thread Jun Wu
Excerpts from Gregory Szorc's message of 2016-12-17 16:40:12 -0800:
> It's probably not worth worrying about just yet, but with Linux PID 
> namespaces, multiple processes may think they have the same PID, even if that 
> PID maps to different values inside the kernel.
> 
> Mozilla has encountered problems with multiple hg processes running in 
> separate containers and PID namespaces acquiring the same lock from a shared 
> filesystem simultaneously because multiple hg processes share the same PID 
> and hostname in different "containers."

Pid namespace breaks mercurial lock, which is a symlink with the content
"host:pid".

Note that chg lock is not related to correctness. Without lock, chg also
behaves correctly, with the downside:

  1. More redirections.
  2. Spawn servers with a same config hash in parallel unnecessarily.

This series resolves 1. But that's just "nice-to-have". Even if we just drop
locks today, chg won't have correctness issues.

> 
> > On Dec 16, 2016, at 17:49, Jun Wu  wrote:
> > 
> > # HG changeset patch
> > # User Jun Wu 
> > # Date 1481938472 0
> > #  Sat Dec 17 01:34:32 2016 +
> > # Node ID 6c9ce8399350d8287599cd802b91adf73db08759
> > # Parent  69d25b06467d65bf6d1e85d34d8fc57ec321b51d
> > # Available At https://bitbucket.org/quark-zju/hg-draft 
> > #  hg pull https://bitbucket.org/quark-zju/hg-draft  -r 
> > 6c9ce8399350
> > chg: start server at a unique address
> > 
> > See the previous patch for motivation. Previously, the server is started at
> > a globally shared address, which requires a lock. This patch appends pid to
> > the address so it becomes unique.
> > 
> > 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();
> > 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 evolve-ext] fold: disallow multiple revisions without --exact

2016-12-18 Thread Martin von Zweigbergk via Mercurial-devel
On Fri, Dec 16, 2016, 23:53 Pierre-Yves David <
pierre-yves.da...@ens-lyon.org> wrote:

>
>
> On 11/05/2016 12:58 AM, Martin von Zweigbergk via Mercurial-devel wrote:
> > # HG changeset patch
> > # User Martin von Zweigbergk 
> > # Date 1478303512 25200
> > #  Fri Nov 04 16:51:52 2016 -0700
> > # Node ID bb80851fe9a6e14263f0076074108556377141f9
> > # Parent  cb2bac3253fbd52894ffcb4719a148fe6a3da38b
> > fold: disallow multiple revisions without --exact
> >
> > It's very easy to think that "hg fold 4::6" will fold exactly those
> > revisions. In reality, it will fold those *and* any revisions between
> > them and the working copy. It seems very likely that users who pass
> > more than one revision wants to fold exactly those revisions, so let's
> > abort and hint that they may be looking for --exact.
>
> That seems a good first step to take in all cases. Unfortunately, the
> patch fails to apply for an unclear reason. Can you send me a new version?
>
> In addition, I suggest the following changes:
>
> - we should allow multiple revision if '.' is one of them. They will be
> no surprise in this case.
>

Even if there is a gap between the revisions and '.'? I wonder if we should
fail only if the given commits form a contiguous range (I assume that's
required for --exact). So "hg fold .^^" would work because it is a single
revision. "hg fold .^^ .^" would fail because it's multiple revisions. "hg
fold .^ ." would work because it includes '.'. "hg fold .^ .^2" works
because it's not a contiguous range (without the implicit '.'). It does
feel like a little too much logic too the degree that it may surprise
users, but I think the behavior without it may surprise users even more.



> - update to the commit message,
>
> abort: multiple revisions specified
> (do you want --exact?, see "hg help fold" for details)
>
>
> (I'll tell more about possible changes to the default in the rest of the
> thread)
>

Having said the above, I'm more in favor of making --exact the default and
not needing the protection mentioned above, so I'm looking forward to
hearing what you have to say here.


> > diff -r cb2bac3253fb -r bb80851fe9a6 hgext/evolve.py
> > --- a/hgext/evolve.pyWed Nov 02 18:56:44 2016 +0100
> > +++ b/hgext/evolve.pyFri Nov 04 16:51:52 2016 -0700
> > @@ -3115,6 +3115,11 @@
> >  revs = scmutil.revrange(repo, revs)
> >
> >  if not opts['exact']:
> > +if len(revs) > 1:
> > +raise error.Abort(_("cannot fold from working directory to "
> > +"more than one revision"),
> > +  hint=_("did you mean to use --exact?"))
> > +
> >  # Try to extend given revision starting from the working
> directory
> >  extrevs = repo.revs('(%ld::.) or (.::%ld)', revs, revs)
> >  discardedrevs = [r for r in revs if r not in extrevs]
> > diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-evolve.t
> > --- a/tests/test-evolve.tWed Nov 02 18:56:44 2016 +0100
> > +++ b/tests/test-evolve.tFri Nov 04 16:51:52 2016 -0700
> > @@ -688,7 +688,11 @@
> >$ hg fold -r 4 -r 6 --exact
> >abort: cannot fold non-linear revisions (multiple roots given)
> >[255]
> > -  $ hg fold 10 1
> > +  $ hg fold 4::5
> > +  abort: cannot fold from working directory to more than one revision
> > +  (did you mean to use --exact?)
> > +  [255]
> > +  $ hg fold 1
> >abort: cannot fold non-linear revisions
> >(given revisions are unrelated to parent of working directory)
> >[255]
> > diff -r cb2bac3253fb -r bb80851fe9a6 tests/test-userguide.t
> > --- a/tests/test-userguide.tWed Nov 02 18:56:44 2016 +0100
> > +++ b/tests/test-userguide.tFri Nov 04 16:51:52 2016 -0700
> > @@ -109,7 +109,7 @@
> >7:05e61aab8294  step 1
> >8:be6d5bc8e4cc  step 2
> >9:35f432d9f7c1  step 3
> > -  $ hg fold -d '0 0' -m 'fix bug 64' -r 7::
> > +  $ hg fold -d '0 0' -m 'fix bug 64' -r 7
> >3 changesets folded
> >1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> >$ hg --hidden shortlog -G -r 6::
>
>
> --
> Pierre-Yves David
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 1 of 4] py3: make sure encoding.encoding is a bytes variable

2016-12-18 Thread Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <7895pul...@gmail.com>
# Date 1481999125 -19800
#  Sat Dec 17 23:55:25 2016 +0530
# Node ID af87471131e2fecc9e6edeb2a5c2e155953da4bb
# Parent  1c8efe62f1f36fdf1a1bd6fcc5924cf557141d4a
py3: make sure encoding.encoding is a bytes variable

encoding.encoding returns unicodes when locale.getpreferredencoding() is used
to get the preferred encoding. This patch fixes that.

diff -r 1c8efe62f1f3 -r af87471131e2 mercurial/encoding.py
--- a/mercurial/encoding.py Sat Dec 17 20:24:46 2016 +0530
+++ b/mercurial/encoding.py Sat Dec 17 23:55:25 2016 +0530
@@ -93,7 +93,7 @@
 try:
 encoding = environ.get("HGENCODING")
 if not encoding:
-encoding = locale.getpreferredencoding() or 'ascii'
+encoding = locale.getpreferredencoding().encode('ascii') or 'ascii'
 encoding = _encodingfixers.get(encoding, lambda: encoding)()
 except locale.Error:
 encoding = 'ascii'
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 3 of 4] py3: have a bytes version of sys.platform

2016-12-18 Thread Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <7895pul...@gmail.com>
# Date 1482002525 -19800
#  Sun Dec 18 00:52:05 2016 +0530
# Node ID e737d795d5813e66ce7a57a74fa30f52f6eb4ec4
# Parent  ffdca92474e806c84d0e3383768b3011202d650e
py3: have a bytes version of sys.platform

sys.platform returns unicodes on Python 3. This patch adds up
pycompat.sysplatform which returns bytes.

diff -r ffdca92474e8 -r e737d795d581 mercurial/pycompat.py
--- a/mercurial/pycompat.py Sun Dec 18 00:44:21 2016 +0530
+++ b/mercurial/pycompat.py Sun Dec 18 00:52:05 2016 +0530
@@ -50,6 +50,7 @@
 # os.getcwd() on Python 3 returns string, but it has os.getcwdb() which
 # returns bytes.
 getcwd = os.getcwdb
+sysplatform = sys.platform.encode('ascii')
 
 # TODO: .buffer might not exist if std streams were replaced; we'll need
 # a silly wrapper to make a bytes stream backed by a unicode one.
@@ -153,6 +154,7 @@
 stdout = sys.stdout
 stderr = sys.stderr
 sysargv = sys.argv
+sysplatform = sys.platform
 getcwd = os.getcwd
 
 stringio = io.StringIO
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 2 of 4] py3: have a bytes version of os.altsep

2016-12-18 Thread Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <7895pul...@gmail.com>
# Date 1482002061 -19800
#  Sun Dec 18 00:44:21 2016 +0530
# Node ID ffdca92474e806c84d0e3383768b3011202d650e
# Parent  af87471131e2fecc9e6edeb2a5c2e155953da4bb
py3: have a bytes version of os.altsep

os.altsep returns unicodes on Python 3. We need a bytes version hence added
pycompat.altsep.

diff -r af87471131e2 -r ffdca92474e8 mercurial/pycompat.py
--- a/mercurial/pycompat.py Sat Dec 17 23:55:25 2016 +0530
+++ b/mercurial/pycompat.py Sun Dec 18 00:44:21 2016 +0530
@@ -44,6 +44,9 @@
 osname = os.name.encode('ascii')
 ospathsep = os.pathsep.encode('ascii')
 ossep = os.sep.encode('ascii')
+osaltsep = os.altsep
+if osaltsep:
+osaltsep = osaltsep.encode('ascii')
 # os.getcwd() on Python 3 returns string, but it has os.getcwdb() which
 # returns bytes.
 getcwd = os.getcwdb
@@ -145,6 +148,7 @@
 osname = os.name
 ospathsep = os.pathsep
 ossep = os.sep
+osaltsep = os.altsep
 stdin = sys.stdin
 stdout = sys.stdout
 stderr = sys.stderr
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


[PATCH 4 of 4] py3: replace os.altsep with pycompat.altsep

2016-12-18 Thread Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <7895pul...@gmail.com>
# Date 1482004032 -19800
#  Sun Dec 18 01:17:12 2016 +0530
# Node ID ebca87566128e20c8f044f4e2d9aaec36efef625
# Parent  e737d795d5813e66ce7a57a74fa30f52f6eb4ec4
py3: replace os.altsep with pycompat.altsep

All the occurences of os.altsep are replaced with pycompat.altsep which
returns bytes.

diff -r e737d795d581 -r ebca87566128 mercurial/hgweb/common.py
--- a/mercurial/hgweb/common.py Sun Dec 18 00:52:05 2016 +0530
+++ b/mercurial/hgweb/common.py Sun Dec 18 01:17:12 2016 +0530
@@ -143,7 +143,7 @@
 for part in parts:
 if (part in ('', os.curdir, os.pardir) or
 pycompat.ossep in part or
-os.altsep is not None and os.altsep in part):
+pycompat.osaltsep is not None and pycompat.osaltsep in part):
 return
 fpath = os.path.join(*parts)
 if isinstance(directory, str):
diff -r e737d795d581 -r ebca87566128 mercurial/templater.py
--- a/mercurial/templater.pySun Dec 18 00:52:05 2016 +0530
+++ b/mercurial/templater.pySun Dec 18 01:17:12 2016 +0530
@@ -1245,7 +1245,7 @@
 if (not style
 or style in (os.curdir, os.pardir)
 or pycompat.ossep in style
-or os.altsep and os.altsep in style):
+or pycompat.osaltsep and pycompat.osaltsep in style):
 continue
 locations = [os.path.join(style, 'map'), 'map-' + style]
 locations.append('map')
diff -r e737d795d581 -r ebca87566128 mercurial/util.py
--- a/mercurial/util.py Sun Dec 18 00:52:05 2016 +0530
+++ b/mercurial/util.py Sun Dec 18 01:17:12 2016 +0530
@@ -1304,8 +1304,8 @@
 return dict((normcase(n), n) for n in os.listdir(dir))
 
 seps = pycompat.ossep
-if os.altsep:
-seps = seps + os.altsep
+if pycompat.osaltsep:
+seps = seps + pycompat.osaltsep
 # Protect backslashes. This gets silly very quickly.
 seps.replace('\\','')
 pattern = remod.compile(r'([^%s]+)|([%s]+)' % (seps, seps))
@@ -1371,7 +1371,7 @@
 def endswithsep(path):
 '''Check path ends with os.sep or os.altsep.'''
 return (path.endswith(pycompat.ossep)
-or os.altsep and path.endswith(os.altsep))
+or pycompat.osaltsep and path.endswith(pycompat.osaltsep))
 
 def splitpath(path):
 '''Split path by os.sep.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


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

2016-12-18 Thread Yuya Nishihara
# 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)

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 

Re: [PATCH 3 of 6] py3: replace os.sep with pycompat.ossep (part 1 of 4)

2016-12-18 Thread Yuya Nishihara
On Sat, 17 Dec 2016 21:11:42 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pul...@gmail.com>
> # Date 1481984790 -19800
> #  Sat Dec 17 19:56:30 2016 +0530
> # Node ID 4107888f9b89e178ab8369a6d63ebb2e17dc26b9
> # Parent  32865b15b8417a3ad673b72ffc713dffdebcd04a
> py3: replace os.sep with pycompat.ossep (part 1 of 4)

>  def endswithsep(path):
>  '''Check path ends with os.sep or os.altsep.'''
> -return path.endswith(os.sep) or os.altsep and path.endswith(os.altsep)
> +return path.endswith(pycompat.ossep) or os.altsep\
> +and path.endswith(os.altsep)

I want to keep the 'x and y' in the same line since it is stronger than 'or'.
Rewritten as:

return (path.endswith(pycompat.ossep)
or os.altsep and path.endswith(os.altsep))

BTW, we'll need pycompat.osaltsep.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 1 of 6] py3: use %d instead of %s for integers

2016-12-18 Thread Yuya Nishihara
On Sat, 17 Dec 2016 21:11:40 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pul...@gmail.com>
> # Date 1481983600 -19800
> #  Sat Dec 17 19:36:40 2016 +0530
> # Node ID ea408eb4d17eb54f68ffcbabb1c32cacfb64955d
> # Parent  935092e525b0ee5656d0830162a1c2adf8248de3
> py3: use %d instead of %s for integers

Queued with a minor style fix, thanks.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


Re: [PATCH 5 of 6] py3: replace os.sep with pycompat.ossep (part 3 of 4)

2016-12-18 Thread Yuya Nishihara
On Sat, 17 Dec 2016 21:11:44 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pul...@gmail.com>
> # Date 1481985864 -19800
> #  Sat Dec 17 20:14:24 2016 +0530
> # Node ID c816d1b5b807f8076b027f9cb9ffdeafe5fd7e62
> # Parent  a82ca84371f5f4b4213aec534ab0ccdfbd04cf9b
> py3: replace os.sep with pycompat.ossep (part 3 of 4)

> @@ -139,7 +142,7 @@
>  parts = fname.split('/')
>  for part in parts:
>  if (part in ('', os.curdir, os.pardir) or
> -os.sep in part or os.altsep is not None and os.altsep in part):
> +pycompat.ossep in part or os.altsep is not None and os.altsep in 
> part):

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


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

2016-12-18 Thread Yuya Nishihara
On Sat, 17 Dec 2016 16:40:12 -0800, Gregory Szorc wrote:
> It's probably not worth worrying about just yet, but with Linux PID 
> namespaces, multiple processes may think they have the same PID, even if that 
> PID maps to different values inside the kernel.
> 
> Mozilla has encountered problems with multiple hg processes running in 
> separate containers and PID namespaces acquiring the same lock from a shared 
> filesystem simultaneously because multiple hg processes share the same PID 
> and hostname in different "containers."

We wouldn't need to care much about that for chg, since there would be little
benefit to mount /tmp as a shared filesystem (and we have a somewhat similar
issue in the current code which uses flock.)

The overall changes look good to me.
___
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-18 Thread Yuya Nishihara
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.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel