Re: Output buffering on Windows 10
For context, my change to reopen stdout was done so that pager wouldn't replace an instrumented stdout with a fresh one that lacked instrumentation. The idea was that we want line buffered or unbuffered output to anything that writes to a TTY eventually, whether it's a pager or direct to a terminal; fully buffered output is only appropriate when writing to a pipe or to disk. I think that the right fix is to ask for line buffered for POSIX and unbuffered for Windows if the original stdout is a TTY. Simon On 14/06/2018, 14:24, "Sune Foldager" wrote: Greetings I’d like to discuss a problem which results in console output not working correctly on Windows 10 and not having done so since late may 2017! The actual cause is due to a changeset by Simon Farnsworth in the beginning of february last year (changeset 3a4c0905f357): util: always force line buffered stdout when stdout is a tty (BC) …which essentially causes stdout to be reopened in line buffered mode as long as stdout is a tty at program start. This was previously only done when the pager was activated (in pager.py). Apparently this was done to make some later changes (by Simon) easier. The reopen happens in util.py: +if isatty(stdout): +stdout = os.fdopen(stdout.fileno(), 'wb', 1) On Linux, I guess this is a no-op since the default buffering mode for ttys is line buffering. Unfortunately this doesn’t work on Windows. The default on Windows is unbuffered and furthermore Windows doesn’t even support line buffering mode, treating it as fully buffered. Now, normally this wouldn’t affect Windows since the “win32” color mode was used there, which involves breaking the text down and calling flush a lot (at least once after each ui.write). But now comes Matt Harbison’s change (changeset 98c2b44bdf9a) which enables ANSI mode on Windows, causing it to use the same code paths as Linux. This activates the bug. With the bug active, all output is fully buffered on Windows which can readily be seen by doing something “slow” like hg in, hg clone or similar. The end result is very unsatisfactory. As a workaround (at my workplace), we have set our color.mode back to win32 (instead of auto) and specified the color.pagermode manually to ansi. But we’d obviously like to use ANSI mode, now that it’s supported by Windows. I’d love to fix this bug, but I don’t know how, since the most obvious solution would be to exactly back out of Simon’s changeset, only enabling buffered mode (full on Windows, line on Linux) when the pager is about to run. But Simon clearly didn’t make that patch for nothing so… I’m not sure what the best course of action is. I am also a bit puzzled that no one has noticed this bug, e.g. Mark when he enabled this behaviour, but of course it doesn’t happen for “fast” commands like hg debugcolor or similar. Thanks :) — Sune ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D477: revlog: add option to mmap revlog index
simonfar added inline comments.
INLINE COMMENTS
> test-revlog-mmapindex.t:12-13
> +
> +mmap index which is now more than 4k long
> + $ hg log -l 5 -T '{rev}\n' --config experimental.mmapindexthreshold=4k
> + 100
Two things:
1. Can you add a `ui.debug()` or similar so that you can show that `mmap()` is
in use here?
2. Can you repeat the test without `mmap()` to show that `mmap()` and
not-`mmap()` both still work?
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D477
To: mbthomas, #fbhgext, indygreg, #hg-reviewers, durin42
Cc: quark, durin42, simonfar, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D476: util: add an mmapread method
simonfar added inline comments. INLINE COMMENTS > util.py:413 > +try: > +return mmap.mmap(fp.fileno(), 0, access=mmap.ACCESS_READ) > +except ValueError: # cannot mmap an empty file As per comment on https://phab.mercurial-scm.org/D477 - do we want this to be raw, or `buffer(mmap.mmap(...`? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D476 To: mbthomas, #fbhgext, #hg-reviewers Cc: simonfar, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D477: revlog: add option to mmap revlog index
simonfar added inline comments. INLINE COMMENTS > revlog.py:343 > +if self._mmapindex: > +indexdata = util.buffer(util.mmapread(f)) > +else: Is there any reason for `mmapread` to not immediately wrap the data in a `util.buffer` for you? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D477 To: mbthomas, #fbhgext, indygreg, #hg-reviewers Cc: simonfar, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D119: color: add autoterminfo option to prefer terminfo over ansi for non-windows
simonfar added a comment. Looks good to me, though. INLINE COMMENTS > color.py:259-264 > elif realmode == 'terminfo': > _terminfosetup(ui, mode) > if not ui._terminfoparams: > ## FIXME Shouldn't we return None in this case too? > modewarn() > realmode = 'ansi' For the real reviewer - this combines with `modewarn()` above to do the silent fallback promised above. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D119 EMAIL PREFERENCES https://phab.mercurial-scm.org/settings/panel/emailpreferences/ To: spectral, #hg-reviewers Cc: simonfar, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Hidden Commits in 4.3
On 05/04/2017 02:11, Durham Goode wrote: There's been a lot of discussion about how to hide and unhide commits lately [0][1], and I feel the complexity of our current approach is hurting our ability to reason about it, making it impossible to make progress. I would like to formally propose a new pattern for dealing with hidden commits, along with the concrete steps to getting it enabled in core by default by the August release. The proposal is quite concise, so check out this 1-page Google doc for the details and to comment: https://urldefense.proofpoint.com/v2/url?u=https-3A__goo.gl_7DJ9AI=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=cTl6eEfd0SyhW5M6ZM65eJ2ZldOrwdJ_KM0pKS9tZ5M=T30BYaK4BkpV_DBoFTcWZm0_VGBJG0FEHA5jUJqcxXw= Is there any reason why this is a Google Doc, not a HiddenCommitsPlan on the mercurial-scm.org wiki? If people find this approach promising, I can commit to trying to get this upstream by the August release. So if you have questions or concerns, please let me know so I can address them. [0] see: "obsolete: track node versions" [1] see: "repo: add an ability to hide nodes in an appropriate way" ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=cTl6eEfd0SyhW5M6ZM65eJ2ZldOrwdJ_KM0pKS9tZ5M=NFt0PtRadAIuciZ1Qg6kefWsdRxaXeOt_aG7bSnFTcQ= -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 4 V2] obsolete: allow cycles
On 23/03/2017 22:15, Jun Wu wrote: Excerpts from Augie Fackler's message of 2017-03-23 17:53:39 -0400: On Wed, Mar 22, 2017 at 1:42 AM, Jun Wu <qu...@fb.com> wrote: As for alternatives, a correct solution needs to refer to the marker it is replacing. Since cycles should be rare, can we record the full "replaced marker" data inline in the new marker? In local storage, that could be an If "markers being replaced" are explicitly recorded, you will miss remote markers that can be possibly replaced because you don't know them at the time appending a new local marker. So you end up with some "conflict resolution" logic during exchange. That is not very different from just using the offsets - since obsstore is append-only, new markers just "replace" old ones (I don't think there is an exception that the newly added marker is intended to be replaced by a previous one when working locally). It's simpler but has the same exchange headache. I wonder: could we get away from using dates by putting some kind of generation number in the marker? So if a marker would create a cycle, we increment its generation number relative to the previous highest value in the cycle. The problem is you don't know if a marker will create a cycle (or should invalidate another marker), because remote markers are unknown. If you do that during exchange, it makes exchange more complex. I think that is not very different from just using the offsets. Basically, if the new filed which is meant to be spread globally, date is probably the best option. If it is not meant to be spread globally, offset is already a decent choice, without format change. But offsets brings up the exchange headache. A thought experiment, to tease out concerns here; if we replaced the use of date with hashfunc(marker) (where hashfunc is a randomly chosen crypto hash), would people still have concerns? In other words, if we take the semantics of "date" away, and treat "date" as a random number that happens to always exist, do people's concerns still hold water? -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH v2 stable] subrepo: move prompts out of the if (issue5505)
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1490009815 25200
# Mon Mar 20 04:36:55 2017 -0700
# Branch stable
# Node ID 43fbb6661cfe578135791a3caff1473846c17899
# Parent 10c0ee33853539bd2721a05d4753785c2494d243
subrepo: move prompts out of the if (issue5505)
Prompts weren't available in the else clause
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -194,7 +194,9 @@
r = "%s:%s:%s" % r
repo.ui.debug(" subrepo %s: %s %s\n" % (s, msg, r))
+promptssrc = filemerge.partextras(labels)
for s, l in sorted(s1.iteritems()):
+prompts = None
a = sa.get(s, nullstate)
ld = l # local state with possible dirty flag for compares
if wctx.sub(s).dirty():
@@ -202,9 +204,9 @@
if wctx == actx: # overwrite
a = ld
+prompts = promptssrc.copy()
+prompts['s'] = s
if s in s2:
-prompts = filemerge.partextras(labels)
-prompts['s'] = s
r = s2[s]
if ld == r or r == a: # no change or local is newer
sm[s] = l
@@ -267,6 +269,7 @@
wctx.sub(s).remove()
for s, r in sorted(s2.items()):
+prompts = None
if s in s1:
continue
elif s not in sa:
@@ -274,6 +277,8 @@
mctx.sub(s).get(r)
sm[s] = r
elif r != sa[s]:
+prompts = promptssrc.copy()
+prompts['s'] = s
if repo.ui.promptchoice(
_(' remote%(o)s changed subrepository %(s)s'
' which local%(l)s removed\n'
diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t
--- a/tests/test-subrepo.t
+++ b/tests/test-subrepo.t
@@ -349,7 +349,7 @@
local removed, remote changed, keep changed
$ hg merge 6
- remote [merge rev] changed subrepository s which local [working copy]
removed
+ remote [merge rev] changed subrepository t which local [working copy]
removed
use (c)hanged version or (d)elete? c
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
@@ -380,7 +380,7 @@
$ hg merge --config ui.interactive=true 6 < d
> EOF
- remote [merge rev] changed subrepository s which local [working copy]
removed
+ remote [merge rev] changed subrepository t which local [working copy]
removed
use (c)hanged version or (d)elete? d
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
@@ -404,7 +404,7 @@
$ hg co -C 6
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg merge 11
- local [working copy] changed subrepository s which remote [merge rev]
removed
+ local [working copy] changed subrepository t which remote [merge rev]
removed
use (c)hanged version or (d)elete? c
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
@@ -436,7 +436,7 @@
$ hg merge --config ui.interactive=true 11 < d
> EOF
- local [working copy] changed subrepository s which remote [merge rev]
removed
+ local [working copy] changed subrepository t which remote [merge rev]
removed
use (c)hanged version or (d)elete? d
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] subrepo: move prompts out of the if (issue5505)
On 17/03/2017 17:28, FUJIWARA Katsunori wrote:
At Fri, 17 Mar 2017 13:10:57 -0700,
Simon Farnsworth wrote:
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1489781443 25200
# Fri Mar 17 13:10:43 2017 -0700
# Node ID 0ccc212fc0224209864e8c902920a91acd1bc414
# Parent a5bad127128d8f60060be53d161acfa7a32a17d5
subrepo: move prompts out of the if (issue5505)
Prompts weren't available in the else clause
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -195,6 +195,7 @@
r = "%s:%s:%s" % r
repo.ui.debug(" subrepo %s: %s %s\n" % (s, msg, r))
+prompts = filemerge.partextras(labels)
for s, l in sorted(s1.iteritems()):
a = sa.get(s, nullstate)
ld = l # local state with possible dirty flag for compares
@@ -203,9 +204,8 @@
if wctx == actx: # overwrite
a = ld
+prompts['s'] = s
if s in s2:
-prompts = filemerge.partextras(labels)
-prompts['s'] = s
r = s2[s]
if ld == r or r == a: # no change or local is newer
sm[s] = l
@@ -275,6 +275,7 @@
mctx.sub(s).get(r)
sm[s] = r
elif r != sa[s]:
+prompts['s'] = s
if repo.ui.promptchoice(
_(' remote%(o)s changed subrepository %(s)s'
' which local%(l)s removed\n'
How about copying before modification at each repetitions ?
promptssrc = filemerge.partextras(labels)
for s, l in sorted(s1.iteritems()):
prompts = promptssrc.copy()
prompts['s'] = s
for s, r in sorted(s2.items()):
elif r != sa[s]:
prompts = promptssrc.copy()
prompts['s'] = s
("del prompts" before s2 loop for safety is over-killing ? :-) )
Sharing result of filemerge.partextras() between each repetitions and
modifying it directly might cause wrong information, which is assigned
at previous repetition, like below.
In this case, shallow copy seems cheap enough.
That seems like a good idea - del prompts does seem like overkill, but
ending each repetition with prompts = None would ensure that the bug I
introduced into the test case is caught. I'll do it in a v2 when I'm
back in the UK.
diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t
--- a/tests/test-subrepo.t
+++ b/tests/test-subrepo.t
@@ -349,7 +349,7 @@
local removed, remote changed, keep changed
$ hg merge 6
- remote [merge rev] changed subrepository s which local [working copy]
removed
+ remote [merge rev] changed subrepository t which local [working copy]
removed
use (c)hanged version or (d)elete? c
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
@@ -380,7 +380,7 @@
$ hg merge --config ui.interactive=true 6 < d
> EOF
- remote [merge rev] changed subrepository s which local [working copy]
removed
+ remote [merge rev] changed subrepository t which local [working copy]
removed
use (c)hanged version or (d)elete? d
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
According to "hg debugsub" output in test-subrepo.t, merging revision
6 into revision 11 decreases managed subrepos from "s, t" to "s".
$ hg ci -m6 # change sub
committing subrepository t
$ hg debugsub
path s
source s
revision e4ece1bf43360ddc8f6a96432201a37b7cd27ae4
path t
source t
revision 6747d179aa9a688023c4b0cad32e4c92bb7f34ad
$ hg ci -m11
created new head
$ hg debugsub
path s
source s
revision e4ece1bf43360ddc8f6a96432201a37b7cd27ae4
Therefore, subrepo removed on "local" side is not "s" but "t" at this
merging, and changes (or fixing wrong expectations) above seem
reasonable.
@@ -404,7 +404,7 @@
$ hg co -C 6
2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ hg merge 11
- local [working copy] changed subrepository s which remote [merge rev]
removed
+ local [working copy] changed subrepository t which remote [merge rev]
removed
use (c)hanged version or (d)elete? c
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
@@ -436,7 +436,7 @@
$ hg merge --config ui.interactive=true 11 < d
> EOF
- local [working copy] changed subrepository s which remote [merge rev]
removed
+ local [working copy] changed subrepository t which remote [merge rev]
removed
use (c)hanged version or (d)elete? d
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
On the other hand, merging revision 11 into revision 6 increases
managed subrepos from "s" to "s,
Re: Dropping support for Python 2.6 on Windows
On 16/03/2017 10:14, Augie Fackler wrote: On Thu, Mar 16, 2017 at 09:54:27AM -0700, Gregory Szorc wrote: There is a brewing discussion about the future of Python 2.6 in Mercurial. I'd like to start with what I think will be an easy proposal: officially dropping support for 2.6 on Windows. I'm in favor of this. Perhaps also sound the alarm that 2.6 will be dropped either in 4.3 or 4.4, since we keep accidentally breaking it, and dropping it entirely would be a nice quality of life improvement for the devs. Also, there's some work afoot in service of pypi and pip that should let us get cffi'd SecureTransport bindings on OS X, and probably also a nicer http client I don't have to write, but thoes are all unlikely to support 2.6. Just as added data for this decision, we (Facebook) will stop shipping and testing Mercurial built against Python 2.6 no later than 2017-04-30 (when we lose internal security support for our Python 2.6 systems). So the 4.3 release is likely to be the last release where we provide Python 2.6 testing during the freeze. We're hoping that our last consumers of Python 2.6 Mercurial will move onto Python 2.7 before 2017-03-31, so for the 4.3 freeze we're likely to only be running the test suite, not also getting feedback from real users. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 3 V3] chgcache: implement a smartcache layer
+# That said, in this test we do know nobody else will touch the file,
+# so it's fine.
I'd prefer to avoid the need for this comment - instead:
finalhash = vfs.stat(filename).st_size
if finalhash == newhash:
return newhash, value
else:
return None, value
This has the advantage of being a worked example for people who learn
better from examples than from documentation.
+return newhash, value
+
+loadfuncs = {'foo': readfoo}
+vfs = scmutil.vfs(os.environ['TESTTMP'])
+
+cache = chgcache.smartcache('vfs', vfs, loadfuncs)
+
+def printcache():
+print('cache["foo"] = %r' % cache.get('foo'))
+
+printcache() # None, because the file does not exist
+
+vfs.write(filename, 'a')
+printcache() # cache miss, 'a'
+printcache() # cache hit, 'a'
+
+vfs.write(filename, 'ab')
+printcache() # cache miss, 'ab'
+
+vfs.write(filename, 'cd')
+printcache() # cache hit, 'ab'
+
+vfs.unlink('foo')
+printcache() # None, will invalidate the cache
+
+vfs.write(filename, 'ef')
+printcache() # cache miss, 'ef'
diff --git a/tests/test-chgcache.py.out b/tests/test-chgcache.py.out
new file mode 100644
--- /dev/null
+++ b/tests/test-chgcache.py.out
@@ -0,0 +1,12 @@
+cache["foo"] = None
+cache miss
+cache["foo"] = 'a'
+cache hit
+cache["foo"] = 'a'
+cache miss
+cache["foo"] = 'ab'
+cache hit
+cache["foo"] = 'ab'
+cache["foo"] = None
+cache miss
+cache["foo"] = 'ef'
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=reQmKdchqeMVRwrhw7ZqWyDvfs90FzZDm_PbdOvq4oo=DZWjoUWpkqSpspZkpl_xGThGgkNdeyF3WiYgAFIIKnM=
--
Simon Farnsworth
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 7 of 7] hgk: set a blocked tag when the user invokes view
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1488799672 28800
# Mon Mar 06 03:27:52 2017 -0800
# Node ID a24141da65e18e293bcd62f85f05050f01815942
# Parent 5b8f7a33145a182a4d4985972a8e4425eb20908f
hgk: set a blocked tag when the user invokes view
diff --git a/hgext/hgk.py b/hgext/hgk.py
--- a/hgext/hgk.py
+++ b/hgext/hgk.py
@@ -345,4 +345,4 @@
cmd = ui.config("hgk", "path", "hgk") + " %s %s" % (optstr, " ".join(etc))
ui.debug("running %s\n" % cmd)
-ui.system(cmd)
+ui.system(cmd, blockedtag='hgk_view')
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 3 of 7] dispatch: set a blockedtag when running an external alias
# HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1488799544 28800 # Mon Mar 06 03:25:44 2017 -0800 # Node ID 6cb267dc6b8156333413b6bbadc819807d73241c # Parent 1c0e78f6c4db0c43799ee7d2fe68290c9f172849 dispatch: set a blockedtag when running an external alias diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py --- a/mercurial/dispatch.py +++ b/mercurial/dispatch.py @@ -354,7 +354,8 @@ return '' cmd = re.sub(r'\$(\d+|\$)', _checkvar, self.definition[1:]) cmd = aliasinterpolate(self.name, args, cmd) -return ui.system(cmd, environ=env) +return ui.system(cmd, environ=env, + blockedtag='alias_%s' % self.name) self.fn = fn return ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 7] patch: set a blockedtag when running an external filter
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1488799529 28800
# Mon Mar 06 03:25:29 2017 -0800
# Node ID 1c0e78f6c4db0c43799ee7d2fe68290c9f172849
# Parent dc84da3efb972a86375c59fbb859bc31fc2d6335
patch: set a blockedtag when running an external filter
diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -1064,7 +1064,8 @@
# Start the editor and wait for it to complete
editor = ui.geteditor()
ret = ui.system("%s \"%s\"" % (editor, patchfn),
-environ={'HGUSER': ui.username()})
+environ={'HGUSER': ui.username()},
+blockedtag='filterpatch')
if ret != 0:
ui.warn(_("editor exited with exit code %d\n") % ret)
continue
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 7] sshpeer: set a blockedtag when starting ssh
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1488799509 28800
# Mon Mar 06 03:25:09 2017 -0800
# Node ID dc84da3efb972a86375c59fbb859bc31fc2d6335
# Parent bca31954883ec7ffd16ee940bb84f12f60d286c8
sshpeer: set a blockedtag when starting ssh
So that the data is readable.
diff --git a/mercurial/sshpeer.py b/mercurial/sshpeer.py
--- a/mercurial/sshpeer.py
+++ b/mercurial/sshpeer.py
@@ -150,7 +150,7 @@
util.shellquote("%s init %s" %
(_serverquote(remotecmd), _serverquote(self.path
ui.debug('running %s\n' % cmd)
-res = ui.system(cmd)
+res = ui.system(cmd, blockedtag='sshpeer')
if res != 0:
self._abort(error.RepoError(_("could not create remote repo")))
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] filemerge: tag merge tool for blocked times
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1488799180 28800
# Mon Mar 06 03:19:40 2017 -0800
# Node ID bca31954883ec7ffd16ee940bb84f12f60d286c8
# Parent b4cd912d7704cd976e1bee3a3c927e0e578ec88f
filemerge: tag merge tool for blocked times
Merge tools can take a while - let's ensure that they're appropriately tagged
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -493,7 +493,7 @@
repo.ui.status(_('running merge tool %s for file %s\n') %
(tool, fcd.path()))
repo.ui.debug('launching merge tool: %s\n' % cmd)
-r = ui.system(cmd, cwd=repo.root, environ=env)
+r = ui.system(cmd, cwd=repo.root, environ=env, blockedtag='mergetool')
repo.ui.debug('merge tool returned: %s\n' % r)
return True, r, False
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] hook: give exthooks tags for blocking time
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1488798555 28800
# Mon Mar 06 03:09:15 2017 -0800
# Node ID 9bdc781849eb9bba369cf9698cb8a1f5aef2b966
# Parent b4cd912d7704cd976e1bee3a3c927e0e578ec88f
hook: give exthooks tags for blocking time
The ui.system autogenerated tag isn't really useful - as they're named, let's
use the name the user gave us.
diff --git a/mercurial/hook.py b/mercurial/hook.py
--- a/mercurial/hook.py
+++ b/mercurial/hook.py
@@ -142,7 +142,7 @@
cwd = repo.root
else:
cwd = pycompat.getcwd()
-r = ui.system(cmd, environ=env, cwd=cwd)
+r = ui.system(cmd, environ=env, cwd=cwd, blockedtag='exthook-%s' % (name,))
duration = util.timer() - starttime
ui.log('exthook', 'exthook-%s: %s finished in %0.2f seconds\n',
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 9 of 9 pager v2] tests: prove that ignore works
I'm no longer seeing issues with these patches, and it'd be nice to reduce the pile of extensions you need to enable to get a modern workflow out of Hg. On 16/02/2017 16:59, Augie Fackler wrote: # HG changeset patch # User Augie Fackler <au...@google.com> # Date 1486441324 18000 # Mon Feb 06 23:22:04 2017 -0500 # Node ID a88d5ea0dd2855d49adae8a6c820e278ec594512 # Parent 4c2f3f1b67a71401faff082dbca79a3f212b5499 tests: prove that ignore works diff --git a/tests/test-pager.t b/tests/test-pager.t --- a/tests/test-pager.t +++ b/tests/test-pager.t @@ -204,3 +204,21 @@ explicit flags work too: 8: a 8 9: a 9 10: a 10 + +Put annotate in the ignore list for pager: + $ cat >> $HGRCPATH < [pager] + > ignore = annotate + > EOF + $ hg blame a + 0: a + 1: a 1 + 2: a 2 + 3: a 3 + 4: a 4 + 5: a 5 + 6: a 6 + 7: a 7 + 8: a 8 + 9: a 9 + 10: a 10 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=CJ1L5Gx6iWAEndbgZQfzQvR8cqQ7iXJ7f9BNw2tH77I=zrhcVF0k6zjVEr8tds1cdSaECURlK-z9TgSyekYiFN8= -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 9 pager] pager: move pager-initiating code into core
e use of interactive functionality, such as
+prompting the user or activating curses.
+
+Args:
+ command: The full, non-aliased name of the command. That is, "log"
+ not "history, "summary" not "summ", etc.
+"""
+if (self.pageractive
+# TODO: if we want to allow HGPLAINEXCEPT=pager,
+# formatted() will need some adjustment.
+or not self.formatted()
+or self.plain()
+# TODO: expose debugger-enabled on the UI object
+or '--debugger' in sys.argv):
+# We only want to paginate if the ui appears to be
+# interactive, the user didn't say HGPLAIN or
+# HGPLAINEXCEPT=pager, and the user didn't specify --debug.
+return
+
+# TODO: add a "system defaults" config section so this default
+# of more(1) can be easily replaced with a global
+# configuration file. For example, on OS X the sane default is
+# less(1), not more(1), and on debian it's
+# sensible-pager(1). We should probably also give the system
+# default editor command similar treatment.
+envpager = encoding.environ.get('PAGER', 'more')
+pagercmd = self.config('pager', 'pager', envpager)
+self.pageractive = True
+# Preserve the formatted-ness of the UI. This is important
+# because we mess with stdout, which might confuse
+# auto-detection of things being formatted.
+self.setconfig('ui', 'formatted', self.formatted(), 'pager')
+self.setconfig('ui', 'interactive', False, 'pager')
The equivalent signal handling change belongs here.
+self._runpager(pagercmd)
+
+def _runpager(self, command):
+"""Actually start the pager and set up file descriptors.
+
+This is separate in part so that extensions (like chg) can
+override how a pager is invoked.
+"""
+pager = subprocess.Popen(command, shell=True, bufsize=-1,
+ close_fds=util.closefds,
stdin=subprocess.PIPE,
+ stdout=util.stdout, stderr=util.stderr)
+
+# back up original file descriptors
+stdoutfd = os.dup(util.stdout.fileno())
+stderrfd = os.dup(util.stderr.fileno())
+
+os.dup2(pager.stdin.fileno(), util.stdout.fileno())
+if self._isatty(util.stderr):
+os.dup2(pager.stdin.fileno(), util.stderr.fileno())
+
+@atexit.register
+def killpager():
+if util.safehasattr(signal, "SIGINT"):
+signal.signal(signal.SIGINT, signal.SIG_IGN)
+# restore original fds, closing pager.stdin copies in the process
+os.dup2(stdoutfd, util.stdout.fileno())
+os.dup2(stderrfd, util.stderr.fileno())
+pager.stdin.close()
+pager.wait()
+
def interface(self, feature):
"""what interface to use for interactive console features?
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=ZiiKlZHpcE6FnMZhBU6-nzVLClXOgHOARg0HpwT43yU=7H11BAlHtU6CGYWE4DAym4-8Q81xpiKmHHJ2JBpLKko=
--
Simon Farnsworth
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 05 of 10 v5] ui: log time spent blocked on stdio
On 15/02/2017 22:06, Simon Farnsworth wrote: diff --git a/tests/test-logtoprocess.t b/tests/test-logtoprocess.t --- a/tests/test-logtoprocess.t +++ b/tests/test-logtoprocess.t @@ -61,10 +61,10 @@ > logtoprocess= > pager= > [logtoprocess] - > uiblocked=echo "\$EVENT command \$OPT_COMMAND_DURATION ms" + > uiblocked=echo "\$EVENT stdio \$OPT_STDIO_BLOCKED ms command \$OPT_COMMAND_DURATION ms" > [ui] > logblockedtimes=True > EOF $ hg log - uiblocked command [0-9]+.[0-9]* ms (re) + uiblocked stdio [0-9]+.[0-9]* ms command [0-9]+.[0-9]* ms (re) This test works for me locally, but the new regex seems to fail on the buildbot: https://buildbot.mercurial-scm.org/builders/hg%20tests/builds/710/steps/run-tests.py%20(python%202.7.10)/logs/stdio - uiblocked stdio [0-9]+.[0-9]* ms command [0-9]+.[0-9]* ms (re) + uiblocked stdio 0.0138282775879 ms command 31.3360691071 ms Can anyone see what I've done wrong? The regex is supposed to remove the times, but clearly doesn't in some cases. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: help: I broke test-gendoc-ro.t and I have no idea how
On 16/02/2017 00:21, Augie Fackler wrote:
With my pager-modernization work applied, I've got one test failure that is
blocking me from mailing:
augie% make test-gendoc-ro.t
cd tests && python run-tests.py -j16 test-gendoc-ro.t
--- /usr/local/google/home/augie/Programming/hg/crew/tests/test-gendoc-ro.t
+++ /usr/local/google/home/augie/Programming/hg/crew/tests/test-gendoc-ro.t.err
@@ -2,3 +2,5 @@
$ $TESTDIR/check-gendoc ro
checking for parse errors
+ gendoc.txt:55: (WARNING/2) Inline interpreted text or phrase reference
start-string without end-string.
+ gendoc.txt:55: (WARNING/2) Inline interpreted text or phrase reference
start-string without end-string.
ERROR: test-gendoc-ro.t output changed
This is caused by something I did in
https://urldefense.proofpoint.com/v2/url?u=https-3A__hg.durin42.com_hg-2Dwip_rev_b7285db7c604=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=ChpAFOjlEYfm475yHPTI0tuFVtHpxkiVGexL5vsOh_I=-92RK09HMha8EYY79jfb7CRI0zekREVBhPPhmvIUYg0=
, but I have no idea what. Can anyone help me figure out this last problem?
It looks to me like a consequence of moving pager into core, combined
with a marginally iffy decision by the Romanian translator.
When pager is an extension, test-gendoc-ro.t will not test the Romanian
translation of _("when to paginate (boolean, always, auto, or never)") -
it's not in the source English text, so won't end up in the translated
output.
You move _("when to paginate (boolean, always, auto, or never)") from
pager.py to commands.py, which results in the source English text
containing that phrase.
The Romanian translator has translated _("when to paginate (boolean,
always, auto, or never)") as "când să se pagineze (boolean,
`always`=întotdeauna, auto, sau `never`=niciodată)". runrst then
attempts to interpret `always`=întotdeauna and `never`=niciodată as
references, and throws up this warning.
I can hunt round the office tomorrow for a friendly Romanian who can
come up with a better way to indicate the relationship between the
Romanian words and the English arguments - or you could decide to accept
this warning for now until the Romanian translation is updated.
--
Simon Farnsworth
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 09 of 10 v5] extdiff: log time spent in external diff program
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1487194446 28800
# Wed Feb 15 13:34:06 2017 -0800
# Node ID c17e5f194dca47819ca2d636a3c9cfdf02733ba7
# Parent 124f329bc78f53abce06a1e8e6244fcdcc551e34
extdiff: log time spent in external diff program
We can't fix the time external diff programs take to run. Log that duration
for us to remove from any stats we gather
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -273,7 +273,7 @@
cmdline = re.sub(regex, quote, cmdline)
ui.debug('running %r in %s\n' % (cmdline, tmproot))
-ui.system(cmdline, cwd=tmproot)
+ui.system(cmdline, cwd=tmproot, blockedtag='extdiff')
for copy_fn, working_fn, mtime in fns_and_mtime:
if os.lstat(copy_fn).st_mtime != mtime:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 10 of 10 v5] histedit: log the time taken to read in the commands list
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1487194446 28800
# Wed Feb 15 13:34:06 2017 -0800
# Node ID b9cf9ffdf15f67b42e87272e2fb328102e8284ba
# Parent c17e5f194dca47819ca2d636a3c9cfdf02733ba7
histedit: log the time taken to read in the commands list
If we're being fed an external command list from stdin (histedit --commands -),
then the time spent reading stdin is outside our control. Log it.
diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -992,7 +992,8 @@
def _readfile(ui, path):
if path == '-':
-return ui.fin.read()
+with ui.timeblockedsection('histedit'):
+return ui.fin.read()
else:
with open(path, 'rb') as f:
return f.read()
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 08 of 10 v5] crecord: log blocked time waiting for curses input
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1487194446 28800
# Wed Feb 15 13:34:06 2017 -0800
# Node ID 124f329bc78f53abce06a1e8e6244fcdcc551e34
# Parent ce6e773a6719fab87fa098021aac72b42709aa33
crecord: log blocked time waiting for curses input
We want to know when we're blocked waiting for the user - log the time spent
waiting in the curses keyboard handlers
diff --git a/mercurial/crecord.py b/mercurial/crecord.py
--- a/mercurial/crecord.py
+++ b/mercurial/crecord.py
@@ -1375,7 +1375,8 @@
pass
helpwin.refresh()
try:
-helpwin.getkey()
+with self.ui.timeblockedsection('crecord'):
+helpwin.getkey()
except curses.error:
pass
@@ -1392,7 +1393,8 @@
self.stdscr.refresh()
confirmwin.refresh()
try:
-response = chr(self.stdscr.getch())
+with self.ui.timeblockedsection('crecord'):
+response = chr(self.stdscr.getch())
except ValueError:
response = None
@@ -1412,7 +1414,8 @@
are you sure you want to review/edit and confirm the selected changes [yn]?
""")
-response = self.confirmationwindow(confirmtext)
+with self.ui.timeblockedsection('crecord'):
+response = self.confirmationwindow(confirmtext)
if response is None:
response = "n"
if response.lower().startswith("y"):
@@ -1655,7 +1658,8 @@
while True:
self.updatescreen()
try:
-keypressed = self.statuswin.getkey()
+with self.ui.timeblockedsection('crecord'):
+keypressed = self.statuswin.getkey()
if self.errorstr is not None:
self.errorstr = None
continue
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 05 of 10 v5] ui: log time spent blocked on stdio
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1487195406 28800
# Wed Feb 15 13:50:06 2017 -0800
# Node ID 1487dd34f44315371738b13519cc4af1c81a7b07
# Parent 4d0b19ca8a56341fe2a77fd243232185ab4bf5e0
ui: log time spent blocked on stdio
We use a wrapper around Mercurial at Facebook that logs key statistics (like
elpased time) to our standard performance tooling.
This is less useful than it could be, because we currently can't tell when a
command is slow because we need to fix Mercurial versus when a command is
slow because the user isn't interacting quickly.
Teach Mercurial to log the time it spends blocked, so that our tooling can
pick it up and submit it with the elapsed time - we can then do the math in
our tooling to see if Mercurial is slow, or if the user simply failed to
interact.
Combining this with the command duration log means that we can ensure that
we concentrate performance efforts on the things that bite Facebook users.
The perfwrite microbenchmark shifts from:
Linux:
! wall 3.213560 comb 0.41 user 0.35 sys 0.06 (best of 4)
Mac:
! wall 0.342325 comb 0.18 user 0.11 sys 0.07 (best of 20)
before this change to:
! wall 3.478070 comb 0.50 user 0.42 sys 0.08 (best of 3)
Mac:
! wall 0.218112 comb 0.22 user 0.15 sys 0.07 (best of 15)
showing a small hit in comb time, but firmly in the noise on wall time.
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -199,6 +199,7 @@
@contextlib.contextmanager
def timeblockedsection(self, key):
+# this is open-coded below - search for timeblockedsection to find them
starttime = util.timer()
try:
yield
@@ -776,31 +777,44 @@
self._buffers[-1].extend(a for a in args)
else:
self._progclear()
-for a in args:
-self.fout.write(a)
+# opencode timeblockedsection because this is a critical path
+starttime = util.timer()
+try:
+for a in args:
+self.fout.write(a)
+finally:
+self._blockedtimes['stdio_blocked'] += \
+(util.timer() - starttime) * 1000
def write_err(self, *args, **opts):
self._progclear()
try:
if self._bufferstates and self._bufferstates[-1][0]:
return self.write(*args, **opts)
-if not getattr(self.fout, 'closed', False):
-self.fout.flush()
-for a in args:
-self.ferr.write(a)
-# stderr may be buffered under win32 when redirected to files,
-# including stdout.
-if not getattr(self.ferr, 'closed', False):
-self.ferr.flush()
+with self.timeblockedsection('stdio'):
+if not getattr(self.fout, 'closed', False):
+self.fout.flush()
+for a in args:
+self.ferr.write(a)
+# stderr may be buffered under win32 when redirected to files,
+# including stdout.
+if not getattr(self.ferr, 'closed', False):
+self.ferr.flush()
except IOError as inst:
if inst.errno not in (errno.EPIPE, errno.EIO, errno.EBADF):
raise
def flush(self):
-try: self.fout.flush()
-except (IOError, ValueError): pass
-try: self.ferr.flush()
-except (IOError, ValueError): pass
+# opencode timeblockedsection because this is a critical path
+starttime = util.timer()
+try:
+try: self.fout.flush()
+except (IOError, ValueError): pass
+try: self.ferr.flush()
+except (IOError, ValueError): pass
+finally:
+self._blockedtimes['stdio_blocked'] += \
+(util.timer() - starttime) * 1000
def _isatty(self, fh):
if self.configbool('ui', 'nontty', False):
@@ -962,7 +976,8 @@
sys.stdout = self.fout
# prompt ' ' must exist; otherwise readline may delete entire line
# - http://bugs.python.org/issue12833
-line = raw_input(' ')
+with self.timeblockedsection('stdio'):
+line = raw_input(' ')
sys.stdin = oldin
sys.stdout = oldout
@@ -1042,13 +1057,14 @@
self.write_err(self.label(prompt or _('password: '), 'ui.prompt'))
# disable getpass() only if explicitly specified. it's still valid
# to interact with tty even if fin is not a tty.
-if self.configbool('ui', 'nontty'):
-l = self.fin.readline()
-if not l:
-raise EOFError
-return l.rstrip('\n')
-else:
-return getpass.getpass('')
+with self.timeblockedsection(
[PATCH 06 of 10 v5] ui: time calls to ui.system
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1487194152 28800
# Wed Feb 15 13:29:12 2017 -0800
# Node ID 1ae8bc5565b680008d82e93bee9455432b6ba0b2
# Parent 1487dd34f44315371738b13519cc4af1c81a7b07
ui: time calls to ui.system
We want to know when we're blocked on ui.system, and why. Allow the user to
supply a tag - otherwise we record on an unspecific tag derived from cmd.
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -35,6 +35,9 @@
urlreq = util.urlreq
+# for use with str.translate(None, _keepalnum), to keep just alphanumerics
+_keepalnum = ''.join(c for c in map(chr, range(256)) if not c.isalnum())
+
samplehgrcs = {
'user':
"""# example user config (see 'hg help config' for more info)
@@ -1146,15 +1149,19 @@
return t
-def system(self, cmd, environ=None, cwd=None, onerr=None, errprefix=None):
+def system(self, cmd, environ=None, cwd=None, onerr=None, errprefix=None,
+ blockedtag=None):
'''execute shell command with appropriate output stream. command
output will be redirected if fout is not stdout.
'''
+if blockedtag is None:
+blockedtag = 'unknown_system_' + cmd.translate(None, _keepalnum)
out = self.fout
if any(s[1] for s in self._bufferstates):
out = self
-return util.system(cmd, environ=environ, cwd=cwd, onerr=onerr,
- errprefix=errprefix, out=out)
+with self.timeblockedsection(blockedtag):
+return util.system(cmd, environ=environ, cwd=cwd, onerr=onerr,
+ errprefix=errprefix, out=out)
def traceback(self, exc=None, force=False):
'''print exception traceback if traceback printing enabled or forced.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 04 of 10 v5] contrib: add a write microbenchmark to perf.py
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1487192846 28800
# Wed Feb 15 13:07:26 2017 -0800
# Node ID 4d0b19ca8a56341fe2a77fd243232185ab4bf5e0
# Parent f3a219226ba0658f72801329d07c1ba516152b70
contrib: add a write microbenchmark to perf.py
I'm adding some performance logging to ui.write - this benchmark lets us
confirm that the cost of that logging is acceptably low.
At this point, the microbenchmark on Linux over SSH shows:
! wall 3.213560 comb 0.41 user 0.35 sys 0.06 (best of 4)
while on the Mac locally, it shows:
! wall 0.342325 comb 0.18 user 0.11 sys 0.07 (best of 20)
diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -1269,6 +1269,17 @@
timer(fn, title=title)
fm.end()
+@command('perfwrite', formatteropts)
+def perfwrite(ui, repo, **opts):
+"""microbenchmark ui.write
+"""
+timer, fm = gettimer(ui, opts)
+def write():
+for i in range(10):
+ui.write(('Testing write performance\n'))
+timer(write)
+fm.end()
+
def uisetup(ui):
if (util.safehasattr(cmdutil, 'openrevlog') and
not util.safehasattr(commands, 'debugrevlogopts')):
diff --git a/tests/test-contrib-perf.t b/tests/test-contrib-perf.t
--- a/tests/test-contrib-perf.t
+++ b/tests/test-contrib-perf.t
@@ -109,6 +109,7 @@
perfvolatilesets
benchmark the computation of various volatile set
perfwalk (no help text available)
+ perfwrite microbenchmark ui.write
(use 'hg help -v perfstatusext' to show built-in aliases and global options)
$ hg perfaddremove
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 03 of 10 v5] ui: provide a mechanism to track and log blocked time
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1487193465 28800
# Wed Feb 15 13:17:45 2017 -0800
# Node ID f3a219226ba0658f72801329d07c1ba516152b70
# Parent 1c71bddbe01e76c1c48b5479ff67d47645afd7b6
ui: provide a mechanism to track and log blocked time
We want to log the time Mercurial spends trapped in things outside
programmatic control. Provide a mechanism to give us both command runtime
and as many different sources of blocking as we deem useful.
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -137,6 +137,9 @@
finally:
duration = util.timer() - starttime
req.ui.flush()
+if req.ui.logblockedtimes:
+req.ui._blockedtimes['command_duration'] = duration * 1000
+req.ui.log('uiblocked', 'ui blocked ms', **req.ui._blockedtimes)
req.ui.log("commandfinish", "%s exited %s after %0.2f seconds\n",
msg, ret or 0, duration)
return ret
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -7,6 +7,7 @@
from __future__ import absolute_import
+import collections
import contextlib
import errno
import getpass
@@ -138,6 +139,8 @@
self.callhooks = True
# Insecure server connections requested.
self.insecureconnections = False
+# Blocked time
+self.logblockedtimes = False
if src:
self.fout = src.fout
@@ -155,6 +158,7 @@
self.fixconfig()
self.httppasswordmgrdb = src.httppasswordmgrdb
+self._blockedtimes = src._blockedtimes
else:
self.fout = util.stdout
self.ferr = util.stderr
@@ -164,6 +168,7 @@
self.environ = encoding.environ
self.httppasswordmgrdb = httppasswordmgrdbproxy()
+self._blockedtimes = collections.defaultdict(int)
allowed = self.configlist('experimental', 'exportableenviron')
if '*' in allowed:
@@ -192,6 +197,15 @@
self._progbar.resetstate() # reset last-print time of progress bar
self.httppasswordmgrdb = httppasswordmgrdbproxy()
+@contextlib.contextmanager
+def timeblockedsection(self, key):
+starttime = util.timer()
+try:
+yield
+finally:
+self._blockedtimes[key + '_blocked'] += \
+(util.timer() - starttime) * 1000
+
def formatter(self, topic, opts):
return formatter.formatter(self, topic, opts)
@@ -295,6 +309,7 @@
self._reportuntrusted = self.debugflag or self.configbool("ui",
"report_untrusted", True)
self.tracebackflag = self.configbool('ui', 'traceback', False)
+self.logblockedtimes = self.configbool('ui', 'logblockedtimes')
if section in (None, 'trusted'):
# update trust information
diff --git a/tests/test-logtoprocess.t b/tests/test-logtoprocess.t
--- a/tests/test-logtoprocess.t
+++ b/tests/test-logtoprocess.t
@@ -10,6 +10,7 @@
> def foo(ui, repo):
> ui.log('foo', 'a message: %(bar)s\n', bar='spam')
> EOF
+ $ cp $HGRCPATH $HGRCPATH.bak
$ cat >> $HGRCPATH << EOF
> [extensions]
> logtoprocess=
@@ -52,3 +53,18 @@
logtoprocess commandfinish output:
logtoprocess foo output:
spam
+
+Confirm that logging blocked time catches stdio properly:
+ $ cp $HGRCPATH.bak $HGRCPATH
+ $ cat >> $HGRCPATH << EOF
+ > [extensions]
+ > logtoprocess=
+ > pager=
+ > [logtoprocess]
+ > uiblocked=echo "\$EVENT command \$OPT_COMMAND_DURATION ms"
+ > [ui]
+ > logblockedtimes=True
+ > EOF
+
+ $ hg log
+ uiblocked command [0-9]+.[0-9]* ms (re)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 02 of 10 v5] mercurial: switch to util.timer for all interval timings
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1487193459 28800
# Wed Feb 15 13:17:39 2017 -0800
# Node ID 1c71bddbe01e76c1c48b5479ff67d47645afd7b6
# Parent 36ad17f00656ef853e0bd7b79e9cd98b58c92a16
mercurial: switch to util.timer for all interval timings
util.timer is now the best available interval timer, at the expense of not
having a known epoch. Let's use it whenever the epoch is irrelevant.
diff --git a/contrib/hgperf b/contrib/hgperf
old mode 100644
new mode 100755
--- a/contrib/hgperf
+++ b/contrib/hgperf
@@ -55,17 +55,15 @@
import mercurial.util
import mercurial.dispatch
-import time
-
def timer(func, title=None):
results = []
-begin = time.time()
+begin = mercurial.util.timer()
count = 0
while True:
ostart = os.times()
-cstart = time.time()
+cstart = mercurial.util.timer()
r = func()
-cstop = time.time()
+cstop = mercurial.util.timer()
ostop = os.times()
count += 1
a, b = ostart, ostop
diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -190,13 +190,13 @@
def _timer(fm, func, title=None):
results = []
-begin = time.time()
+begin = util.timer()
count = 0
while True:
ostart = os.times()
-cstart = time.time()
+cstart = util.timer()
r = func()
-cstop = time.time()
+cstop = util.timer()
ostop = os.times()
count += 1
a, b = ostart, ostop
diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -9,7 +9,6 @@
import array
import struct
-import time
from .node import (
bin,
@@ -21,6 +20,7 @@
encoding,
error,
scmutil,
+util,
)
array = array.array
@@ -261,7 +261,7 @@
missing heads, and a generator of nodes that are strictly a superset of
heads missing, this function updates self to be correct.
"""
-starttime = time.time()
+starttime = util.timer()
cl = repo.changelog
# collect new branch entries
newbranches = {}
@@ -314,7 +314,7 @@
self.tiprev = tiprev
self.filteredhash = scmutil.filteredhash(repo, self.tiprev)
-duration = time.time() - starttime
+duration = util.timer() - starttime
repo.ui.log('branchcache', 'updated %s branch cache in %.4f seconds\n',
repo.filtername, duration)
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -123,7 +123,7 @@
return -1
msg = ' '.join(' ' in a and repr(a) or a for a in req.args)
-starttime = time.time()
+starttime = util.timer()
ret = None
try:
ret = _runcatch(req)
@@ -135,7 +135,7 @@
raise
ret = -1
finally:
-duration = time.time() - starttime
+duration = util.timer() - starttime
req.ui.flush()
req.ui.log("commandfinish", "%s exited %s after %0.2f seconds\n",
msg, ret or 0, duration)
diff --git a/mercurial/hook.py b/mercurial/hook.py
--- a/mercurial/hook.py
+++ b/mercurial/hook.py
@@ -9,7 +9,6 @@
import os
import sys
-import time
from .i18n import _
from . import (
@@ -88,7 +87,7 @@
% (hname, funcname))
ui.note(_("calling hook %s: %s\n") % (hname, funcname))
-starttime = time.time()
+starttime = util.timer()
try:
r = obj(ui=ui, repo=repo, hooktype=name, **args)
@@ -106,7 +105,7 @@
ui.traceback()
return True, True
finally:
-duration = time.time() - starttime
+duration = util.timer() - starttime
ui.log('pythonhook', 'pythonhook-%s: %s finished in %0.2f seconds\n',
name, funcname, duration)
if r:
@@ -118,7 +117,7 @@
def _exthook(ui, repo, name, cmd, args, throw):
ui.note(_("running hook %s: %s\n") % (name, cmd))
-starttime = time.time()
+starttime = util.timer()
env = {}
# make in-memory changes visible to external process
@@ -145,7 +144,7 @@
cwd = pycompat.getcwd()
r = ui.system(cmd, environ=env, cwd=cwd)
-duration = time.time() - starttime
+duration = util.timer() - starttime
ui.log('exthook', 'exthook-%s: %s finished in %0.2f seconds\n',
name, cmd, duration)
if r:
diff --git a/mercurial/profiling.py b/mercurial/profiling.py
--- a/mercurial/profiling.py
+++ b/mercurial/profiling.py
@@ -8,7 +8,6 @@
from __future__ import absolute_import, print_function
import contextlib
-import time
from .i18n import _
from . import (
@@ -66,7 +65,7 @@
collapse_recursion = True
thread = flamegraph.ProfileThread(fp, 1.0 / freq,
filter_, collapse_recursion)
-star
[PATCH 01 of 10 v5] util: introduce timer()
# HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1487188439 28800 # Wed Feb 15 11:53:59 2017 -0800 # Node ID 36ad17f00656ef853e0bd7b79e9cd98b58c92a16 # Parent afaf3c2b129c8940387fd9928ae4fdc28259d13c util: introduce timer() As documented for timeit.default_timer, there are better timers available for performance measures on some platforms. These timers don't have a set epoch, and thus are only useful for interval measurements, but have higher resolution, and thus get you a better measurement overall. Use the same selection logic as Python's timeit.default_timer. This is a platform clock on Python 2 and early Python 3, and time.perf_counter on Python 3.3 and later (where time.perf_counter is introduced as the best timer to use). diff --git a/mercurial/util.py b/mercurial/util.py --- a/mercurial/util.py +++ b/mercurial/util.py @@ -1203,8 +1203,13 @@ if pycompat.osname == 'nt': checkosfilename = checkwinfilename +timer = time.clock else: checkosfilename = platform.checkosfilename +timer = time.time + +if safehasattr(time, "perf_counter"): +timer = time.perf_counter def makelock(info, pathname): try: ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2] lock: include Linux pid namespace identifier in prefix
On 13/02/2017 19:55, Augie Fackler wrote: On Fri, Feb 10, 2017 at 04:50:55PM -0800, Jun Wu wrote: I think if we do know the repo is not on NFS, and the system supports flock(), flock() is way more robust and solves all kinds of pain here. I hereby propose a new repo requirement "flock", once set, use flock instead of the traditional lock file. It's off by default. Thoughts? I'm...not categorically opposed to it, though it feels pretty risky. I know git doesn't use flock() either - presumably there's a good reason I don't know about that neither tool relies on it? The usual reason is the complete clusterfsck that's flock() on NFS, combined with the corporate love of NFS homedirs. Basically, every significant OS (Linux, macOS, FreeBSD) supports a mode where flock() works, but is only respected on the local host (the server is never told about the lock). This is a recipe for silent corruption in places that use NFSv2 or v3, since there's no way to tell whether locks are handled server-side by every client, or if one or more clients are handling locks locally and not telling the server about it. For bonus "excitement", if users have any degree of admin control of their own system, they can get a slight performance boost by switching locking to local-only (as there's no longer one RTT delay for each flock() call). At least, until they forget they did that, and use a second host for a concurrent operation... -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 8 v4] mercurial: use best available timer for perf measurements
On 13/02/2017 18:49, Pulkit Goyal wrote: On Mon, Feb 13, 2017 at 10:59 PM, Simon Farnsworth <simon...@fb.com <mailto:simon...@fb.com>> wrote: # HG changeset patch # User Simon Farnsworth <simon...@fb.com <mailto:simon...@fb.com>> # Date 1486994849 28800 # Mon Feb 13 06:07:29 2017 -0800 # Node ID 88b51cd7e8e3764af542c25d79a33f5cbda37ac6 # Parent a0e3d808690d57d1c9dff840e0b8ee099526397b mercurial: use best available timer for perf measurements As documented for timer.default_timer, there are better timers available for performance measures on some platforms. These timers don't have a set epoch, and thus are only useful for interval measurements, but have higher resolution, and thus get you a better measurement overall. On newer Python (3.3 or later), there's time.perf_counter, which is Looks like you forgot to complete the commit message. Yep - looks like I damaged this while histediting - will fix up. diff --git a/contrib/hgperf b/contrib/hgperf --- a/contrib/hgperf +++ b/contrib/hgperf @@ -55,17 +55,15 @@ import mercurial.util import mercurial.dispatch -import time - def timer(func, title=None): results = [] -begin = time.time() +begin = mercurial.util.timer() count = 0 while True: ostart = os.times() -cstart = time.time() +cstart = mercurial.util.timer() r = func() -cstop = time.time() +cstop = mercurial.mercurial.util.timer() Repeated mercurial here. I guess this is due to some auto completion. Will fix, and make sure the script runs before resubmitting. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 8 v4] ui: provide a mechanism to track and log blocked time
On 13/02/2017 18:06, Bryan O'Sullivan wrote: On Mon, Feb 13, 2017 at 9:29 AM, Simon Farnsworth <simon...@fb.com <mailto:simon...@fb.com>> wrote: +duration = (util.timer() - starttime) * 1000 +key += '_blocked' +self._blockedtimes[key] += duration Could be a one-liner, which will have a tiny but positive impact on perf: self._blockedtimes[key + '_blocked'] += (util.timer() - starttime) * 1000 Will change for the next resend. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 7 of 8 v4] extdiff: log time spent in external diff program
On 13/02/2017 18:04, Bryan O'Sullivan wrote:
On Mon, Feb 13, 2017 at 9:29 AM, Simon Farnsworth <simon...@fb.com
<mailto:simon...@fb.com>> wrote:
+with ui.timeblockedsection('extdiff'):
+ui.system(cmdline, cwd=tmproot)
Why not simply instrument ui.system directly, and leave its callers
untouched?
I want the caller to tell me what tag to apply to this blocking reason,
so that I can account for the blocking in hook.py (where it's running a
non-interactive process) and sshpeer.py (where it's network blocking as
we talk to a server) differently to extdiff.py and filemerge.py (where
it's an interactive process).
I could push the instrumentation into ui.system, conditional on being
passed a tag (with a catch-all for "not tagged") if people think that
would be more useful?
--
Simon Farnsworth
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 8 of 8 v4] histedit: log the time taken to read in the commands list
On 13/02/2017 18:03, Bryan O'Sullivan wrote: On Mon, Feb 13, 2017 at 9:29 AM, Simon Farnsworth <simon...@fb.com <mailto:simon...@fb.com>> wrote: histedit: log the time taken to read in the commands list If we're being fed an external command list (histedit --commands), then the time spent reading that file is out of our control. Log it. I don't think it's worth measuring this, and this patch can reasonably be dropped. I disagree, because the external list can also be fed from stdin. I'll beef up this commit message to make it clearer. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 8 v4] ui: log time spent blocked on stdio
On 13/02/2017 18:00, Bryan O'Sullivan wrote: On Mon, Feb 13, 2017 at 9:29 AM, Simon Farnsworth <simon...@fb.com <mailto:simon...@fb.com>> wrote: The perfwrite microbenchmark shifts to: Linux: ! wall 3.316087 comb 0.90 user 0.81 sys 0.09 (best of 3) Mac: ! wall 0.939282 comb 0.58 user 0.47 sys 0.11 (best of 8) If I open-code timings in ui.write instead of using the context manager, I see: Linux: ! wall 3.478070 comb 0.50 user 0.42 sys 0.08 (best of 3) Mac: ! wall 0.218112 comb 0.22 user 0.15 sys 0.07 (best of 15) Thanks for adding this data. (For future reference, it's good to include an analysis of the difference instead of just the plain numbers, as I have to flip back and forth between two emails and memorize the numbers to do the comparison in my head.) These numbers suggest to me that open-coding the performance measurement is the right thing to do for ui.write and ui.flush. I'll make that change locally, but I'm not going to resubmit for a few days, to give other interested parties time to weigh in. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 8 of 8 v4] histedit: log the time taken to read in the commands list
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486994849 28800
# Mon Feb 13 06:07:29 2017 -0800
# Node ID b7557fd38e32b4d82b992003a242cd2a7c9496c5
# Parent 0727b4d77849fbefbf1ce1de6d9fe22ad2c5e1bd
histedit: log the time taken to read in the commands list
If we're being fed an external command list (histedit --commands), then the
time spent reading that file is out of our control. Log it.
diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -991,11 +991,12 @@
return goalnew
def _readfile(ui, path):
-if path == '-':
-return ui.fin.read()
-else:
-with open(path, 'rb') as f:
-return f.read()
+with ui.timeblockedsection('histedit'):
+if path == '-':
+return ui.fin.read()
+else:
+with open(path, 'rb') as f:
+return f.read()
def _validateargs(ui, repo, state, freeargs, opts, goal, rules, revs):
# TODO only abort if we try to histedit mq patches, not just
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 6 of 8 v4] crecord: log blocked time waiting for curses input
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486994849 28800
# Mon Feb 13 06:07:29 2017 -0800
# Node ID 67a55b66a69520f84552cf3c1a7d93202c3f43da
# Parent 74a43a6466712c42799528a25c4ca74eb2d25919
crecord: log blocked time waiting for curses input
We want to know when we're blocked waiting for the user - log the time spent
waiting in the curses keyboard handlers
diff --git a/mercurial/crecord.py b/mercurial/crecord.py
--- a/mercurial/crecord.py
+++ b/mercurial/crecord.py
@@ -1375,7 +1375,8 @@
pass
helpwin.refresh()
try:
-helpwin.getkey()
+with self.ui.timeblockedsection('crecord'):
+helpwin.getkey()
except curses.error:
pass
@@ -1392,7 +1393,8 @@
self.stdscr.refresh()
confirmwin.refresh()
try:
-response = chr(self.stdscr.getch())
+with self.ui.timeblockedsection('crecord'):
+response = chr(self.stdscr.getch())
except ValueError:
response = None
@@ -1412,7 +1414,8 @@
are you sure you want to review/edit and confirm the selected changes [yn]?
""")
-response = self.confirmationwindow(confirmtext)
+with self.ui.timeblockedsection('crecord'):
+response = self.confirmationwindow(confirmtext)
if response is None:
response = "n"
if response.lower().startswith("y"):
@@ -1655,7 +1658,8 @@
while True:
self.updatescreen()
try:
-keypressed = self.statuswin.getkey()
+with self.ui.timeblockedsection('crecord'):
+keypressed = self.statuswin.getkey()
if self.errorstr is not None:
self.errorstr = None
continue
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 7 of 8 v4] extdiff: log time spent in external diff program
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486994849 28800
# Mon Feb 13 06:07:29 2017 -0800
# Node ID 0727b4d77849fbefbf1ce1de6d9fe22ad2c5e1bd
# Parent 67a55b66a69520f84552cf3c1a7d93202c3f43da
extdiff: log time spent in external diff program
We can't fix the time external diff programs take to run. Log that duration
for us to remove from any stats we gather
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -273,7 +273,8 @@
cmdline = re.sub(regex, quote, cmdline)
ui.debug('running %r in %s\n' % (cmdline, tmproot))
-ui.system(cmdline, cwd=tmproot)
+with ui.timeblockedsection('extdiff'):
+ui.system(cmdline, cwd=tmproot)
for copy_fn, working_fn, mtime in fns_and_mtime:
if os.lstat(copy_fn).st_mtime != mtime:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 5 of 8 v4] ui: log time spent blocked on editor
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486994849 28800
# Mon Feb 13 06:07:29 2017 -0800
# Node ID 74a43a6466712c42799528a25c4ca74eb2d25919
# Parent 923dcef529e523b391e26840cfa9ea6fd53c4131
ui: log time spent blocked on editor
The user's editor is outside our control. Log how long we spend blocked on
it.
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -1075,13 +1075,14 @@
editor = self.geteditor()
-self.system("%s \"%s\"" % (editor, name),
-environ=environ,
-onerr=error.Abort, errprefix=_("edit failed"))
+with self.timeblockedsection('editor'):
+self.system("%s \"%s\"" % (editor, name),
+environ=environ,
+onerr=error.Abort, errprefix=_("edit failed"))
-f = open(name)
-t = f.read()
-f.close()
+f = open(name)
+t = f.read()
+f.close()
finally:
os.unlink(name)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 3 of 8 v4] contrib: add a write microbenchmark to perf.py
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486994849 28800
# Mon Feb 13 06:07:29 2017 -0800
# Node ID 5a595ee2509989e440d70eb0e134fea915a0a8d6
# Parent 00f01e9a24bc050ab5cbbfc5a8dc99e992e31d2b
contrib: add a write microbenchmark to perf.py
I'm adding some performance logging to ui.write - this benchmark lets us
confirm that the cost of that logging is acceptably low.
At this point, the microbenchmark on Linux over SSH shows:
! wall 3.213560 comb 0.41 user 0.35 sys 0.06 (best of 4)
while on the Mac locally, it shows:
! wall 0.342325 comb 0.18 user 0.11 sys 0.07 (best of 20)
diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -1269,6 +1269,17 @@
timer(fn, title=title)
fm.end()
+@command('perfwrite', formatteropts)
+def perfwrite(ui, repo, **opts):
+"""microbenchmark ui.write
+"""
+timer, fm = gettimer(ui, opts)
+def write():
+for i in range(10):
+ui.write(('Testing write performance\n'))
+timer(write)
+fm.end()
+
def uisetup(ui):
if (util.safehasattr(cmdutil, 'openrevlog') and
not util.safehasattr(commands, 'debugrevlogopts')):
diff --git a/tests/test-contrib-perf.t b/tests/test-contrib-perf.t
--- a/tests/test-contrib-perf.t
+++ b/tests/test-contrib-perf.t
@@ -109,6 +109,7 @@
perfvolatilesets
benchmark the computation of various volatile set
perfwalk (no help text available)
+ perfwrite microbenchmark ui.write
(use 'hg help -v perfstatusext' to show built-in aliases and global options)
$ hg perfaddremove
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 4 of 8 v4] ui: log time spent blocked on stdio
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486994849 28800
# Mon Feb 13 06:07:29 2017 -0800
# Node ID 923dcef529e523b391e26840cfa9ea6fd53c4131
# Parent 5a595ee2509989e440d70eb0e134fea915a0a8d6
ui: log time spent blocked on stdio
We use a wrapper around Mercurial at Facebook that logs key statistics (like
elpased time) to our standard performance tooling.
This is less useful than it could be, because we currently can't tell when a
command is slow because we need to fix Mercurial versus when a command is
slow because the user isn't interacting quickly.
Teach Mercurial to log the time it spends blocked, so that our tooling can
pick it up and submit it with the elapsed time - we can then do the math in
our tooling to see if Mercurial is slow, or if the user simply failed to
interact.
Combining this with the command duration log means that we can ensure that
we concentrate performance efforts on the things that bite Facebook users.
The perfwrite microbenchmark shifts to:
Linux:
! wall 3.316087 comb 0.90 user 0.81 sys 0.09 (best of 3)
Mac:
! wall 0.939282 comb 0.58 user 0.47 sys 0.11 (best of 8)
If I open-code timings in ui.write instead of using the context manager, I see:
Linux:
! wall 3.478070 comb 0.50 user 0.42 sys 0.08 (best of 3)
Mac:
! wall 0.218112 comb 0.22 user 0.15 sys 0.07 (best of 15)
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -728,31 +728,34 @@
self._buffers[-1].extend(a for a in args)
else:
self._progclear()
-for a in args:
-self.fout.write(a)
+with self.timeblockedsection('stdio'):
+for a in args:
+self.fout.write(a)
def write_err(self, *args, **opts):
self._progclear()
try:
if self._bufferstates and self._bufferstates[-1][0]:
return self.write(*args, **opts)
-if not getattr(self.fout, 'closed', False):
-self.fout.flush()
-for a in args:
-self.ferr.write(a)
-# stderr may be buffered under win32 when redirected to files,
-# including stdout.
-if not getattr(self.ferr, 'closed', False):
-self.ferr.flush()
+with self.timeblockedsection('stdio'):
+if not getattr(self.fout, 'closed', False):
+self.fout.flush()
+for a in args:
+self.ferr.write(a)
+# stderr may be buffered under win32 when redirected to files,
+# including stdout.
+if not getattr(self.ferr, 'closed', False):
+self.ferr.flush()
except IOError as inst:
if inst.errno not in (errno.EPIPE, errno.EIO, errno.EBADF):
raise
def flush(self):
-try: self.fout.flush()
-except (IOError, ValueError): pass
-try: self.ferr.flush()
-except (IOError, ValueError): pass
+with self.timeblockedsection('stdio'):
+try: self.fout.flush()
+except (IOError, ValueError): pass
+try: self.ferr.flush()
+except (IOError, ValueError): pass
def _isatty(self, fh):
if self.configbool('ui', 'nontty', False):
@@ -914,7 +917,8 @@
sys.stdout = self.fout
# prompt ' ' must exist; otherwise readline may delete entire line
# - http://bugs.python.org/issue12833
-line = raw_input(' ')
+with self.timeblockedsection('stdio'):
+line = raw_input(' ')
sys.stdin = oldin
sys.stdout = oldout
@@ -994,13 +998,14 @@
self.write_err(self.label(prompt or _('password: '), 'ui.prompt'))
# disable getpass() only if explicitly specified. it's still valid
# to interact with tty even if fin is not a tty.
-if self.configbool('ui', 'nontty'):
-l = self.fin.readline()
-if not l:
-raise EOFError
-return l.rstrip('\n')
-else:
-return getpass.getpass('')
+with self.timeblockedsection('stdio'):
+if self.configbool('ui', 'nontty'):
+l = self.fin.readline()
+if not l:
+raise EOFError
+return l.rstrip('\n')
+else:
+return getpass.getpass('')
except EOFError:
raise error.ResponseExpected()
def status(self, *msg, **opts):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 8 v4] ui: provide a mechanism to track and log blocked time
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486994849 28800
# Mon Feb 13 06:07:29 2017 -0800
# Node ID 00f01e9a24bc050ab5cbbfc5a8dc99e992e31d2b
# Parent 88b51cd7e8e3764af542c25d79a33f5cbda37ac6
ui: provide a mechanism to track and log blocked time
We want to log the time Mercurial spends trapped in things outside
programmatic control. Provide a mechanism to give us both command runtime
and as many different sources of blocking as we deem useful
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -137,6 +137,9 @@
finally:
duration = util.timer() - starttime
req.ui.flush()
+if req.ui.logblockedtimes:
+req.ui._blockedtimes['command_duration'] = duration * 1000
+req.ui.log('uiblocked', 'ui blocked ms', **req.ui._blockedtimes)
req.ui.log("commandfinish", "%s exited %s after %0.2f seconds\n",
msg, ret or 0, duration)
return ret
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -7,6 +7,7 @@
from __future__ import absolute_import
+import collections
import contextlib
import errno
import getpass
@@ -120,6 +121,8 @@
self.callhooks = True
# Insecure server connections requested.
self.insecureconnections = False
+# Blocked time
+self.logblockedtimes = False
if src:
self.fout = src.fout
@@ -137,6 +140,7 @@
self.fixconfig()
self.httppasswordmgrdb = src.httppasswordmgrdb
+self._blockedtimes = src._blockedtimes
else:
self.fout = util.stdout
self.ferr = util.stderr
@@ -146,6 +150,7 @@
self.environ = encoding.environ
self.httppasswordmgrdb = urlreq.httppasswordmgrwithdefaultrealm()
+self._blockedtimes = collections.defaultdict(lambda: 0)
allowed = self.configlist('experimental', 'exportableenviron')
if '*' in allowed:
@@ -174,6 +179,14 @@
self._progbar.resetstate() # reset last-print time of progress bar
self.httppasswordmgrdb = urlreq.httppasswordmgrwithdefaultrealm()
+@contextlib.contextmanager
+def timeblockedsection(self, key):
+starttime = util.timer()
+yield
+duration = (util.timer() - starttime) * 1000
+key += '_blocked'
+self._blockedtimes[key] += duration
+
def formatter(self, topic, opts):
return formatter.formatter(self, topic, opts)
@@ -277,6 +290,7 @@
self._reportuntrusted = self.debugflag or self.configbool("ui",
"report_untrusted", True)
self.tracebackflag = self.configbool('ui', 'traceback', False)
+self.logblockedtimes = self.configbool('ui', 'logblockedtimes')
if section in (None, 'trusted'):
# update trust information
diff --git a/tests/test-logtoprocess.t b/tests/test-logtoprocess.t
--- a/tests/test-logtoprocess.t
+++ b/tests/test-logtoprocess.t
@@ -10,6 +10,7 @@
> def foo(ui, repo):
> ui.log('foo', 'a message: %(bar)s\n', bar='spam')
> EOF
+ $ cp $HGRCPATH $HGRCPATH.bak
$ cat >> $HGRCPATH << EOF
> [extensions]
> logtoprocess=
@@ -52,3 +53,18 @@
logtoprocess commandfinish output:
logtoprocess foo output:
spam
+
+Confirm that logging blocked time catches stdio properly:
+ $ cp $HGRCPATH.bak $HGRCPATH
+ $ cat >> $HGRCPATH << EOF
+ > [extensions]
+ > logtoprocess=
+ > pager=
+ > [logtoprocess]
+ > uiblocked=echo "\$EVENT stdio \$OPT_STDIO_BLOCKED ms command
\$OPT_COMMAND_DURATION ms"
+ > [ui]
+ > logblockedtimes=True
+ > EOF
+
+ $ hg log
+ uiblocked stdio [0-9]+.[0-9]* ms command [0-9]+.[0-9]* ms (re)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 8 v4] mercurial: use best available timer for perf measurements
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486994849 28800
# Mon Feb 13 06:07:29 2017 -0800
# Node ID 88b51cd7e8e3764af542c25d79a33f5cbda37ac6
# Parent a0e3d808690d57d1c9dff840e0b8ee099526397b
mercurial: use best available timer for perf measurements
As documented for timer.default_timer, there are better timers available for
performance measures on some platforms. These timers don't have a set epoch,
and thus are only useful for interval measurements, but have higher
resolution, and thus get you a better measurement overall.
On newer Python (3.3 or later), there's time.perf_counter, which is
diff --git a/contrib/hgperf b/contrib/hgperf
--- a/contrib/hgperf
+++ b/contrib/hgperf
@@ -55,17 +55,15 @@
import mercurial.util
import mercurial.dispatch
-import time
-
def timer(func, title=None):
results = []
-begin = time.time()
+begin = mercurial.util.timer()
count = 0
while True:
ostart = os.times()
-cstart = time.time()
+cstart = mercurial.util.timer()
r = func()
-cstop = time.time()
+cstop = mercurial.mercurial.util.timer()
ostop = os.times()
count += 1
a, b = ostart, ostop
diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -190,13 +190,13 @@
def _timer(fm, func, title=None):
results = []
-begin = time.time()
+begin = util.timer()
count = 0
while True:
ostart = os.times()
-cstart = time.time()
+cstart = util.timer()
r = func()
-cstop = time.time()
+cstop = util.timer()
ostop = os.times()
count += 1
a, b = ostart, ostop
diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -9,7 +9,6 @@
import array
import struct
-import time
from .node import (
bin,
@@ -21,6 +20,7 @@
encoding,
error,
scmutil,
+util,
)
array = array.array
@@ -261,7 +261,7 @@
missing heads, and a generator of nodes that are strictly a superset of
heads missing, this function updates self to be correct.
"""
-starttime = time.time()
+starttime = util.timer()
cl = repo.changelog
# collect new branch entries
newbranches = {}
@@ -314,7 +314,7 @@
self.tiprev = tiprev
self.filteredhash = scmutil.filteredhash(repo, self.tiprev)
-duration = time.time() - starttime
+duration = util.timer() - starttime
repo.ui.log('branchcache', 'updated %s branch cache in %.4f seconds\n',
repo.filtername, duration)
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -2203,7 +2203,7 @@
if opts.get('force_lock') or opts.get('force_lock'):
return 0
-now = time.time()
+now = util.timer()
held = 0
def report(vfs, name, method):
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -123,7 +123,7 @@
return -1
msg = ' '.join(' ' in a and repr(a) or a for a in req.args)
-starttime = time.time()
+starttime = util.timer()
ret = None
try:
ret = _runcatch(req)
@@ -135,7 +135,7 @@
raise
ret = -1
finally:
-duration = time.time() - starttime
+duration = util.timer() - starttime
req.ui.flush()
req.ui.log("commandfinish", "%s exited %s after %0.2f seconds\n",
msg, ret or 0, duration)
diff --git a/mercurial/hook.py b/mercurial/hook.py
--- a/mercurial/hook.py
+++ b/mercurial/hook.py
@@ -9,7 +9,6 @@
import os
import sys
-import time
from .i18n import _
from . import (
@@ -88,7 +87,7 @@
% (hname, funcname))
ui.note(_("calling hook %s: %s\n") % (hname, funcname))
-starttime = time.time()
+starttime = util.timer()
try:
r = obj(ui=ui, repo=repo, hooktype=name, **args)
@@ -106,7 +105,7 @@
ui.traceback()
return True, True
finally:
-duration = time.time() - starttime
+duration = util.timer() - starttime
ui.log('pythonhook', 'pythonhook-%s: %s finished in %0.2f seconds\n',
name, funcname, duration)
if r:
@@ -118,7 +117,7 @@
def _exthook(ui, repo, name, cmd, args, throw):
ui.note(_("running hook %s: %s\n") % (name, cmd))
-starttime = time.time()
+starttime = util.timer()
env = {}
# make in-memory changes visible to external process
@@ -145,7 +144,7 @@
cwd = pycompat.getcwd()
r = ui.system(cmd, environ=env, cwd=cwd)
-duration = time.time() - starttime
+duration = util.timer() - starttime
ui.log('exthook', 'exthook-%s: %s finished in %0.2f secon
Re: [PATCH 1 of 8 v3] mercurial: use timeit.default_timer for interval measurement
On 11/02/2017 23:40, Bryan O'Sullivan wrote:
On Fri, Feb 10, 2017 at 1:06 PM, Simon Farnsworth <simon...@fb.com
<mailto:simon...@fb.com>> wrote:
# Parent 1f51b4658f21bbb797e922d155c1046eddccf91d
mercurial: use timeit.default_timer for interval measurement
In Python 2.6 and later, timeit.default_timer() provides the highest
resolution timer for profiling and performance measurement, but
without a
specified epoch (on some platforms, epoch is Python start time).
Switch interval measures from time.time() to timeit.default_timer() to
exploit this.
There's a fair bit of unused code in timeit that is unnecessary to pull
in, and loading modules has a cost.
Here's a better version:
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1203,8 +1203,10 @@ def checkwinfilename(path):
if pycompat.osname == 'nt':
checkosfilename = checkwinfilename
+timer = time.clock
else:
checkosfilename = platform.checkosfilename
+timer = time.time
It ends up more complex than that, because Python 3.3 introduces
time.perf_counter(), which ties into platform specific performance
counters where available to get a very high resolution clock.
Something like:
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1203,8 +1203,12 @@ def checkwinfilename(path):
if pycompat.osname == 'nt':
checkosfilename = checkwinfilename
+timer = time.clock
else:
checkosfilename = platform.checkosfilename
+timer = time.time
+if util.safehasattr(time, perf_counter):
+timer = time.perf_counter
def makelock(info, pathname):
try:
--
Simon Farnsworth
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 6 of 8 v3] crecord: log blocked time waiting for curses input
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486760569 28800
# Fri Feb 10 13:02:49 2017 -0800
# Node ID 7650c3dee1cab050f3835ae95590636473b58874
# Parent 8e20c41fe4903745b668ae3de58a0d3e9514a78e
crecord: log blocked time waiting for curses input
We want to know when we're blocked waiting for the user - log the time spent
waiting in the curses keyboard handlers
diff --git a/mercurial/crecord.py b/mercurial/crecord.py
--- a/mercurial/crecord.py
+++ b/mercurial/crecord.py
@@ -1375,7 +1375,8 @@
pass
helpwin.refresh()
try:
-helpwin.getkey()
+with self.ui.timeblockedsection('crecord'):
+helpwin.getkey()
except curses.error:
pass
@@ -1392,7 +1393,8 @@
self.stdscr.refresh()
confirmwin.refresh()
try:
-response = chr(self.stdscr.getch())
+with self.ui.timeblockedsection('crecord'):
+response = chr(self.stdscr.getch())
except ValueError:
response = None
@@ -1412,7 +1414,8 @@
are you sure you want to review/edit and confirm the selected changes [yn]?
""")
-response = self.confirmationwindow(confirmtext)
+with self.ui.timeblockedsection('crecord'):
+response = self.confirmationwindow(confirmtext)
if response is None:
response = "n"
if response.lower().startswith("y"):
@@ -1655,7 +1658,8 @@
while True:
self.updatescreen()
try:
-keypressed = self.statuswin.getkey()
+with self.ui.timeblockedsection('crecord'):
+keypressed = self.statuswin.getkey()
if self.errorstr is not None:
self.errorstr = None
continue
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 4 of 8 v3] ui: log time spent blocked on stdio
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486760435 28800
# Fri Feb 10 13:00:35 2017 -0800
# Node ID dbe4c89f86d75130fabcde8ac2f4ce18ef6d356e
# Parent 838c119320b457e9dfcc45d3d8a50a48d5ea5243
ui: log time spent blocked on stdio
We use a wrapper around Mercurial at Facebook that logs key statistics (like
elpased time) to our standard performance tooling.
This is less useful than it could be, because we currently can't tell when a
command is slow because we need to fix Mercurial versus when a command is
slow because the user isn't interacting quickly.
Teach Mercurial to log the time it spends blocked, so that our tooling can
pick it up and submit it with the elapsed time - we can then do the math in
our tooling to see if Mercurial is slow, or if the user simply failed to
interact.
Combining this with the command duration log means that we can ensure that
we concentrate performance efforts on the things that bite Facebook users.
The perf microbenchmark regresses marginally on Linux:
! wall 0.07 comb 0.00 user 0.00 sys 0.00 (best of 87366)
However, on my Mac, the timing remains at:
! wall 0.00 comb 0.00 user 0.00 sys 0.00 (best of 190351)
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -729,31 +729,34 @@
self._buffers[-1].extend(a for a in args)
else:
self._progclear()
-for a in args:
-self.fout.write(a)
+with self.timeblockedsection('stdio'):
+for a in args:
+self.fout.write(a)
def write_err(self, *args, **opts):
self._progclear()
try:
if self._bufferstates and self._bufferstates[-1][0]:
return self.write(*args, **opts)
-if not getattr(self.fout, 'closed', False):
-self.fout.flush()
-for a in args:
-self.ferr.write(a)
-# stderr may be buffered under win32 when redirected to files,
-# including stdout.
-if not getattr(self.ferr, 'closed', False):
-self.ferr.flush()
+with self.timeblockedsection('stdio'):
+if not getattr(self.fout, 'closed', False):
+self.fout.flush()
+for a in args:
+self.ferr.write(a)
+# stderr may be buffered under win32 when redirected to files,
+# including stdout.
+if not getattr(self.ferr, 'closed', False):
+self.ferr.flush()
except IOError as inst:
if inst.errno not in (errno.EPIPE, errno.EIO, errno.EBADF):
raise
def flush(self):
-try: self.fout.flush()
-except (IOError, ValueError): pass
-try: self.ferr.flush()
-except (IOError, ValueError): pass
+with self.timeblockedsection('stdio'):
+try: self.fout.flush()
+except (IOError, ValueError): pass
+try: self.ferr.flush()
+except (IOError, ValueError): pass
def _isatty(self, fh):
if self.configbool('ui', 'nontty', False):
@@ -915,7 +918,8 @@
sys.stdout = self.fout
# prompt ' ' must exist; otherwise readline may delete entire line
# - http://bugs.python.org/issue12833
-line = raw_input(' ')
+with self.timeblockedsection('stdio'):
+line = raw_input(' ')
sys.stdin = oldin
sys.stdout = oldout
@@ -995,13 +999,14 @@
self.write_err(self.label(prompt or _('password: '), 'ui.prompt'))
# disable getpass() only if explicitly specified. it's still valid
# to interact with tty even if fin is not a tty.
-if self.configbool('ui', 'nontty'):
-l = self.fin.readline()
-if not l:
-raise EOFError
-return l.rstrip('\n')
-else:
-return getpass.getpass('')
+with self.timeblockedsection('stdio'):
+if self.configbool('ui', 'nontty'):
+l = self.fin.readline()
+if not l:
+raise EOFError
+return l.rstrip('\n')
+else:
+return getpass.getpass('')
except EOFError:
raise error.ResponseExpected()
def status(self, *msg, **opts):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 3 of 8 v3] contrib: add a write microbenchmark to perf.py
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486760403 28800
# Fri Feb 10 13:00:03 2017 -0800
# Node ID 838c119320b457e9dfcc45d3d8a50a48d5ea5243
# Parent 25b4512a095b2f3788b255274a15d68dbc10f7b4
contrib: add a write microbenchmark to perf.py
I'm adding some performance logging to ui.write - this benchmark lets us
confirm that the cost of that logging is acceptably low.
At this point, the microbenchmark on Linux over SSH shows:
! wall 0.03 comb 0.00 user 0.00 sys 0.00 (best of 82813)
while on the Mac locally, it shows:
! wall 0.00 comb 0.00 user 0.00 sys 0.00 (best of 318339)
diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -1257,6 +1257,16 @@
timer(fn, title=title)
fm.end()
+@command('perfwrite', formatteropts)
+def perfwrite(ui, repo, **opts):
+"""microbenchmark ui.write
+"""
+timer, fm = gettimer(ui, opts)
+def write():
+ui.write(('Testing write performance\n'))
+timer(write)
+fm.end()
+
def uisetup(ui):
if (util.safehasattr(cmdutil, 'openrevlog') and
not util.safehasattr(commands, 'debugrevlogopts')):
diff --git a/tests/test-contrib-perf.t b/tests/test-contrib-perf.t
--- a/tests/test-contrib-perf.t
+++ b/tests/test-contrib-perf.t
@@ -109,6 +109,7 @@
perfvolatilesets
benchmark the computation of various volatile set
perfwalk (no help text available)
+ perfwrite microbenchmark ui.write
(use 'hg help -v perfstatusext' to show built-in aliases and global options)
$ hg perfaddremove
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 5 of 8 v3] ui: log time spent blocked on editor
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486760569 28800
# Fri Feb 10 13:02:49 2017 -0800
# Node ID 8e20c41fe4903745b668ae3de58a0d3e9514a78e
# Parent dbe4c89f86d75130fabcde8ac2f4ce18ef6d356e
ui: log time spent blocked on editor
The user's editor is outside our control. Log how long we spend blocked on
it.
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -1076,13 +1076,14 @@
editor = self.geteditor()
-self.system("%s \"%s\"" % (editor, name),
-environ=environ,
-onerr=error.Abort, errprefix=_("edit failed"))
+with self.timeblockedsection('editor'):
+self.system("%s \"%s\"" % (editor, name),
+environ=environ,
+onerr=error.Abort, errprefix=_("edit failed"))
-f = open(name)
-t = f.read()
-f.close()
+f = open(name)
+t = f.read()
+f.close()
finally:
os.unlink(name)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 8 v3] ui: provide a mechanism to track and log blocked time
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486757727 28800
# Fri Feb 10 12:15:27 2017 -0800
# Node ID 25b4512a095b2f3788b255274a15d68dbc10f7b4
# Parent bd40abfb4e5dbaf6e373636c8e7bab7a6bf34e17
ui: provide a mechanism to track and log blocked time
We want to log the time Mercurial spends trapped in things outside
programmatic control. Provide a mechanism to give us both command runtime
and as many different sources of blocking as we deem useful
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -138,6 +138,9 @@
finally:
duration = timeit.default_timer() - starttime
req.ui.flush()
+if req.ui.logblockedtimes:
+req.ui._blockedtimes['command_duration'] = duration * 1000
+req.ui.log('uiblocked', 'ui blocked ms', **req.ui._blockedtimes)
req.ui.log("commandfinish", "%s exited %s after %0.2f seconds\n",
msg, ret or 0, duration)
return ret
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -7,6 +7,7 @@
from __future__ import absolute_import
+import collections
import contextlib
import errno
import getpass
@@ -16,6 +17,7 @@
import socket
import sys
import tempfile
+import timeit
import traceback
from .i18n import _
@@ -120,6 +122,8 @@
self.callhooks = True
# Insecure server connections requested.
self.insecureconnections = False
+# Blocked time
+self.logblockedtimes = False
if src:
self.fout = src.fout
@@ -137,6 +141,7 @@
self.fixconfig()
self.httppasswordmgrdb = src.httppasswordmgrdb
+self._blockedtimes = src._blockedtimes
else:
self.fout = util.stdout
self.ferr = util.stderr
@@ -146,6 +151,7 @@
self.environ = encoding.environ
self.httppasswordmgrdb = urlreq.httppasswordmgrwithdefaultrealm()
+self._blockedtimes = collections.defaultdict(lambda: 0)
allowed = self.configlist('experimental', 'exportableenviron')
if '*' in allowed:
@@ -174,6 +180,14 @@
self._progbar.resetstate() # reset last-print time of progress bar
self.httppasswordmgrdb = urlreq.httppasswordmgrwithdefaultrealm()
+@contextlib.contextmanager
+def timeblockedsection(self, key):
+starttime = timeit.default_timer()
+yield
+duration = (timeit.default_timer() - starttime) * 1000
+key += '_blocked'
+self._blockedtimes[key] += duration
+
def formatter(self, topic, opts):
return formatter.formatter(self, topic, opts)
@@ -277,6 +291,7 @@
self._reportuntrusted = self.debugflag or self.configbool("ui",
"report_untrusted", True)
self.tracebackflag = self.configbool('ui', 'traceback', False)
+self.logblockedtimes = self.configbool('ui', 'logblockedtimes')
if section in (None, 'trusted'):
# update trust information
diff --git a/tests/test-logtoprocess.t b/tests/test-logtoprocess.t
--- a/tests/test-logtoprocess.t
+++ b/tests/test-logtoprocess.t
@@ -10,6 +10,7 @@
> def foo(ui, repo):
> ui.log('foo', 'a message: %(bar)s\n', bar='spam')
> EOF
+ $ cp $HGRCPATH $HGRCPATH.bak
$ cat >> $HGRCPATH << EOF
> [extensions]
> logtoprocess=
@@ -52,3 +53,18 @@
logtoprocess commandfinish output:
logtoprocess foo output:
spam
+
+Confirm that logging blocked time catches stdio properly:
+ $ cp $HGRCPATH.bak $HGRCPATH
+ $ cat >> $HGRCPATH << EOF
+ > [extensions]
+ > logtoprocess=
+ > pager=
+ > [logtoprocess]
+ > uiblocked=echo "\$EVENT stdio \$OPT_STDIO_BLOCKED ms command
\$OPT_COMMAND_DURATION ms"
+ > [ui]
+ > logblockedtimes=True
+ > EOF
+
+ $ hg log
+ uiblocked stdio * ms command * ms (glob)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 8 v3] mercurial: use timeit.default_timer for interval measurement
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486748338 28800
# Fri Feb 10 09:38:58 2017 -0800
# Node ID bd40abfb4e5dbaf6e373636c8e7bab7a6bf34e17
# Parent 1f51b4658f21bbb797e922d155c1046eddccf91d
mercurial: use timeit.default_timer for interval measurement
In Python 2.6 and later, timeit.default_timer() provides the highest
resolution timer for profiling and performance measurement, but without a
specified epoch (on some platforms, epoch is Python start time).
Switch interval measures from time.time() to timeit.default_timer() to
exploit this.
diff --git a/contrib/hgperf b/contrib/hgperf
--- a/contrib/hgperf
+++ b/contrib/hgperf
@@ -55,17 +55,17 @@
import mercurial.util
import mercurial.dispatch
-import time
+import timeit
def timer(func, title=None):
results = []
-begin = time.time()
+begin = timeit.default_timer()
count = 0
while True:
ostart = os.times()
-cstart = time.time()
+cstart = timeit.default_timer()
r = func()
-cstop = time.time()
+cstop = timeit.default_timer()
ostop = os.times()
count += 1
a, b = ostart, ostop
diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -24,6 +24,7 @@
import random
import sys
import time
+import timeit
from mercurial import (
bdiff,
changegroup,
@@ -190,13 +191,13 @@
def _timer(fm, func, title=None):
results = []
-begin = time.time()
+begin = timeit.default_timer()
count = 0
while True:
ostart = os.times()
-cstart = time.time()
+cstart = timeit.default_timer()
r = func()
-cstop = time.time()
+cstop = timeit.default_timer()
ostop = os.times()
count += 1
a, b = ostart, ostop
diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -9,7 +9,7 @@
import array
import struct
-import time
+import timeit
from .node import (
bin,
@@ -261,7 +261,7 @@
missing heads, and a generator of nodes that are strictly a superset of
heads missing, this function updates self to be correct.
"""
-starttime = time.time()
+starttime = timeit.default_timer()
cl = repo.changelog
# collect new branch entries
newbranches = {}
@@ -314,7 +314,7 @@
self.tiprev = tiprev
self.filteredhash = scmutil.filteredhash(repo, self.tiprev)
-duration = time.time() - starttime
+duration = timeit.default_timer() - starttime
repo.ui.log('branchcache', 'updated %s branch cache in %.4f seconds\n',
repo.filtername, duration)
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -16,6 +16,7 @@
import sys
import tempfile
import time
+import timeit
from .i18n import _
from .node import (
@@ -2202,7 +2203,7 @@
if opts.get('force_lock') or opts.get('force_lock'):
return 0
-now = time.time()
+now = timeit.default_timer()
held = 0
def report(vfs, name, method):
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -17,6 +17,7 @@
import signal
import sys
import time
+import timeit
import traceback
@@ -123,7 +124,7 @@
return -1
msg = ' '.join(' ' in a and repr(a) or a for a in req.args)
-starttime = time.time()
+starttime = timeit.default_timer()
ret = None
try:
ret = _runcatch(req)
@@ -135,7 +136,7 @@
raise
ret = -1
finally:
-duration = time.time() - starttime
+duration = timeit.default_timer() - starttime
req.ui.flush()
req.ui.log("commandfinish", "%s exited %s after %0.2f seconds\n",
msg, ret or 0, duration)
diff --git a/mercurial/hook.py b/mercurial/hook.py
--- a/mercurial/hook.py
+++ b/mercurial/hook.py
@@ -9,7 +9,7 @@
import os
import sys
-import time
+import timeit
from .i18n import _
from . import (
@@ -88,7 +88,7 @@
% (hname, funcname))
ui.note(_("calling hook %s: %s\n") % (hname, funcname))
-starttime = time.time()
+starttime = timeit.default_timer()
try:
r = obj(ui=ui, repo=repo, hooktype=name, **args)
@@ -106,7 +106,7 @@
ui.traceback()
return True, True
finally:
-duration = time.time() - starttime
+duration = timeit.default_timer() - starttime
ui.log('pythonhook', 'pythonhook-%s: %s finished in %0.2f seconds\n',
name, funcname, duration)
if r:
@@ -118,7 +118,7 @@
def _exthook(ui, repo, name, cmd, args, throw):
ui.note(_("running hook %s: %s\n") % (name, cmd))
-starttime = time.time()
+starttime =
[PATCH 5 of 6 v2] extdiff: log time spent in external diff program
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486676255 28800
# Thu Feb 09 13:37:35 2017 -0800
# Node ID acb1103ff1de3c9c40a4b1c6b19ca65161329d02
# Parent ff695f191c30036c1171d8509f55ec60d19e0614
extdiff: log time spent in external diff program
We can't fix the time external diff programs take to run. Log that duration
for us to remove from any stats we gather
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -273,7 +273,8 @@
cmdline = re.sub(regex, quote, cmdline)
ui.debug('running %r in %s\n' % (cmdline, tmproot))
-ui.system(cmdline, cwd=tmproot)
+with ui.timeblockedsection('extdiff'):
+ui.system(cmdline, cwd=tmproot)
for copy_fn, working_fn, mtime in fns_and_mtime:
if os.lstat(copy_fn).st_mtime != mtime:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 6 v2] ui: log time spent blocked on stdio
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486676409 28800
# Thu Feb 09 13:40:09 2017 -0800
# Node ID be7dac985b12af413b43fa9685e29a90f80de4ef
# Parent ec1dac3c0d5fc7d0d15af324b345958a41000960
ui: log time spent blocked on stdio
We use a wrapper around Mercurial at Facebook that logs key statistics (like
elpased time) to our standard performance tooling.
This is less useful than it could be, because we currently can't tell when a
command is slow because we need to fix Mercurial versus when a command is
slow because the user isn't interacting quickly.
Teach Mercurial to log the time it spends blocked, so that our tooling can
pick it up and submit it with the elapsed time - we can then do the math in
our tooling to see if Mercurial is slow, or if the user simply failed to
interact.
Combining this with the command duration log means that we can ensure that
we concentrate performance efforts on the things that bite Facebook users.
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -728,31 +728,34 @@
self._buffers[-1].extend(a for a in args)
else:
self._progclear()
-for a in args:
-self.fout.write(a)
+with self.timeblockedsection('stdio'):
+for a in args:
+self.fout.write(a)
def write_err(self, *args, **opts):
self._progclear()
try:
if self._bufferstates and self._bufferstates[-1][0]:
return self.write(*args, **opts)
-if not getattr(self.fout, 'closed', False):
-self.fout.flush()
-for a in args:
-self.ferr.write(a)
-# stderr may be buffered under win32 when redirected to files,
-# including stdout.
-if not getattr(self.ferr, 'closed', False):
-self.ferr.flush()
+with self.timeblockedsection('stdio'):
+if not getattr(self.fout, 'closed', False):
+self.fout.flush()
+for a in args:
+self.ferr.write(a)
+# stderr may be buffered under win32 when redirected to files,
+# including stdout.
+if not getattr(self.ferr, 'closed', False):
+self.ferr.flush()
except IOError as inst:
if inst.errno not in (errno.EPIPE, errno.EIO, errno.EBADF):
raise
def flush(self):
-try: self.fout.flush()
-except (IOError, ValueError): pass
-try: self.ferr.flush()
-except (IOError, ValueError): pass
+with self.timeblockedsection('stdio'):
+try: self.fout.flush()
+except (IOError, ValueError): pass
+try: self.ferr.flush()
+except (IOError, ValueError): pass
def _isatty(self, fh):
if self.configbool('ui', 'nontty', False):
@@ -914,7 +917,8 @@
sys.stdout = self.fout
# prompt ' ' must exist; otherwise readline may delete entire line
# - http://bugs.python.org/issue12833
-line = raw_input(' ')
+with self.timeblockedsection('stdio'):
+line = raw_input(' ')
sys.stdin = oldin
sys.stdout = oldout
@@ -994,13 +998,14 @@
self.write_err(self.label(prompt or _('password: '), 'ui.prompt'))
# disable getpass() only if explicitly specified. it's still valid
# to interact with tty even if fin is not a tty.
-if self.configbool('ui', 'nontty'):
-l = self.fin.readline()
-if not l:
-raise EOFError
-return l.rstrip('\n')
-else:
-return getpass.getpass('')
+with self.timeblockedsection('stdio'):
+if self.configbool('ui', 'nontty'):
+l = self.fin.readline()
+if not l:
+raise EOFError
+return l.rstrip('\n')
+else:
+return getpass.getpass('')
except EOFError:
raise error.ResponseExpected()
def status(self, *msg, **opts):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 3 of 6 v2] ui: log time spent blocked on editor
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486676443 28800
# Thu Feb 09 13:40:43 2017 -0800
# Node ID e21b9166fc2cdcda22a071de30f97b44c5796da1
# Parent be7dac985b12af413b43fa9685e29a90f80de4ef
ui: log time spent blocked on editor
The user's editor is outside our control. Log how long we spend blocked on
it.
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -1075,13 +1075,14 @@
editor = self.geteditor()
-self.system("%s \"%s\"" % (editor, name),
-environ=environ,
-onerr=error.Abort, errprefix=_("edit failed"))
+with self.timeblockedsection('editor'):
+self.system("%s \"%s\"" % (editor, name),
+environ=environ,
+onerr=error.Abort, errprefix=_("edit failed"))
-f = open(name)
-t = f.read()
-f.close()
+f = open(name)
+t = f.read()
+f.close()
finally:
os.unlink(name)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 6 of 6 v2] histedit: log the time taken to read in the commands list
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486676306 28800
# Thu Feb 09 13:38:26 2017 -0800
# Node ID f8b12c890147ebce0c06663f478cd76b89f6beab
# Parent acb1103ff1de3c9c40a4b1c6b19ca65161329d02
histedit: log the time taken to read in the commands list
If we're being fed an external command list (histedit --commands), then the
time spent reading that file is out of our control. Log it.
diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -991,11 +991,12 @@
return goalnew
def _readfile(ui, path):
-if path == '-':
-return ui.fin.read()
-else:
-with open(path, 'rb') as f:
-return f.read()
+with ui.timeblockedsection('histedit'):
+if path == '-':
+return ui.fin.read()
+else:
+with open(path, 'rb') as f:
+return f.read()
def _validateargs(ui, repo, state, freeargs, opts, goal, rules, revs):
# TODO only abort if we try to histedit mq patches, not just
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 6 v2] ui: provide a mechanism to track and log blocked time
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486675708 28800
# Thu Feb 09 13:28:28 2017 -0800
# Node ID ec1dac3c0d5fc7d0d15af324b345958a41000960
# Parent 1f51b4658f21bbb797e922d155c1046eddccf91d
ui: provide a mechanism to track and log blocked time
We want to log the time Mercurial spends trapped in things outside
programmatic control. Provide a mechanism to give us both command runtime
and as many different sources of blocking as we deem useful
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -17,6 +17,7 @@
import signal
import sys
import time
+import timeit
import traceback
@@ -123,7 +124,7 @@
return -1
msg = ' '.join(' ' in a and repr(a) or a for a in req.args)
-starttime = time.time()
+starttime = timeit.default_timer()
ret = None
try:
ret = _runcatch(req)
@@ -135,8 +136,11 @@
raise
ret = -1
finally:
-duration = time.time() - starttime
+duration = timeit.default_timer() - starttime
req.ui.flush()
+if req.ui.logblockedtimes:
+req.ui._blockedtimes['command_duration'] = duration * 1000
+req.ui.log('uiblocked', 'ui blocked ms', **req.ui._blockedtimes)
req.ui.log("commandfinish", "%s exited %s after %0.2f seconds\n",
msg, ret or 0, duration)
return ret
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -16,6 +16,7 @@
import socket
import sys
import tempfile
+import timeit
import traceback
from .i18n import _
@@ -120,6 +121,8 @@
self.callhooks = True
# Insecure server connections requested.
self.insecureconnections = False
+# Blocked time
+self.logblockedtimes = False
if src:
self.fout = src.fout
@@ -137,6 +140,7 @@
self.fixconfig()
self.httppasswordmgrdb = src.httppasswordmgrdb
+self._blockedtimes = src._blockedtimes
else:
self.fout = util.stdout
self.ferr = util.stderr
@@ -146,6 +150,7 @@
self.environ = encoding.environ
self.httppasswordmgrdb = urlreq.httppasswordmgrwithdefaultrealm()
+self._blockedtimes = {}
allowed = self.configlist('experimental', 'exportableenviron')
if '*' in allowed:
@@ -174,6 +179,14 @@
self._progbar.resetstate() # reset last-print time of progress bar
self.httppasswordmgrdb = urlreq.httppasswordmgrwithdefaultrealm()
+@contextlib.contextmanager
+def timeblockedsection(self, key):
+starttime = timeit.default_timer()
+yield
+duration = (timeit.default_timer() - starttime) * 1000
+key += '_blocked'
+self._blockedtimes[key] = self._blockedtimes.get(key, 0) + duration
+
def formatter(self, topic, opts):
return formatter.formatter(self, topic, opts)
@@ -277,6 +290,7 @@
self._reportuntrusted = self.debugflag or self.configbool("ui",
"report_untrusted", True)
self.tracebackflag = self.configbool('ui', 'traceback', False)
+self.logblockedtimes = self.configbool('ui', 'logblockedtimes')
if section in (None, 'trusted'):
# update trust information
diff --git a/tests/test-logtoprocess.t b/tests/test-logtoprocess.t
--- a/tests/test-logtoprocess.t
+++ b/tests/test-logtoprocess.t
@@ -10,6 +10,7 @@
> def foo(ui, repo):
> ui.log('foo', 'a message: %(bar)s\n', bar='spam')
> EOF
+ $ cp $HGRCPATH $HGRCPATH.bak
$ cat >> $HGRCPATH << EOF
> [extensions]
> logtoprocess=
@@ -52,3 +53,18 @@
logtoprocess commandfinish output:
logtoprocess foo output:
spam
+
+Confirm that logging blocked time catches stdio properly:
+ $ cp $HGRCPATH.bak $HGRCPATH
+ $ cat >> $HGRCPATH << EOF
+ > [extensions]
+ > logtoprocess=
+ > pager=
+ > [logtoprocess]
+ > uiblocked=echo "\$EVENT stdio \$OPT_STDIO_BLOCKED ms command
\$OPT_COMMAND_DURATION ms"
+ > [ui]
+ > logblockedtimes=True
+ > EOF
+
+ $ hg log
+ uiblocked stdio * ms command * ms (glob)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 4 of 6 v2] crecord: log blocked time waiting for curses input
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486676200 28800
# Thu Feb 09 13:36:40 2017 -0800
# Node ID ff695f191c30036c1171d8509f55ec60d19e0614
# Parent e21b9166fc2cdcda22a071de30f97b44c5796da1
crecord: log blocked time waiting for curses input
We want to know when we're blocked waiting for the user - log the time spent
waiting in the curses keyboard handlers
diff --git a/mercurial/crecord.py b/mercurial/crecord.py
--- a/mercurial/crecord.py
+++ b/mercurial/crecord.py
@@ -1375,7 +1375,8 @@
pass
helpwin.refresh()
try:
-helpwin.getkey()
+with self.ui.timeblockedsection('crecord'):
+helpwin.getkey()
except curses.error:
pass
@@ -1392,7 +1393,8 @@
self.stdscr.refresh()
confirmwin.refresh()
try:
-response = chr(self.stdscr.getch())
+with self.ui.timeblockedsection('crecord'):
+response = chr(self.stdscr.getch())
except ValueError:
response = None
@@ -1412,7 +1414,8 @@
are you sure you want to review/edit and confirm the selected changes [yn]?
""")
-response = self.confirmationwindow(confirmtext)
+with self.ui.timeblockedsection('crecord'):
+response = self.confirmationwindow(confirmtext)
if response is None:
response = "n"
if response.lower().startswith("y"):
@@ -1655,7 +1658,8 @@
while True:
self.updatescreen()
try:
-keypressed = self.statuswin.getkey()
+with self.ui.timeblockedsection('crecord'):
+keypressed = self.statuswin.getkey()
if self.errorstr is not None:
self.errorstr = None
continue
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] zeroconf: fail nicely on IPv6 only system
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486570121 28800
# Wed Feb 08 08:08:41 2017 -0800
# Node ID a847eb00fcfeffce458e11b80ad38d4d4e7a700f
# Parent d50cda2a403786836d1f0d5c99401599dc4f43ec
zeroconf: fail nicely on IPv6 only system
zeroconf only knows how to deal with IPv4; I develop on a system where the only
IPv4 address is 127.0.0.1.
Teach zeroconf to ignore IPv6 addresses when looking for plausible IPv4
connectivity.
diff --git a/hgext/zeroconf/__init__.py b/hgext/zeroconf/__init__.py
--- a/hgext/zeroconf/__init__.py
+++ b/hgext/zeroconf/__init__.py
@@ -64,7 +64,9 @@
# Generic method, sometimes gives useless results
try:
dumbip = socket.gethostbyaddr(socket.gethostname())[2][0]
-if not dumbip.startswith('127.') and ':' not in dumbip:
+if ':' in dumbip:
+dumbip = '127.0.0.1'
+if not dumbip.startswith('127.'):
return dumbip
except (socket.gaierror, socket.herror):
dumbip = '127.0.0.1'
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] pager: exit cleanly on SIGPIPE (BC)
This replaces "dispatch: treat SIGPIPE as a termination signal (BC)".
I'm now debugging test-paths.t, so that I can stop being acclimatised to
the red failure prompt after running tests, to improve my chances of
noticing this sort of issue.
Simon
On 08/02/2017 15:47, Simon Farnsworth wrote:
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486568650 28800
# Wed Feb 08 07:44:10 2017 -0800
# Node ID d50cda2a403786836d1f0d5c99401599dc4f43ec
# Parent 1f51b4658f21bbb797e922d155c1046eddccf91d
pager: exit cleanly on SIGPIPE (BC)
Changeset aaa751585325 removes SIGPIPE handling completely. This is wrong,
as it means that Mercurial does not exit when the pager does. Instead, raise
SignalInterrupt when SIGPIPE happens with a pager attached, to trigger the
normal exit path.
This will cause "killed!" to be printed to stderr (hence the BC warning),
but in the normal pager use case (where the pager gets both stderr and
stdout), this messsage is lost as we only get SIGPIPE when the pager quits.
diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -72,6 +72,7 @@
commands,
dispatch,
encoding,
+error,
extensions,
util,
)
@@ -114,6 +115,9 @@
os.dup2(stderrfd, util.stderr.fileno())
pager.wait()
+def catchterm(*args):
+raise error.SignalInterrupt
+
def uisetup(ui):
class pagerui(ui.__class__):
def _runpager(self, pagercmd):
@@ -153,6 +157,8 @@
if usepager:
ui.setconfig('ui', 'formatted', ui.formatted(), 'pager')
ui.setconfig('ui', 'interactive', False, 'pager')
+if util.safehasattr(signal, "SIGPIPE"):
+signal.signal(signal.SIGPIPE, catchterm)
ui._runpager(p)
return orig(ui, options, cmd, cmdfunc)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=JGdkzd8E23MJxg59Iu-jc-4lKtVDPSGtiCxGk7Tvyfk=26T_I6At53KfO_UPKKWvM8tgFUbZStBE44m6jbEfsrk=
--
Simon Farnsworth
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] pager: exit cleanly on SIGPIPE (BC)
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486568650 28800
# Wed Feb 08 07:44:10 2017 -0800
# Node ID d50cda2a403786836d1f0d5c99401599dc4f43ec
# Parent 1f51b4658f21bbb797e922d155c1046eddccf91d
pager: exit cleanly on SIGPIPE (BC)
Changeset aaa751585325 removes SIGPIPE handling completely. This is wrong,
as it means that Mercurial does not exit when the pager does. Instead, raise
SignalInterrupt when SIGPIPE happens with a pager attached, to trigger the
normal exit path.
This will cause "killed!" to be printed to stderr (hence the BC warning),
but in the normal pager use case (where the pager gets both stderr and
stdout), this messsage is lost as we only get SIGPIPE when the pager quits.
diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -72,6 +72,7 @@
commands,
dispatch,
encoding,
+error,
extensions,
util,
)
@@ -114,6 +115,9 @@
os.dup2(stderrfd, util.stderr.fileno())
pager.wait()
+def catchterm(*args):
+raise error.SignalInterrupt
+
def uisetup(ui):
class pagerui(ui.__class__):
def _runpager(self, pagercmd):
@@ -153,6 +157,8 @@
if usepager:
ui.setconfig('ui', 'formatted', ui.formatted(), 'pager')
ui.setconfig('ui', 'interactive', False, 'pager')
+if util.safehasattr(signal, "SIGPIPE"):
+signal.signal(signal.SIGPIPE, catchterm)
ui._runpager(p)
return orig(ui, options, cmd, cmdfunc)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] dispatch: treat SIGPIPE as a termination signal (BC)
Please scrub this - it breaks `hg serve`. I'll send a v2 shortly, that moves the SIGPIPE handling into pager and thus has no BC implications. On 07/02/2017 20:08, Simon Farnsworth wrote: # HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1486498104 28800 # Tue Feb 07 12:08:24 2017 -0800 # Node ID e1150763706818fffa62c81a72a88574d20caea1 # Parent 1f51b4658f21bbb797e922d155c1046eddccf91d dispatch: treat SIGPIPE as a termination signal (BC) pager previously set SIGPIPE to immediately exit the process, ignoring any Python @atexit handlers, exception handling etc - just instant termination. Simply removing this as per changeset aaa751585325 meant that when you aborted a long running pager command, Mercurial would not quit; instead, we should add SIGPIPE to the list of termination signals in dispatch.py. This is a slight BC break, as previously, a process that was piping data into or out of Mercurial would not kill Mercurial if it died before closing its end of the pipe, whereas it will now cause Mercurial to exit. diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py --- a/mercurial/dispatch.py +++ b/mercurial/dispatch.py @@ -147,7 +147,7 @@ ui = req.ui try: -for name in 'SIGBREAK', 'SIGHUP', 'SIGTERM': +for name in 'SIGBREAK', 'SIGHUP', 'SIGTERM', 'SIGPIPE': num = getattr(signal, name, None) if num: signal.signal(num, catchterm) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=I0DTIf8TaBNbZo11b6eM-M9WIaAj0Va9TR_a9fvAsvk=629TWBre1WtsIXDvJQh3UKr9baaDbd6LlVXnmQeyP5E= -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] dispatch: treat SIGPIPE as a termination signal (BC)
On 07/02/2017 20:17, Jun Wu wrote: This approach looks good to me. The only problem is it will print "killed!" on SIGPIPE so maybe a follow-up to make it silent. Is there a practical case where "killed!" on stderr when the command is aborted due to SIGPIPE is a bad thing? In the pager case, this is only visible if stderr is redirected, but stdout is not - if stderr isn't redirected, then the pager (which has just exited - otherwise we wouldn't be getting SIGPIPE) will be sent the string, and won't print it. In other cases, I think the extra message is desirable (and justifies the BC marker). It tells you that where old Mercurial would continue processing after your stdin feed or stdout capture died unexpectedly, this Mercurial now quits. Excerpts from Simon Farnsworth's message of 2017-02-07 12:08:29 -0800: # HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1486498104 28800 # Tue Feb 07 12:08:24 2017 -0800 # Node ID e1150763706818fffa62c81a72a88574d20caea1 # Parent 1f51b4658f21bbb797e922d155c1046eddccf91d dispatch: treat SIGPIPE as a termination signal (BC) pager previously set SIGPIPE to immediately exit the process, ignoring any Python @atexit handlers, exception handling etc - just instant termination. Simply removing this as per changeset aaa751585325 meant that when you aborted a long running pager command, Mercurial would not quit; instead, we should add SIGPIPE to the list of termination signals in dispatch.py. This is a slight BC break, as previously, a process that was piping data into or out of Mercurial would not kill Mercurial if it died before closing its end of the pipe, whereas it will now cause Mercurial to exit. diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py --- a/mercurial/dispatch.py +++ b/mercurial/dispatch.py @@ -147,7 +147,7 @@ ui = req.ui try: -for name in 'SIGBREAK', 'SIGHUP', 'SIGTERM': +for name in 'SIGBREAK', 'SIGHUP', 'SIGTERM', 'SIGPIPE': num = getattr(signal, name, None) if num: signal.signal(num, catchterm) -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] pager: backed out changeset aaa751585325
I've sent a patch ("[PATCH] dispatch: treat SIGPIPE as a termination
signal (BC)") which fixes this up properly, instead of simply backing it
out.
The underlying bug that aaa751585325 tries to fix is that setting
SIGPIPE to SIG_DFL results in the interpreter not running any bytecode
after SIGPIPE comes in, which isn't great if you're trying to do cleanup
or logging at exit.
Simon
On 07/02/2017 19:23, Jun Wu wrote:
# HG changeset patch
# User Jun Wu <qu...@fb.com>
# Date 1486495380 28800
# Tue Feb 07 11:23:00 2017 -0800
# Node ID 8cfbd33c54779d1bbd50412e99f8256eea954401
# Parent a68510b69f413545722c086eaeb840dd5e8305b4
# Available At
https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_quark-2Dzju_hg-2Ddraft=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=Kkdsd1P9qEpbCBrbRyenM6kQC-sD5CNSJJxglAJzfec=3RFgXLLGBcJtw4HNz3jveuIu_mqKeZu-G4oap06m0AA=
# hg pull
https://urldefense.proofpoint.com/v2/url?u=https-3A__bitbucket.org_quark-2Dzju_hg-2Ddraft=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=Kkdsd1P9qEpbCBrbRyenM6kQC-sD5CNSJJxglAJzfec=3RFgXLLGBcJtw4HNz3jveuIu_mqKeZu-G4oap06m0AA=
-r 8cfbd33c5477
pager: backed out changeset aaa751585325
When a command needs a long time (ex. "hg log" on a big repo), and the user
exits the pager, they expect the hg command to exit immediately.
To be able to do that, terminating on SIGPIPE is important. Therefore revert
the change that ignores SIGPIPE.
diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -145,4 +145,6 @@ def uisetup(ui):
ui.setconfig('ui', 'formatted', ui.formatted(), 'pager')
ui.setconfig('ui', 'interactive', False, 'pager')
+if util.safehasattr(signal, "SIGPIPE"):
+signal.signal(signal.SIGPIPE, signal.SIG_DFL)
ui._runpager(p)
return orig(ui, options, cmd, cmdfunc)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel=DwIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=Kkdsd1P9qEpbCBrbRyenM6kQC-sD5CNSJJxglAJzfec=lRjI_yaSrj_N7KUWb3_WFaRF_2KzfxHikgvyHTXS-Lg=
--
Simon Farnsworth
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] dispatch: treat SIGPIPE as a termination signal (BC)
# HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1486498104 28800 # Tue Feb 07 12:08:24 2017 -0800 # Node ID e1150763706818fffa62c81a72a88574d20caea1 # Parent 1f51b4658f21bbb797e922d155c1046eddccf91d dispatch: treat SIGPIPE as a termination signal (BC) pager previously set SIGPIPE to immediately exit the process, ignoring any Python @atexit handlers, exception handling etc - just instant termination. Simply removing this as per changeset aaa751585325 meant that when you aborted a long running pager command, Mercurial would not quit; instead, we should add SIGPIPE to the list of termination signals in dispatch.py. This is a slight BC break, as previously, a process that was piping data into or out of Mercurial would not kill Mercurial if it died before closing its end of the pipe, whereas it will now cause Mercurial to exit. diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py --- a/mercurial/dispatch.py +++ b/mercurial/dispatch.py @@ -147,7 +147,7 @@ ui = req.ui try: -for name in 'SIGBREAK', 'SIGHUP', 'SIGTERM': +for name in 'SIGBREAK', 'SIGHUP', 'SIGTERM', 'SIGPIPE': num = getattr(signal, name, None) if num: signal.signal(num, catchterm) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Backwards compatibility before all else? [was Re: [PATCH v2] pager: migrate heavily-used extension into core]
On 06/02/2017 20:03, Augie Fackler wrote: On Feb 6, 2017, at 15:00, Bryan O'Sullivan <b...@serpentine.com> wrote: On Sun, Feb 5, 2017 at 7:24 PM, Augie Fackler <r...@durin42.com> wrote: I'm inclined to *not* add special code to see if the old pager extension has been disabled. That's achievable by disabling the pager instead. This would require a release note, of course (just in case anyone reads them). I’m conflicted here: I’ve been chasing my tail on a problem with emacs tramp-mode for months, and finally root-caused it to a missing flag in my pager settings for less, only triggered by emacs running hg over ssh. It was pretty baffling. On the other hand, it’s clearly the most-right choice to have the pager on as part of the recommended configuration. I’ve been using it (as an experiment) at work for over a year, and I’ve finally gotten used to it and (for the most part) like it. What do other people think? Well, this is an interesting case of a bigger pattern. To be honest, I think that the long-time insistence on (and acquiescence to) backwards compatibility for every little aspect of behaviour for all eternity has been extremely stifling. The fact that git users have thrived despite a decade of occasional incompatible changes (including enabling pager use by default) suggests that the compatibility-before-all perspective wasn't ever really prioritised correctly in the first place. I think that the community would do well to slightly loosen its standards for breaking changes. Yes, such changes will cause occasional problems for a small number of people, but the alternative of stagnation isn't super appealing. As a general principle, I agree. I think we'll have to take it on a case-by-case basis though: I'd like to avoid things that would cause widespread carnage in scripts that have been built over the years using sh scripts, chewing gum, and bailing wire. (My bias on the pager thing, btw, is to go with it, and try and encourage some widespread testing early in the cycle to try and sniff out any problems. It's rough in this case because our biggest captive tester audiences already default the pager to enabled.) I'd hope that part of that case-by-case basis should be "how many BC breaks have we taken this cycle", with an objective of ensuring that someone who tries to keep up with our releases doesn't get burnt badly by a release that happens to break everything they depend upon. With suitable release notes (another discussion ongoing right now) calling out the BC breaks, giving people who care a chance to object and have the BC break made configurable or undone for the next release, we should be able to keep our users happy while still improving. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH v3] util: always force line buffered stdout when stdout is a tty (BC)
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486163427 28800
# Fri Feb 03 15:10:27 2017 -0800
# Node ID 08b6e1bb20ca6bd1465a3d22bab49debfe776bae
# Parent 1f51b4658f21bbb797e922d155c1046eddccf91d
util: always force line buffered stdout when stdout is a tty (BC)
pager replaced stdout with a line buffered version to work around glibc
deciding on a buffering strategy on the first write to stdout. This is going
to make my next patch hard, as replacing stdout will make tracking time
spent blocked on it more challenging.
Move the line buffering requirement to util.py, and remove it from pager.
This means that the abuse of ui.formatted=True and pager set to cat or
equivalent
no longer results in a line-buffered output to a pipe, hence (BC), although
I don't expect anyone to be affected
diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -87,14 +87,10 @@
close_fds=util.closefds, stdin=subprocess.PIPE,
stdout=util.stdout, stderr=util.stderr)
-# back up original file objects and descriptors
-olduifout = ui.fout
-oldstdout = util.stdout
+# back up original file descriptors
stdoutfd = os.dup(util.stdout.fileno())
stderrfd = os.dup(util.stderr.fileno())
-# create new line-buffered stdout so that output can show up immediately
-ui.fout = util.stdout = newstdout = os.fdopen(util.stdout.fileno(), 'wb',
1)
os.dup2(pager.stdin.fileno(), util.stdout.fileno())
if ui._isatty(util.stderr):
os.dup2(pager.stdin.fileno(), util.stderr.fileno())
@@ -103,15 +99,10 @@
def killpager():
if util.safehasattr(signal, "SIGINT"):
signal.signal(signal.SIGINT, signal.SIG_IGN)
-pager.stdin.close()
-ui.fout = olduifout
-util.stdout = oldstdout
-# close new stdout while it's associated with pager; otherwise stdout
-# fd would be closed when newstdout is deleted
-newstdout.close()
-# restore original fds: stdout is open again
+# restore original fds, closing pager.stdin copies in the process
os.dup2(stdoutfd, util.stdout.fileno())
os.dup2(stderrfd, util.stderr.fileno())
+pager.stdin.close()
pager.wait()
def uisetup(ui):
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -63,6 +63,18 @@
urlreq = pycompat.urlreq
xmlrpclib = pycompat.xmlrpclib
+def isatty(fp):
+try:
+return fp.isatty()
+except AttributeError:
+return False
+
+# glibc determines buffering on first write to stdout - if we replace a TTY
+# destined stdout with a pipe destined stdout (e.g. pager), we want line
+# buffering
+if isatty(stdout):
+stdout = os.fdopen(stdout.fileno(), 'wb', 1)
+
if pycompat.osname == 'nt':
from . import windows as platform
stdout = platform.winstdout(pycompat.stdout)
@@ -2750,12 +2762,6 @@
u.user = u.passwd = None
return str(u)
-def isatty(fp):
-try:
-return fp.isatty()
-except AttributeError:
-return False
-
timecount = unitcountfn(
(1, 1e3, _('%.0f s')),
(100, 1, _('%.1f s')),
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 6 v2] util: add an elapsed time wrapper
On 03/02/2017 23:00, Bryan O'Sullivan wrote: On Fri, Feb 3, 2017 at 2:55 PM, Simon Farnsworth <simon...@fb.com <mailto:simon...@fb.com>> wrote: We already capture start and exit times in the external wrapper process - so your added detail of measuring "useful work" (in addition to "total time") tells us if we're seeing a huge overhead in setup/teardown of Mercurial. I think you might have missed a bit of crucial detail. If Mercurial sits for 100 minutes after writing its output to the pager before the user quits the pager, that's not going to show up as I/O time with your scheme, but it is going to show up as elapsed time. This is why capturing that measuring elapsed time is not useful. There's an indeterminate amount of time where literally nothing is happening (it's not teardown, and it's not overhead, it's just waiting for the pager) that you need to be able to discount. Yes - I'd accounted for that in an early prototype (which included pager.wait() in stdio time) but lost it by this version. Thanks for the reminder! -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 6 v2] util: add an elapsed time wrapper
On 03/02/2017 22:36, Bryan O'Sullivan wrote: On Fri, Feb 3, 2017 at 2:26 PM, Simon Farnsworth <simon...@fb.com <mailto:simon...@fb.com>> wrote: Assuming the community doesn't object strongly, I'll redo to be unconditionally measuring time (and only conditionally logging it), as the overhead of conditionality seems excessive for this purpose. If you'd also use the same approach as I do of direct measurement in the places that actually do the I/O, instead of indirection, that would be great. In fact, I think you should take over my patches and adapt them to your needs. The added detail in my patches of measuring when Mercurial stops doing work, instead of when it exits, is going to be important to your purposes, because just counting ui I/O time won't capture that. We already capture start and exit times in the external wrapper process - so your added detail of measuring "useful work" (in addition to "total time") tells us if we're seeing a huge overhead in setup/teardown of Mercurial. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 6 v2] util: add an elapsed time wrapper
On 03/02/2017 00:54, Bryan O'Sullivan wrote: On Thu, Feb 2, 2017 at 11:32 AM, Bryan O'Sullivan <b...@serpentine.com <mailto:b...@serpentine.com>> wrote: To be honest, this seems like a heavily over-engineered approach to me. I don't like giving such vague feedback, but I was hopping off the shuttle this morning and couldn't write anything more concrete at the time. Sorry about that. I also feel somewhat apologetic about seagulling in abruptly with a critique after months and months of silence, but ... hi? I wanted to substantiate my sense of unease with this patchset. Here's what I came up with. First, you'll want to take a look at my different approach, which I just sent out a moment ago. (It's not cooked, and I forgot to add the "RFC" tag, but you get the idea.) Next, performance. Here's a little benchmark: http://pastebin.com/f9eFEWiq <https://urldefense.proofpoint.com/v2/url?u=http-3A__pastebin.com_f9eFEWiq=DwMFaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=Z_ILapZ4MOSTKVH08OWF7ibTePT7tfFDPNqh5YnzmFQ=iN5BYr4m1lyQkPVl1UVCnw43-0a3kpVooUdu2lfAGzU=> On my laptop, base Mercurial runs it in 0.15 seconds. With performance logging enabled, your patchset takes 0.36 seconds. And my patchset takes 0.16, with no need to enable or disable anything. To be clear, the performance numbers should not be persuasive, because a typical invocation of Mercurial will call ui.write maybe 1% as often as the benchmark does. But what they do is give me another angle from which to look at "this doesn't feel right to me". I'd assumed that with Mercurial encouraging extension authors to wrap functions, the overhead wasn't significant. Your patchset and message had me run a couple of experiments on my Mac, using the timeit module, and a trivial function: def func(): print "Hello" This is both trivial, and on the sort of complexity order of the functions I'm trying to measure. The baseline I got was that 1,000,000 invocations of this function take 2.0 seconds elapsed time. I then added if True to the function, to get a sense of the pain of making it conditional: def func(): if True: print "Hello" This takes 2.3 seconds for the same million repetitions. I then restored the unconditional version of func(); using a simple wrapper that just does if True: func(), I get 2.4 seconds, while a class-based unconditional wrapper is 2.7 seconds: class wrapper: def __init__(self, func): self.func = func def __call__(self, *args, **kwargs): self.func(*args, **kwargs) It looks to me like the conditional actually adds significant pain in its own right, almost as much as the complex class based wrapper does. Assuming the community doesn't object strongly, I'll redo to be unconditionally measuring time (and only conditionally logging it), as the overhead of conditionality seems excessive for this purpose. Your patch adds a bunch of allocation and indirection on what for this benchmark is a fast path, which is why it appears so costly. Abstractions in Python have a heavy runtime cost and of course the usual "now I have an abstraction to understand" overhead, and so you have to be particularly judicious in creating and using them. The above approach of indirection via a wrapper object doesn't meet my personal "abstraction that pays for the cost of understanding and using it" bar, especially as the alternative approach in my patchset is both simple to understand and has almost no overhead. Also, the way that you're conditionally recording time used in both this and later patches (for crecord and extdiff) strikes me as unnecessarily complex. My patches demonstrate that doing so unconditionally is simpler and has inconsequential overhead. I definitely want the reporting to be conditional - if you're not going to use the time, we shouldn't output it. My mental model for Python costs was wrong (and is now corrected), and I'll redo the time gathering accordingly. From my point of view, the goals are: 1. Establish groups of "not Mercurial's fault" slowness - stdio, extdiff, editor invoked for histedit etc. This combines with FB infra to let us focus on cases where Mercurial code is the bottleneck. 2. Do so in such a way that users not on FB infra aren't affected unless they choose to be, even if they're using things like logtoprocess today. It looks like I met goal 2 at the expense of goal 1, and I'm going to need to rework to get a nicer compromise in place between the goals. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 3 of 6 v2] util: always force line buffered stdout when stdout is a tty (BC)
# HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1486063056 28800 # Thu Feb 02 11:17:36 2017 -0800 # Node ID 722c309600ed9596a02674b04cb2caa9a65e8918 # Parent 12d0ac224bb34691d44a2cead5b9795a6cfc2490 util: always force line buffered stdout when stdout is a tty (BC) pager replaced stdout with a line buffered version to work around glibc deciding on a buffering strategy on the first write to stdout. This is going to make my next patch hard, as replacing stdout will make tracking time spent blocked on it more challenging. Move the line buffering requirement to util.py, and remove it from pager. This means that the abuse of ui.formatted=True and pager set to cat or equivalent no longer results in a line-buffered output to a pipe, hence (BC), although I don't expect anyone to be affected diff --git a/hgext/pager.py b/hgext/pager.py --- a/hgext/pager.py +++ b/hgext/pager.py @@ -87,14 +87,10 @@ close_fds=util.closefds, stdin=subprocess.PIPE, stdout=util.stdout, stderr=util.stderr) -# back up original file objects and descriptors -olduifout = ui.fout -oldstdout = util.stdout +# back up original file descriptors stdoutfd = os.dup(util.stdout.fileno()) stderrfd = os.dup(util.stderr.fileno()) -# create new line-buffered stdout so that output can show up immediately -ui.fout = util.stdout = newstdout = os.fdopen(util.stdout.fileno(), 'wb', 1) os.dup2(pager.stdin.fileno(), util.stdout.fileno()) if ui._isatty(util.stderr): os.dup2(pager.stdin.fileno(), util.stderr.fileno()) @@ -103,15 +99,10 @@ def killpager(): if util.safehasattr(signal, "SIGINT"): signal.signal(signal.SIGINT, signal.SIG_IGN) -pager.stdin.close() -ui.fout = olduifout -util.stdout = oldstdout -# close new stdout while it's associated with pager; otherwise stdout -# fd would be closed when newstdout is deleted -newstdout.close() -# restore original fds: stdout is open again +# restore original fds, closing pagin.stdin copies in the process os.dup2(stdoutfd, util.stdout.fileno()) os.dup2(stderrfd, util.stderr.fileno()) +pager.stdin.close() pager.wait() def uisetup(ui): diff --git a/mercurial/util.py b/mercurial/util.py --- a/mercurial/util.py +++ b/mercurial/util.py @@ -69,6 +69,12 @@ else: from . import posix as platform +# glibc determines buffering on first write to stdout - if we replace a TTY +# destined stdout with a pipe destined stdout (e.g. pager), we want line +# buffering +if stdout.isatty(): +stdout = os.fdopen(stdout.fileno(), 'wb', 1) + _ = i18n._ bindunixsocket = platform.bindunixsocket ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 6 of 6 v2] extdiff: measure blocked time
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486063056 28800
# Thu Feb 02 11:17:36 2017 -0800
# Node ID c85a13e4bbc3a26b6bf6f502c207f5491eb0889f
# Parent 581cb462ae6eb9ae95fdd64ec55df2b58ab6233a
extdiff: measure blocked time
Log the time spent in the external diff tool only - this is the bit that's
outside our control
diff --git a/hgext/extdiff.py b/hgext/extdiff.py
--- a/hgext/extdiff.py
+++ b/hgext/extdiff.py
@@ -273,7 +273,18 @@
cmdline = re.sub(regex, quote, cmdline)
ui.debug('running %r in %s\n' % (cmdline, tmproot))
-ui.system(cmdline, cwd=tmproot)
+if ui.logblockedtime:
+system = util.elapsedtimewrapper(ui.system)
+else:
+system = ui.system
+try:
+system(cmdline, cwd=tmproot)
+finally:
+if ui.logblockedtime:
+ui.log('uiblocked',
+ 'extdiff blocked for %0.1f ms', system.elapsedms,
+extdiff_blocked=system.elapsedms)
+
for copy_fn, working_fn, mtime in fns_and_mtime:
if os.lstat(copy_fn).st_mtime != mtime:
diff --git a/tests/test-extdiff.t b/tests/test-extdiff.t
--- a/tests/test-extdiff.t
+++ b/tests/test-extdiff.t
@@ -72,6 +72,26 @@
[1]
#endif
+Should be able to measure blocked time:
+
+#if windows
+$ hg falabala -r 0:1 --config ui.logblockedtime=True --config
extensions.logtoprocess= --config logtoprocess.uiblocked='echo uiblocked
extdiff time $OPT_EXTDIFF_BLOCKED'| head -n50 | sort
+ diffing "*\\extdiff.*\\a.8a5febb7f867\\a" "a.34eed99112ab\\a" (glob)
+ uiblocked extdiff time
+ uiblocked extdiff time
+ uiblocked extdiff time
+ uiblocked extdiff time
+ uiblocked extdiff time .* (glob)
+#else
+ $ hg falabala -r 0:1 --config ui.logblockedtime=True --config
extensions.logtoprocess= --config logtoprocess.uiblocked='echo uiblocked
extdiff time $OPT_EXTDIFF_BLOCKED'| head -n50 | sort
+ diffing */extdiff.*/a.8a5febb7f867/a a.34eed99112ab/a (glob)
+ uiblocked extdiff time
+ uiblocked extdiff time
+ uiblocked extdiff time
+ uiblocked extdiff time
+ uiblocked extdiff time .* (glob)
+#endif
+
Specifying an empty revision should abort.
$ hg extdiff -p diff --patch --rev 'ancestor()' --rev 1
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 5 of 6 v2] crecord: log blocked time waiting for curses input
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486063056 28800
# Thu Feb 02 11:17:36 2017 -0800
# Node ID 581cb462ae6eb9ae95fdd64ec55df2b58ab6233a
# Parent a590327ef7a47e1390bcef0eef686c40ffc4894d
crecord: log blocked time waiting for curses input
We want to know when we're blocked on user input - log it.
diff --git a/mercurial/crecord.py b/mercurial/crecord.py
--- a/mercurial/crecord.py
+++ b/mercurial/crecord.py
@@ -514,6 +514,7 @@
self.ui = ui
self.opts = {}
+self.elapsedms = 0
self.errorstr = None
# list of all chunks
@@ -1374,29 +1375,40 @@
except curses.error:
pass
helpwin.refresh()
+if self.ui.logblockedtime:
+helpwin = util.elapsedtimewrapper(helpwin)
try:
helpwin.getkey()
except curses.error:
pass
+finally:
+if self.ui.logblockedtime:
+self.elapsedms += helpwin.elapsedms
def confirmationwindow(self, windowtext):
"display an informational window, then wait for and return a keypress."
confirmwin = curses.newwin(self.yscreensize, 0, 0, 0)
+if self.ui.logblockedtime:
+confirmwin = util.elapsedtimewrapper(confirmwin)
try:
-lines = windowtext.split("\n")
-for line in lines:
-self.printstring(confirmwin, line, pairname="selected")
-except curses.error:
-pass
-self.stdscr.refresh()
-confirmwin.refresh()
-try:
-response = chr(self.stdscr.getch())
-except ValueError:
-response = None
+try:
+lines = windowtext.split("\n")
+for line in lines:
+self.printstring(confirmwin, line, pairname="selected")
+except curses.error:
+pass
+self.stdscr.refresh()
+confirmwin.refresh()
+try:
+response = chr(self.stdscr.getch())
+except ValueError:
+response = None
-return response
+return response
+finally:
+if self.ui.logblockedtime:
+self.elapsedms += confirmwin.elapsedms
def reviewcommit(self):
"""ask for 'y' to be pressed to confirm selected. return True if
@@ -1614,6 +1626,8 @@
origsigwinchhandler = signal.signal(signal.SIGWINCH,
self.sigwinchhandler)
self.stdscr = stdscr
+if self.ui.logblockedtime:
+self.stdscr = util.elapsedtimewrapper(self.stdscr)
# error during initialization, cannot be printed in the curses
# interface, it should be printed by the calling code
self.initerr = None
@@ -1632,6 +1646,8 @@
self.initcolorpair(curses.COLOR_WHITE, curses.COLOR_BLUE,
name="legend")
# newwin([height, width,] begin_y, begin_x)
self.statuswin = curses.newwin(self.numstatuslines, 0, 0, 0)
+if self.ui.logblockedtime:
+self.statuswin = util.elapsedtimewrapper(self.statuswin)
self.statuswin.keypad(1) # interpret arrow-key, etc. esc sequences
# figure out how much space to allocate for the chunk-pad which is
@@ -1652,15 +1668,23 @@
self.selecteditemendline = self.getnumlinesdisplayed(
self.currentselecteditem, recursechildren=False)
-while True:
-self.updatescreen()
-try:
-keypressed = self.statuswin.getkey()
-if self.errorstr is not None:
-self.errorstr = None
-continue
-except curses.error:
-keypressed = "foobar"
-if self.handlekeypressed(keypressed):
-break
+try:
+while True:
+self.updatescreen()
+try:
+keypressed = self.statuswin.getkey()
+if self.errorstr is not None:
+self.errorstr = None
+continue
+except curses.error:
+keypressed = "foobar"
+if self.handlekeypressed(keypressed):
+break
+finally:
+if self.ui.logblockedtime:
+self.elapsedms += self.stdscr.elapsedms
+self.elapsedms += self.statuswin.elapsedms
+self.ui.log('uiblocked',
+'crecord blocked for %0.1f ms', self.elapsedms,
+crecord_blocked=self.elapsedms)
signal.signal(signal.SIGWINCH, origsigwinchhandler)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 6 v2] util: add an elapsed time wrapper
# HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1486063056 28800 # Thu Feb 02 11:17:36 2017 -0800 # Node ID 12d0ac224bb34691d44a2cead5b9795a6cfc2490 # Parent f443bd95a948aaed36e81edb884085fc2f0f5acf util: add an elapsed time wrapper We want to log the time spent in various commands. Add a wrapper class that can be used to measure time taken by a function or all members of a class diff --git a/mercurial/util.py b/mercurial/util.py --- a/mercurial/util.py +++ b/mercurial/util.py @@ -3543,3 +3543,38 @@ # convenient shortcut dst = debugstacktrace + +class elapsedtimewrapper(object): +def __init__(self, orig): +self.__orig = orig +self.__istiming = False +self.counttime = True +self.elapsedms = 0 + +def __call__(self, *args, **kwargs): +if self.counttime: +return self.__time(self.__orig, *args, **kwargs) +else: +return self.__orig(*args, **kwargs) + +def __getattr__(self, name): +origattr = getattr(self.__orig, name) +if self.counttime and callable(origattr): +def closure(*args, **kwargs): +return self.__time(origattr, *args, **kwargs) +return closure +else: +return origattr + +def __time(self, func, *args, **kwargs): +if self.__istiming: +return func(*args, **kwargs) + +start = time.time() +try: +self.__istiming = True +return func(*args, **kwargs) +finally: +duration = time.time() - start +self.elapsedms += duration * 1000 +self.__istiming = False ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 6 v2] pager: don't terminate with extreme prejudice on SIGPIPE (BC)
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1486063056 28800
# Thu Feb 02 11:17:36 2017 -0800
# Node ID f443bd95a948aaed36e81edb884085fc2f0f5acf
# Parent abf029200e198878a4576a87e095bd8d77d9cea9
pager: don't terminate with extreme prejudice on SIGPIPE (BC)
The default SIGPIPE handler causes Mercurial to exit immediately, without
running any Python cleanup code (except and finally blocks, atexit handlers
etc). This creates problems if you want to do something at exit.
If we need a different exit code for broken pipe from pager, then we should
code that ourselves in Python; this appears to have been cargo-culted from
the fork implementation of pager that's no longer used, where it was needed
to stop Broken Pipe errors appearing on the user's terminal.
diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -153,8 +153,6 @@
if usepager:
ui.setconfig('ui', 'formatted', ui.formatted(), 'pager')
ui.setconfig('ui', 'interactive', False, 'pager')
-if util.safehasattr(signal, "SIGPIPE"):
-signal.signal(signal.SIGPIPE, signal.SIG_DFL)
ui._runpager(p)
return orig(ui, options, cmd, cmdfunc)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 5] pager: stdout is line buffered by default
On 20/01/2017 12:56, Yuya Nishihara wrote: On Thu, 19 Jan 2017 11:02:07 -0800, Simon Farnsworth wrote: # HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1484835774 28800 # Thu Jan 19 06:22:54 2017 -0800 # Node ID 76123ae2e0ccaa58db3d4fc26b75b7251e13ad16 # Parent 036c37bd3ec189480647ff568cee9e0b43a5bc81 pager: stdout is line buffered by default pager only starts when ui.formatted() is true. In normal operation, when ui.formatted() is true, stdout is line buffered anyway. Unfortunately that isn't always true. The buffering mode of FILE object is decided on the first fwrite() (or perhaps fflush()) as far as on glibc. We generally attach the pager fd before writing anything, which makes the stdout fully-buffered. The code here doesn't actually do what the commit message in changeset 62c5e937 claims it will do; sys.stdout is not yet pager.stdin when we reopen, so we affect the original sys.stdout's buffering mode. It's the dup2 that puts pager.stdin's fd into sys.stdout's fd, and that doesn't affect buffering. No. AFAIK, the buffering mode is defined per FILE object, not per file descriptor. Argh. I've written a test program, and this is true of glibc (thanks for the unclear documentation, glibc guys) - I'll improve the comment when I resubmit after the freeze. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 5] pager: stdout is line buffered by default
# HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1484835774 28800 # Thu Jan 19 06:22:54 2017 -0800 # Node ID 76123ae2e0ccaa58db3d4fc26b75b7251e13ad16 # Parent 036c37bd3ec189480647ff568cee9e0b43a5bc81 pager: stdout is line buffered by default pager only starts when ui.formatted() is true. In normal operation, when ui.formatted() is true, stdout is line buffered anyway. The code here doesn't actually do what the commit message in changeset 62c5e937 claims it will do; sys.stdout is not yet pager.stdin when we reopen, so we affect the original sys.stdout's buffering mode. It's the dup2 that puts pager.stdin's fd into sys.stdout's fd, and that doesn't affect buffering. diff --git a/hgext/pager.py b/hgext/pager.py --- a/hgext/pager.py +++ b/hgext/pager.py @@ -87,14 +87,10 @@ close_fds=util.closefds, stdin=subprocess.PIPE, stdout=util.stdout, stderr=util.stderr) -# back up original file objects and descriptors -olduifout = ui.fout -oldstdout = util.stdout +# back up original file descriptors stdoutfd = os.dup(util.stdout.fileno()) stderrfd = os.dup(util.stderr.fileno()) -# create new line-buffered stdout so that output can show up immediately -ui.fout = util.stdout = newstdout = os.fdopen(util.stdout.fileno(), 'wb', 1) os.dup2(pager.stdin.fileno(), util.stdout.fileno()) if ui._isatty(util.stderr): os.dup2(pager.stdin.fileno(), util.stderr.fileno()) @@ -103,15 +99,12 @@ def killpager(): if util.safehasattr(signal, "SIGINT"): signal.signal(signal.SIGINT, signal.SIG_IGN) -pager.stdin.close() -ui.fout = olduifout -util.stdout = oldstdout -# close new stdout while it's associated with pager; otherwise stdout -# fd would be closed when newstdout is deleted -newstdout.close() -# restore original fds: stdout is open again +# restore original fds os.dup2(stdoutfd, util.stdout.fileno()) os.dup2(stderrfd, util.stderr.fileno()) +# close pager's stdin so that it knows we're not going to send any more +# data +pager.stdin.close() pager.wait() def uisetup(ui): ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 5 of 5] crecord: log blocked time waiting for curses input
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1484850980 28800
# Thu Jan 19 10:36:20 2017 -0800
# Node ID 9684b31c29f3f17096ead595fef50b687d518b1c
# Parent 93221dde2fad942c4f920dab1f346da71ac8033d
crecord: log blocked time waiting for curses input
We want to know when we're blocked on user input - log it.
diff --git a/mercurial/crecord.py b/mercurial/crecord.py
--- a/mercurial/crecord.py
+++ b/mercurial/crecord.py
@@ -514,6 +514,7 @@
self.ui = ui
self.opts = {}
+self.elapsedms = 0
self.errorstr = None
# list of all chunks
@@ -1374,29 +1375,40 @@
except curses.error:
pass
helpwin.refresh()
+if self.ui.logblockedtime:
+helpwin = util.elapsedtimewrapper(helpwin)
try:
helpwin.getkey()
except curses.error:
pass
+finally:
+if self.ui.logblockedtime:
+self.elapsedms += helpwin.elapsedms
def confirmationwindow(self, windowtext):
"display an informational window, then wait for and return a keypress."
confirmwin = curses.newwin(self.yscreensize, 0, 0, 0)
+if self.ui.logblockedtime:
+confirmwin = util.elapsedtimewrapper(confirmwin)
try:
-lines = windowtext.split("\n")
-for line in lines:
-self.printstring(confirmwin, line, pairname="selected")
-except curses.error:
-pass
-self.stdscr.refresh()
-confirmwin.refresh()
-try:
-response = chr(self.stdscr.getch())
-except ValueError:
-response = None
+try:
+lines = windowtext.split("\n")
+for line in lines:
+self.printstring(confirmwin, line, pairname="selected")
+except curses.error:
+pass
+self.stdscr.refresh()
+confirmwin.refresh()
+try:
+response = chr(self.stdscr.getch())
+except ValueError:
+response = None
-return response
+return response
+finally:
+if self.ui.logblockedtime:
+self.elapsedms += confirmwin.elapsedms
def reviewcommit(self):
"""ask for 'y' to be pressed to confirm selected. return True if
@@ -1612,6 +1624,8 @@
origsigwinchhandler = signal.signal(signal.SIGWINCH,
self.sigwinchhandler)
self.stdscr = stdscr
+if self.ui.logblockedtime:
+self.stdscr = util.elapsedtimewrapper(self.stdscr)
# error during initialization, cannot be printed in the curses
# interface, it should be printed by the calling code
self.initerr = None
@@ -1630,6 +1644,8 @@
self.initcolorpair(curses.COLOR_WHITE, curses.COLOR_BLUE,
name="legend")
# newwin([height, width,] begin_y, begin_x)
self.statuswin = curses.newwin(self.numstatuslines, 0, 0, 0)
+if self.ui.logblockedtime:
+self.statuswin = util.elapsedtimewrapper(self.statuswin)
self.statuswin.keypad(1) # interpret arrow-key, etc. esc sequences
# figure out how much space to allocate for the chunk-pad which is
@@ -1650,15 +1666,23 @@
self.selecteditemendline = self.getnumlinesdisplayed(
self.currentselecteditem, recursechildren=False)
-while True:
-self.updatescreen()
-try:
-keypressed = self.statuswin.getkey()
-if self.errorstr is not None:
-self.errorstr = None
-continue
-except curses.error:
-keypressed = "foobar"
-if self.handlekeypressed(keypressed):
-break
+try:
+while True:
+self.updatescreen()
+try:
+keypressed = self.statuswin.getkey()
+if self.errorstr is not None:
+self.errorstr = None
+continue
+except curses.error:
+keypressed = "foobar"
+if self.handlekeypressed(keypressed):
+break
+finally:
+if self.ui.logblockedtime:
+self.elapsedms += self.stdscr.elapsedms
+self.elapsedms += self.statuswin.elapsedms
+self.ui.log('uiblocked',
+'crecord blocked for %0.1f ms', self.elapsed_ms,
+crecord_blocked=self.elapsedms)
signal.signal(signal.SIGWINCH, origsigwinchhandler)
___
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
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# 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 evolve v2] tests: use curl instead of wget
As well as the extended commit message, I've also edited https://www.mercurial-scm.org/wiki/HackableMercurial which was the only wiki page I found mentioning msys. I don't know Windows well enough to actually get it right, though (things appear to have changed a bit since NT 3.51). Simon On 25/10/2016 13:23, Simon Farnsworth wrote: # HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1477397752 25200 # Tue Oct 25 05:15:52 2016 -0700 # Branch stable # Node ID f65f9acac6c69e6f2eb90b2ed9b51d818a046f67 # Parent 970a4c13ebc320a034bc0aff21e0ef0a84157a92 tests: use curl instead of wget curl is supplied by default on macOS 10.12, but wget isn't. As curl is easy to install on other OSes, just switch the tests over. For Windows systems, you can obtain cURL from https://urldefense.proofpoint.com/v2/url?u=https-3A__curl.haxx.se_download.html=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=RNJmt18FdGmAqHoKFoP0YdwtpdWeV9vll-MZwSCIk24= - for other systems, please use your native package manager. This undoes 4e7da688a066 and 3ffa12edc05a, as they don't make things much simpler on Windows (you have to install extra packages either way round), but they do make things harder on macOS (as curl is supplied by default, whereas wget isn't). diff --git a/tests/test-simple4server-bundle2.t b/tests/test-simple4server-bundle2.t --- a/tests/test-simple4server-bundle2.t +++ b/tests/test-simple4server-bundle2.t @@ -71,12 +71,12 @@ Capacity testing === - $ wget -q -O - https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dhello=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=8iUVs0V0ElXtCkjkG1Z7w2c3t4YljF--qG-P8hSAQSo= + $ curl -s https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dhello=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=8iUVs0V0ElXtCkjkG1Z7w2c3t4YljF--qG-P8hSAQSo= capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dcapabilities=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=S7W0pW9SHkDd5RGx6ImgpzkDEgimNJVZYEN6RQQXllw= + $ curl -s https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dcapabilities=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=S7W0pW9SHkDd5RGx6ImgpzkDEgimNJVZYEN6RQQXllw= * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (no-eol) (glob) - $ wget -q -O - "https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dlistkeys-26namespace-3Dnamespaces=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=x4s8JJyZEPf1dxyp2EdY-aVjRGZ1mxwgKOD40Uc3VhM= " | sort + $ curl -s "https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dlistkeys-26namespace-3Dnamespaces=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=x4s8JJyZEPf1dxyp2EdY-aVjRGZ1mxwgKOD40Uc3VhM= " | sort bookmarks namespaces obsolete @@ -128,14 +128,14 @@ === (used by bitbucket to select which repo use evolve) - $ wget -q -O - "https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dlistkeys-26namespace-3Dnamespaces=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=x4s8JJyZEPf1dxyp2EdY-aVjRGZ1mxwgKOD40Uc3VhM= " | sort + $ curl -s "https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dlistkeys-26namespace-3Dnamespaces=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=x4s8JJyZEPf1dxyp2EdY-aVjRGZ1mxwgKOD40Uc3VhM= " | sort bookmarks namespaces obsolete phases - $ wget -q -O - https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dhello=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=8iUVs0V0ElXtCkjkG1Z7w2c3t4YljF--qG-P8hSAQSo= + $ curl -s https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dhello=DQIGaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=xpCZbhnEotl-MEx3cSrK8Bq-4nzxq9NGPFZsnbO9v2s=8iUVs0V0ElXtCkjkG1Z7w2c3t4YljF--qG-P8hSAQSo= capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - https://urldefense.proofpoint.com/v2/url?u=http-3A__loca
[PATCH evolve v2] tests: use curl instead of wget
# HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1477397752 25200 # Tue Oct 25 05:15:52 2016 -0700 # Branch stable # Node ID f65f9acac6c69e6f2eb90b2ed9b51d818a046f67 # Parent 970a4c13ebc320a034bc0aff21e0ef0a84157a92 tests: use curl instead of wget curl is supplied by default on macOS 10.12, but wget isn't. As curl is easy to install on other OSes, just switch the tests over. For Windows systems, you can obtain cURL from https://curl.haxx.se/download.html - for other systems, please use your native package manager. This undoes 4e7da688a066 and 3ffa12edc05a, as they don't make things much simpler on Windows (you have to install extra packages either way round), but they do make things harder on macOS (as curl is supplied by default, whereas wget isn't). diff --git a/tests/test-simple4server-bundle2.t b/tests/test-simple4server-bundle2.t --- a/tests/test-simple4server-bundle2.t +++ b/tests/test-simple4server-bundle2.t @@ -71,12 +71,12 @@ Capacity testing === - $ wget -q -O - http://localhost:$HGPORT/?cmd=hello + $ curl -s http://localhost:$HGPORT/?cmd=hello capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - http://localhost:$HGPORT/?cmd=capabilities + $ curl -s http://localhost:$HGPORT/?cmd=capabilities * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (no-eol) (glob) - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces obsolete @@ -128,14 +128,14 @@ === (used by bitbucket to select which repo use evolve) - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces obsolete phases - $ wget -q -O - http://localhost:$HGPORT/?cmd=hello + $ curl -s http://localhost:$HGPORT/?cmd=hello capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - http://localhost:$HGPORT/?cmd=capabilities + $ curl -s http://localhost:$HGPORT/?cmd=capabilities * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (no-eol) (glob) $ echo '[__temporary__]' >> server/.hg/hgrc @@ -144,7 +144,7 @@ $ hg serve -R server -n test -p $HGPORT -d --pid-file=hg.pid -A access.log -E errors.log $ cat hg.pid >> $DAEMON_PIDS - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces phases @@ -154,13 +154,13 @@ $ hg serve -R server -n test -p $HGPORT -d --pid-file=hg.pid -A access.log -E errors.log $ cat hg.pid >> $DAEMON_PIDS - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces obsolete phases - $ wget -q -O - http://localhost:$HGPORT/?cmd=hello + $ curl -s http://localhost:$HGPORT/?cmd=hello capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - http://localhost:$HGPORT/?cmd=capabilities + $ curl -s http://localhost:$HGPORT/?cmd=capabilities * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (no-eol) (glob) diff --git a/tests/test-simple4server.t b/tests/test-simple4server.t --- a/tests/test-simple4server.t +++ b/tests/test-simple4server.t @@ -73,12 +73,12 @@ Capacity testing === - $ wget -q -O - http://localhost:$HGPORT/?cmd=hello + $ curl -s http://localhost:$HGPORT/?cmd=hello capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - http://localhost:$HGPORT/?cmd=capabilities + $ curl -s http://localhost:$HGPORT/?cmd=capabilities * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (no-eol) (glob) - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces obsolete @@ -131,14 +131,14 @@ === (used by bitbucket to select which repo use evolve) - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces;
Re: [PATCH evolve] tests: use curl instead of wget
On 25/10/2016 12:57, Matt Harbison wrote: OT: is there a blog or something somewhere that describes how you guys (or any other enterprises) get the developer's system setup and configured? Things like .hgrc settings, installing and configuring toolchains, staging build scripts, etc. I think I've read some of Greg's about how Mozilla does it, and we're basically trying to solve the same problems. There's nothing detailed out in the public domain; however, it's not a secret that we use chef for all our system configuration, including deciding which packages to install, and putting /etc/mercurial in place on developer systems. In turn, we build an in-house Mercurial package that bundles up all our extensions and Mercurial itself into a nice atomic lump for chef to play with. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH evolve] tests: use curl instead of wget
On 25/10/2016 02:34, Matt Harbison wrote: On Mon, 24 Oct 2016 09:37:27 -0400, Simon Farnsworth <simon...@fb.com> wrote: On 24/10/2016 14:32, Pierre-Yves David wrote: On 10/24/2016 03:26 PM, Simon Farnsworth wrote: # HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1477315431 25200 # Mon Oct 24 06:23:51 2016 -0700 # Branch stable # Node ID 5fbaca977cd43dfd806a3f452543ef0ed4a4732e # Parent 970a4c13ebc320a034bc0aff21e0ef0a84157a92 tests: use curl instead of wget curl is supplied by default on macOS 10.12, but wget isn't. As curl is easy to install on other OSes, just switch the tests over. Hum, 4e7da688a066 and 3ffa12edc05a who did the exact opposite 1.5 year ago. Can you have a look at them and come back with a plan that fits both need? I've looked at the commits you referenced, and there's not enough context for me to understand why Matt switched from cURL (works on Linux and macOS by default, needs a package installed on Windows as far as I understand it), to wget (needs a package installed on Windows and macOS as far as I understand it). Can you give me context for why you took these two commits? I didn't get a chance to see if wget was installed on previous versions of OS X before leaving work today, but I don't recall ever having installed it on 10.6 or 10.10. So maybe that's something new with 10.12? I've just checked a 10.10 VM and a 10.11 system, and those did not have wget installed, either (but they did have curl). I can't go back further easily. Google tells me that wget is not preinstalled on 10.6 Snow Leopard, either (http://apple.stackexchange.com/questions/12665/how-do-i-get-wget-for-snow-leopard for example); I suspect that you had manually installed wget on your Macs. The reason I switched was because wget is available with msys, but curl isn't. I dug up some Windows build of curl a few months ago, but don't recall where. Maybe it's still in my browser history at work. I seem to have a /mingw/bin/curl on my home system, and vaguely remember building that from source a long time ago. It looks like msys doesn't provide curl, but the download "wizard" at https://curl.haxx.se/download.html has current Windows builds listed next to Cygwin builds. Would you like me to do a v2 of the patch summarizing this discussion and linking to that page in the commit message? Since I doubt many (any?) people run the tests on Windows, I don't have a problem with switching back, if how to install curl is documented on the page describing how to run tests on Windows. I forget if I made similar changes in core Mercurial. This is the only test suite I run on macOS that uses wget - I run most of the core test suite, so I'd notice if it needed wget. The reason this has become an issue is that I'm shifting from manual builds to automatic builds of the Facebook internal package (which bundles up Mercurial, some extensions, and our configs for our users), and I want to minimise the work we have to do to keep the automatic builds working. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH evolve] tests: use curl instead of wget
On 24/10/2016 14:32, Pierre-Yves David wrote: On 10/24/2016 03:26 PM, Simon Farnsworth wrote: # HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1477315431 25200 # Mon Oct 24 06:23:51 2016 -0700 # Branch stable # Node ID 5fbaca977cd43dfd806a3f452543ef0ed4a4732e # Parent 970a4c13ebc320a034bc0aff21e0ef0a84157a92 tests: use curl instead of wget curl is supplied by default on macOS 10.12, but wget isn't. As curl is easy to install on other OSes, just switch the tests over. Hum, 4e7da688a066 and 3ffa12edc05a who did the exact opposite 1.5 year ago. Can you have a look at them and come back with a plan that fits both need? I've looked at the commits you referenced, and there's not enough context for me to understand why Matt switched from cURL (works on Linux and macOS by default, needs a package installed on Windows as far as I understand it), to wget (needs a package installed on Windows and macOS as far as I understand it). Can you give me context for why you took these two commits? diff --git a/tests/test-simple4server-bundle2.t b/tests/test-simple4server-bundle2.t --- a/tests/test-simple4server-bundle2.t +++ b/tests/test-simple4server-bundle2.t @@ -71,12 +71,12 @@ Capacity testing === - $ wget -q -O - https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dhello=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=DesF8_W89UXCaJo3gHu4hF5AQTlgK8eV_wP-UyTEsns= + $ curl -s https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dhello=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=DesF8_W89UXCaJo3gHu4hF5AQTlgK8eV_wP-UyTEsns= capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dcapabilities=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=FvjM7sVt9NyPcthiIFzLTjIbezUtsP4Di9PpsfBwVAk= + $ curl -s https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dcapabilities=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=FvjM7sVt9NyPcthiIFzLTjIbezUtsP4Di9PpsfBwVAk= * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (no-eol) (glob) - $ wget -q -O - "https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dlistkeys-26namespace-3Dnamespaces=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=8DDabWdNhDRmKdDCh432QwB_9SEAzDiTb3D0cgeDTZo= " | sort + $ curl -s "https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dlistkeys-26namespace-3Dnamespaces=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=8DDabWdNhDRmKdDCh432QwB_9SEAzDiTb3D0cgeDTZo= " | sort bookmarks namespaces obsolete @@ -128,14 +128,14 @@ === (used by bitbucket to select which repo use evolve) - $ wget -q -O - "https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dlistkeys-26namespace-3Dnamespaces=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=8DDabWdNhDRmKdDCh432QwB_9SEAzDiTb3D0cgeDTZo= " | sort + $ curl -s "https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dlistkeys-26namespace-3Dnamespaces=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=8DDabWdNhDRmKdDCh432QwB_9SEAzDiTb3D0cgeDTZo= " | sort bookmarks namespaces obsolete phases - $ wget -q -O - https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dhello=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=DesF8_W89UXCaJo3gHu4hF5AQTlgK8eV_wP-UyTEsns= + $ curl -s https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dhello=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=DesF8_W89UXCaJo3gHu4hF5AQTlgK8eV_wP-UyTEsns= capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dcapabilities=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQLA=TRqRXJPnl1Xu5WCYWcngUdPHt38RiF9mKDvzm0QciU8=FvjM7sVt9NyPcthiIFzLTjIbezUtsP4Di9PpsfBwVAk= + $ curl -s https://urldefense.proofpoint.com/v2/url?u=http-3A__localhost-3A-24HGPORT_-3Fcmd-3Dcapabilities=DQICaQ=5VD0RTtNlTh3ycd41b3MUw=mEgSWILcY4c4W3zjApBQ
[PATCH evolve] tests: use curl instead of wget
# HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1477315431 25200 # Mon Oct 24 06:23:51 2016 -0700 # Branch stable # Node ID 5fbaca977cd43dfd806a3f452543ef0ed4a4732e # Parent 970a4c13ebc320a034bc0aff21e0ef0a84157a92 tests: use curl instead of wget curl is supplied by default on macOS 10.12, but wget isn't. As curl is easy to install on other OSes, just switch the tests over. diff --git a/tests/test-simple4server-bundle2.t b/tests/test-simple4server-bundle2.t --- a/tests/test-simple4server-bundle2.t +++ b/tests/test-simple4server-bundle2.t @@ -71,12 +71,12 @@ Capacity testing === - $ wget -q -O - http://localhost:$HGPORT/?cmd=hello + $ curl -s http://localhost:$HGPORT/?cmd=hello capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - http://localhost:$HGPORT/?cmd=capabilities + $ curl -s http://localhost:$HGPORT/?cmd=capabilities * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (no-eol) (glob) - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces obsolete @@ -128,14 +128,14 @@ === (used by bitbucket to select which repo use evolve) - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces obsolete phases - $ wget -q -O - http://localhost:$HGPORT/?cmd=hello + $ curl -s http://localhost:$HGPORT/?cmd=hello capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - http://localhost:$HGPORT/?cmd=capabilities + $ curl -s http://localhost:$HGPORT/?cmd=capabilities * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (no-eol) (glob) $ echo '[__temporary__]' >> server/.hg/hgrc @@ -144,7 +144,7 @@ $ hg serve -R server -n test -p $HGPORT -d --pid-file=hg.pid -A access.log -E errors.log $ cat hg.pid >> $DAEMON_PIDS - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces phases @@ -154,13 +154,13 @@ $ hg serve -R server -n test -p $HGPORT -d --pid-file=hg.pid -A access.log -E errors.log $ cat hg.pid >> $DAEMON_PIDS - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces obsolete phases - $ wget -q -O - http://localhost:$HGPORT/?cmd=hello + $ curl -s http://localhost:$HGPORT/?cmd=hello capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - http://localhost:$HGPORT/?cmd=capabilities + $ curl -s http://localhost:$HGPORT/?cmd=capabilities * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (no-eol) (glob) diff --git a/tests/test-simple4server.t b/tests/test-simple4server.t --- a/tests/test-simple4server.t +++ b/tests/test-simple4server.t @@ -73,12 +73,12 @@ Capacity testing === - $ wget -q -O - http://localhost:$HGPORT/?cmd=hello + $ curl -s http://localhost:$HGPORT/?cmd=hello capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (glob) - $ wget -q -O - http://localhost:$HGPORT/?cmd=capabilities + $ curl -s http://localhost:$HGPORT/?cmd=capabilities * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscommon (no-eol) (glob) - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces obsolete @@ -131,14 +131,14 @@ === (used by bitbucket to select which repo use evolve) - $ wget -q -O - "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort + $ curl -s "http://localhost:$HGPORT/?cmd=listkeys=namespaces; | sort bookmarks namespaces obsolete phases - $ wget -q -O - http://localhost:$HGPORT/?cmd=hello + $ curl -s http://localhost:$HGPORT/?cmd=hello capabilities: * _evoext_pushobsmarkers_0 _evoext_pullobsmarkers_0 _evoext_obshash_0 _evoext_obshash_1 _evoext_getbundle_obscom
[PATCH] tests: fix test-casefolding.t
# HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1477063876 -3600 # Fri Oct 21 16:31:16 2016 +0100 # Node ID 1e6f773db8acf006cbfd09205b57e70252faafb8 # Parent 260af19891f2bed679a662be07d1379bb8207592 tests: fix test-casefolding.t The message had changed, but the test was not updated. This test does not run on Linux, but failed on my Mac. diff --git a/tests/test-casefolding.t b/tests/test-casefolding.t --- a/tests/test-casefolding.t +++ b/tests/test-casefolding.t @@ -51,7 +51,8 @@ $ echo b > D/b $ hg ci -Am addb D/b $ hg mv D/b d/b - D/b: not overwriting - file exists + D/b: not overwriting - file already committed + (hg rename --force to replace the file by recording a rename) $ hg mv D/b d/c $ hg st A D/c ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2 v3] evolve: lock the working copy early in next and prev (issue5244)
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1476636773 25200
# Sun Oct 16 09:52:53 2016 -0700
# Node ID 778468237ecf195c4eb1d87bc4f7b5d30e538c63
# Parent ee284d7c5faa5d9f17853f51c17883844b985c58
evolve: lock the working copy early in next and prev (issue5244)
Both next and prev depend on a consistent working copy, but were waiting to
take the lock until they were ready to alter the working copy.
Take the lock before reading the working copy state, and do not release it
until we're definitely not going to change the working copy.
diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -2213,10 +2213,13 @@
"""update to parent revision
Displays the summary line of the destination for clarity."""
-if True:
+wlock = None
+dryrunopt = opts['dry_run']
+if not dryrunopt:
+wlock = repo.wlock()
+try:
wkctx = repo[None]
wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
if len(wparents) != 1:
raise error.Abort('merge in progress')
if not opts['merge']:
@@ -2246,7 +2249,6 @@
ret = hg.update(repo, p.rev())
if not ret:
tr = lock = None
-wlock = repo.wlock()
try:
lock = repo.lock()
tr = repo.transaction('previous')
@@ -2257,7 +2259,8 @@
bmdeactivate(repo)
tr.close()
finally:
-lockmod.release(tr, lock, wlock)
+lockmod.release(tr, lock)
+
displayer.show(p)
return 0
else:
@@ -2265,6 +2268,8 @@
displayer.show(p)
ui.warn(_('multiple parents, explicitly update to one\n'))
return 1
+finally:
+lockmod.release(wlock)
@command('^next',
[('B', 'move-bookmark', False,
@@ -2282,10 +2287,13 @@
Displays the summary line of the destination for clarity.
"""
-if True:
+wlock = None
+dryrunopt = opts['dry_run']
+if not dryrunopt:
+wlock = repo.wlock()
+try:
wkctx = repo[None]
wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
if len(wparents) != 1:
raise error.Abort('merge in progress')
if not opts['merge']:
@@ -2315,7 +2323,6 @@
ret = hg.update(repo, c.rev())
if not ret:
lock = tr = None
-wlock = repo.wlock()
try:
lock = repo.lock()
tr = repo.transaction('next')
@@ -2326,7 +2333,7 @@
bmdeactivate(repo)
tr.close()
finally:
-lockmod.release(tr, lock, wlock)
+lockmod.release(tr, lock)
displayer.show(c)
result = 0
elif children:
@@ -2368,6 +2375,8 @@
return result
return 1
return result
+finally:
+lockmod.release(wlock)
def _reachablefrombookmark(repo, revs, bookmarks):
"""filter revisions and bookmarks reachable from the given bookmark
diff --git a/tests/fake-editor.sh b/tests/fake-editor.sh
new file mode 100755
--- /dev/null
+++ b/tests/fake-editor.sh
@@ -0,0 +1,3 @@
+#!/bin/sh
+sleep 5
+echo "new desc" >> $1
diff --git a/tests/test-prev-next.t b/tests/test-prev-next.t
--- a/tests/test-prev-next.t
+++ b/tests/test-prev-next.t
@@ -206,3 +206,34 @@
move:[5] added d
atop:[6] added b (3)
working directory is now at 47ea25be8aea
+
+prev and next should lock properly against other commands
+
+ $ hg init repo
+ $ cd repo
+ $ HGEDITOR=${TESTDIR}/fake-editor.sh
+ $ echo hi > foo
+ $ hg ci -Am 'one'
+ adding foo
+ $ echo bye > foo
+ $ hg ci -Am 'two'
+
+ $ hg amend --edit &
+ $ sleep 1
+ $ hg prev
+ waiting for lock on working directory of $TESTTMP/repo held by process '*'
on host '*' (glob)
+ got lock after [4-6] seconds (re)
+ 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+ [0] one
+ $ wait
+
+ $ hg amend --edit &
+ $ sleep 1
+ $ hg next --evolve
+ waiting for lock on working directory of $TESTTMP/repo held by process '*'
on host '*' (glob)
+ 1 new unstable changesets
+ got lock after [4-6] seconds (re)
+ move:[2] two
+ atop:[3] one
+ working directory now at a7d885c75614
+ $ wait
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2 v3] evolve: indent cmdnext and cmdprev ready for locking change (issue5244)
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1475939661 25200
# Sat Oct 08 08:14:21 2016 -0700
# Node ID ee284d7c5faa5d9f17853f51c17883844b985c58
# Parent 5383671ef612a1764bbbed13a7ef2d339d0a9c2d
evolve: indent cmdnext and cmdprev ready for locking change (issue5244)
The locking change I'm about to introduce forces an indentation shift. Do
the indentation change with no code change now, to make the next change
easier to review
diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -2213,57 +2213,58 @@
"""update to parent revision
Displays the summary line of the destination for clarity."""
-wkctx = repo[None]
-wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
-if len(wparents) != 1:
-raise error.Abort('merge in progress')
-if not opts['merge']:
-try:
-cmdutil.bailifchanged(repo)
-except error.Abort as exc:
-exc.hint = _('do you want --merge?')
-raise
-
-parents = wparents[0].parents()
-topic = getattr(repo, 'currenttopic', '')
-if topic and not opts.get("no_topic", False):
-parents = [ctx for ctx in parents if ctx.topic() == topic]
-displayer = cmdutil.show_changeset(ui, repo, {'template': shorttemplate})
-if not parents:
-ui.warn(_('no parent in topic "%s"\n') % topic)
-ui.warn(_('(do you want --no-topic)\n'))
-elif len(parents) == 1:
-p = parents[0]
-bm = bmactive(repo)
-shouldmove = opts.get('move_bookmark') and bm is not None
-if dryrunopt:
-ui.write(('hg update %s;\n' % p.rev()))
-if shouldmove:
-ui.write(('hg bookmark %s -r %s;\n' % (bm, p.rev(
+if True:
+wkctx = repo[None]
+wparents = wkctx.parents()
+dryrunopt = opts['dry_run']
+if len(wparents) != 1:
+raise error.Abort('merge in progress')
+if not opts['merge']:
+try:
+cmdutil.bailifchanged(repo)
+except error.Abort as exc:
+exc.hint = _('do you want --merge?')
+raise
+
+parents = wparents[0].parents()
+topic = getattr(repo, 'currenttopic', '')
+if topic and not opts.get("no_topic", False):
+parents = [ctx for ctx in parents if ctx.topic() == topic]
+displayer = cmdutil.show_changeset(ui, repo, {'template':
shorttemplate})
+if not parents:
+ui.warn(_('no parent in topic "%s"\n') % topic)
+ui.warn(_('(do you want --no-topic)\n'))
+elif len(parents) == 1:
+p = parents[0]
+bm = bmactive(repo)
+shouldmove = opts.get('move_bookmark') and bm is not None
+if dryrunopt:
+ui.write(('hg update %s;\n' % p.rev()))
+if shouldmove:
+ui.write(('hg bookmark %s -r %s;\n' % (bm, p.rev(
+else:
+ret = hg.update(repo, p.rev())
+if not ret:
+tr = lock = None
+wlock = repo.wlock()
+try:
+lock = repo.lock()
+tr = repo.transaction('previous')
+if shouldmove:
+repo._bookmarks[bm] = p.node()
+repo._bookmarks.recordchange(tr)
+else:
+bmdeactivate(repo)
+tr.close()
+finally:
+lockmod.release(tr, lock, wlock)
+displayer.show(p)
+return 0
else:
-ret = hg.update(repo, p.rev())
-if not ret:
-tr = lock = None
-wlock = repo.wlock()
-try:
-lock = repo.lock()
-tr = repo.transaction('previous')
-if shouldmove:
-repo._bookmarks[bm] = p.node()
-repo._bookmarks.recordchange(tr)
-else:
-bmdeactivate(repo)
-tr.close()
-finally:
-lockmod.release(tr, lock, wlock)
-displayer.show(p)
-return 0
-else:
-for p in parents:
-displayer.show(p)
-ui.warn(_('multiple parents, explicitly update to one\n'))
-return 1
+for p in parents:
+displayer.show(p)
+ui.warn(_('multiple parents, explicitly update to one\n'))
+return 1
@command('^next',
[('B', 'move-bookmark', False,
@@ -2281,91 +2282,92 @@
Displays the summary line of the destination for clarity.
"""
-wkctx = repo[None]
-wparents = wkctx.parents()
-dryrunopt
Re: [PATCH 1 of 2] evolve: indent cmdnext and cmdprev ready for locking change (issue5244)
On 16/10/2016 15:46, Pierre-Yves David wrote: On 10/16/2016 04:25 PM, Simon Farnsworth wrote: # HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1475939661 25200 # Sat Oct 08 08:14:21 2016 -0700 # Node ID ee284d7c5faa5d9f17853f51c17883844b985c58 # Parent 5383671ef612a1764bbbed13a7ef2d339d0a9c2d evolve: indent cmdnext and cmdprev ready for locking change (issue5244) Looks like an aborted V2. Are you aware of the --confirm flag ? (and the associated patchbomb.confirm=True config) I wasn't. My .hgrc now contains the associated config. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2 v2] evolve: lock the working copy early in next and prev (issue5244)
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1476627823 25200
# Sun Oct 16 07:23:43 2016 -0700
# Node ID 99c560f7d07d3d08b83f5f1802fc329d67e5de2e
# Parent ee284d7c5faa5d9f17853f51c17883844b985c58
evolve: lock the working copy early in next and prev (issue5244)
Both next and prev depend on a consistent working copy, but were waiting to
take the lock until they were ready to alter the working copy.
Take the lock before reading the working copy state, and do not release it
until we're definitely not going to change the working copy.
diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -2213,10 +2213,13 @@
"""update to parent revision
Displays the summary line of the destination for clarity."""
-if True:
+wlock = None
+dryrunopt = opts['dry_run']
+if not dryrunopt:
+wlock = repo.wlock()
+try:
wkctx = repo[None]
wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
if len(wparents) != 1:
raise error.Abort('merge in progress')
if not opts['merge']:
@@ -2246,7 +2249,6 @@
ret = hg.update(repo, p.rev())
if not ret:
tr = lock = None
-wlock = repo.wlock()
try:
lock = repo.lock()
tr = repo.transaction('previous')
@@ -2258,6 +2260,8 @@
tr.close()
finally:
lockmod.release(tr, lock, wlock)
+wlock = None
+
displayer.show(p)
return 0
else:
@@ -2265,6 +2269,8 @@
displayer.show(p)
ui.warn(_('multiple parents, explicitly update to one\n'))
return 1
+finally:
+lockmod.release(wlock)
@command('^next',
[('B', 'move-bookmark', False,
@@ -2282,10 +2288,13 @@
Displays the summary line of the destination for clarity.
"""
-if True:
+wlock = None
+dryrunopt = opts['dry_run']
+if not dryrunopt:
+wlock = repo.wlock()
+try:
wkctx = repo[None]
wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
if len(wparents) != 1:
raise error.Abort('merge in progress')
if not opts['merge']:
@@ -2315,7 +2324,6 @@
ret = hg.update(repo, c.rev())
if not ret:
lock = tr = None
-wlock = repo.wlock()
try:
lock = repo.lock()
tr = repo.transaction('next')
@@ -2327,6 +2335,7 @@
tr.close()
finally:
lockmod.release(tr, lock, wlock)
+wlock = None
displayer.show(c)
result = 0
elif children:
@@ -2368,6 +2377,8 @@
return result
return 1
return result
+finally:
+lockmod.release(wlock)
def _reachablefrombookmark(repo, revs, bookmarks):
"""filter revisions and bookmarks reachable from the given bookmark
diff --git a/tests/fake-editor.sh b/tests/fake-editor.sh
new file mode 100755
--- /dev/null
+++ b/tests/fake-editor.sh
@@ -0,0 +1,3 @@
+#!/bin/sh
+sleep 5
+echo "new desc" >> $1
diff --git a/tests/test-prev-next.t b/tests/test-prev-next.t
--- a/tests/test-prev-next.t
+++ b/tests/test-prev-next.t
@@ -206,3 +206,34 @@
move:[5] added d
atop:[6] added b (3)
working directory is now at 47ea25be8aea
+
+prev and next should lock properly against other commands
+
+ $ hg init repo
+ $ cd repo
+ $ HGEDITOR=${TESTDIR}/fake-editor.sh
+ $ echo hi > foo
+ $ hg ci -Am 'one'
+ adding foo
+ $ echo bye > foo
+ $ hg ci -Am 'two'
+
+ $ hg amend --edit &
+ $ sleep 1
+ $ hg prev
+ waiting for lock on working directory of $TESTTMP/repo held by process '*'
on host '*' (glob)
+ got lock after [4-6] seconds (re)
+ 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+ [0] one
+ $ wait
+
+ $ hg amend --edit &
+ $ sleep 1
+ $ hg next --evolve
+ waiting for lock on working directory of $TESTTMP/repo held by process '*'
on host '*' (glob)
+ 1 new unstable changesets
+ got lock after [4-6] seconds (re)
+ move:[2] two
+ atop:[3] one
+ working directory now at a7d885c75614
+ $ wait
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2 v2] evolve: indent cmdnext and cmdprev ready for locking change (issue5244)
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1475939661 25200
# Sat Oct 08 08:14:21 2016 -0700
# Node ID ee284d7c5faa5d9f17853f51c17883844b985c58
# Parent 5383671ef612a1764bbbed13a7ef2d339d0a9c2d
evolve: indent cmdnext and cmdprev ready for locking change (issue5244)
The locking change I'm about to introduce forces an indentation shift. Do
the indentation change with no code change now, to make the next change
easier to review
diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -2213,57 +2213,58 @@
"""update to parent revision
Displays the summary line of the destination for clarity."""
-wkctx = repo[None]
-wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
-if len(wparents) != 1:
-raise error.Abort('merge in progress')
-if not opts['merge']:
-try:
-cmdutil.bailifchanged(repo)
-except error.Abort as exc:
-exc.hint = _('do you want --merge?')
-raise
-
-parents = wparents[0].parents()
-topic = getattr(repo, 'currenttopic', '')
-if topic and not opts.get("no_topic", False):
-parents = [ctx for ctx in parents if ctx.topic() == topic]
-displayer = cmdutil.show_changeset(ui, repo, {'template': shorttemplate})
-if not parents:
-ui.warn(_('no parent in topic "%s"\n') % topic)
-ui.warn(_('(do you want --no-topic)\n'))
-elif len(parents) == 1:
-p = parents[0]
-bm = bmactive(repo)
-shouldmove = opts.get('move_bookmark') and bm is not None
-if dryrunopt:
-ui.write(('hg update %s;\n' % p.rev()))
-if shouldmove:
-ui.write(('hg bookmark %s -r %s;\n' % (bm, p.rev(
+if True:
+wkctx = repo[None]
+wparents = wkctx.parents()
+dryrunopt = opts['dry_run']
+if len(wparents) != 1:
+raise error.Abort('merge in progress')
+if not opts['merge']:
+try:
+cmdutil.bailifchanged(repo)
+except error.Abort as exc:
+exc.hint = _('do you want --merge?')
+raise
+
+parents = wparents[0].parents()
+topic = getattr(repo, 'currenttopic', '')
+if topic and not opts.get("no_topic", False):
+parents = [ctx for ctx in parents if ctx.topic() == topic]
+displayer = cmdutil.show_changeset(ui, repo, {'template':
shorttemplate})
+if not parents:
+ui.warn(_('no parent in topic "%s"\n') % topic)
+ui.warn(_('(do you want --no-topic)\n'))
+elif len(parents) == 1:
+p = parents[0]
+bm = bmactive(repo)
+shouldmove = opts.get('move_bookmark') and bm is not None
+if dryrunopt:
+ui.write(('hg update %s;\n' % p.rev()))
+if shouldmove:
+ui.write(('hg bookmark %s -r %s;\n' % (bm, p.rev(
+else:
+ret = hg.update(repo, p.rev())
+if not ret:
+tr = lock = None
+wlock = repo.wlock()
+try:
+lock = repo.lock()
+tr = repo.transaction('previous')
+if shouldmove:
+repo._bookmarks[bm] = p.node()
+repo._bookmarks.recordchange(tr)
+else:
+bmdeactivate(repo)
+tr.close()
+finally:
+lockmod.release(tr, lock, wlock)
+displayer.show(p)
+return 0
else:
-ret = hg.update(repo, p.rev())
-if not ret:
-tr = lock = None
-wlock = repo.wlock()
-try:
-lock = repo.lock()
-tr = repo.transaction('previous')
-if shouldmove:
-repo._bookmarks[bm] = p.node()
-repo._bookmarks.recordchange(tr)
-else:
-bmdeactivate(repo)
-tr.close()
-finally:
-lockmod.release(tr, lock, wlock)
-displayer.show(p)
-return 0
-else:
-for p in parents:
-displayer.show(p)
-ui.warn(_('multiple parents, explicitly update to one\n'))
-return 1
+for p in parents:
+displayer.show(p)
+ui.warn(_('multiple parents, explicitly update to one\n'))
+return 1
@command('^next',
[('B', 'move-bookmark', False,
@@ -2281,91 +2282,92 @@
Displays the summary line of the destination for clarity.
"""
-wkctx = repo[None]
-wparents = wkctx.parents()
-dryrunopt
Re: [PATCH 2 of 2] evolve: lock the working copy early in next and prev (issue5244)
On 16/10/2016 12:43, Pierre-Yves David wrote:
On 10/08/2016 05:18 PM, Simon Farnsworth wrote:
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1475939634 25200
# Sat Oct 08 08:13:54 2016 -0700
# Node ID 8a0c9c0158b3e9574a4571af3dce9978844b825d
# Parent ee284d7c5faa5d9f17853f51c17883844b985c58
evolve: lock the working copy early in next and prev (issue5244)
Both next and prev depend on a consistent working copy, but were
waiting to
take the lock until they were ready to alter the working copy.
Take the lock before reading the working copy state, and do not
release it
until we're definitely not going to change the working copy.
The fix approach seems good, but the implementation is a bit strangeto
me. See inline.
I can remove the early unlocks, but the goal is to unlock early when we
no longer need the locks.
diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -2213,10 +2213,13 @@
"""update to parent revision
Displays the summary line of the destination for clarity."""
-if True:
+wlock = None
+dryrunopt = opts['dry_run']
+if not dryrunopt:
+wlock = repo.wlock()
+try:
wkctx = repo[None]
wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
if len(wparents) != 1:
raise error.Abort('merge in progress')
if not opts['merge']:
@@ -2246,7 +2249,6 @@
ret = hg.update(repo, p.rev())
if not ret:
tr = lock = None
-wlock = repo.wlock()
try:
lock = repo.lock()
tr = repo.transaction('previous')
@@ -2258,13 +2260,22 @@
tr.close()
finally:
lockmod.release(tr, lock, wlock)
+wlock = None
+else:
+lockmod.release(wlock)
+wlock = None
+
displayer.show(p)
return 0
else:
+lockmod.release(wlock)
+wlock = None
Why are we issuing multiple lockmode.release call instead of just using
the final finally clause ?
Each lockmod.release() happens at a point where we no longer need the
lock for correctness (and in the old code, didn't have the lock at all).
The aim is to minimise the time we're holding the lock for. I can easily
redo this with wlock held for the entire duration of the command if
you'd prefer that version?
for p in parents:
displayer.show(p)
ui.warn(_('multiple parents, explicitly update to one\n'))
return 1
+finally:
+lockmod.release(wlock)
@command('^next',
[('B', 'move-bookmark', False,
@@ -2282,10 +2293,13 @@
Displays the summary line of the destination for clarity.
"""
-if True:
+wlock = None
+dryrunopt = opts['dry_run']
+if not dryrunopt:
+wlock = repo.wlock()
+try:
wkctx = repo[None]
wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
if len(wparents) != 1:
raise error.Abort('merge in progress')
if not opts['merge']:
@@ -2315,7 +2329,6 @@
ret = hg.update(repo, c.rev())
if not ret:
lock = tr = None
-wlock = repo.wlock()
try:
lock = repo.lock()
tr = repo.transaction('next')
@@ -2327,15 +2340,23 @@
tr.close()
finally:
lockmod.release(tr, lock, wlock)
+wlock = None
+else:
+lockmod.release(wlock)
+wlock = None
displayer.show(c)
result = 0
elif children:
+lockmod.release(wlock)
+wlock = None
ui.warn(_("ambigious next changeset:\n"))
for c in children:
displayer.show(c)
ui.warn(_('explicitly update to one of them\n'))
result = 1
else:
+lockmod.release(wlock)
+wlock = None
aspchildren = _aspiringchildren(repo, [repo['.'].rev()])
if topic:
filtered.extend(repo[c] for c in children
@@ -2368,6 +2389,8 @@
return result
return 1
return result
+finally:
+lockmod.release(wlock)
def _reachablefrombookmark(repo, revs, bookmarks):
"""filter revisions and bookmarks reachable from the given bookmark
diff --git a/tests/fake-editor.sh b/tests/fake-editor.sh
new file mode 100755
--- /dev/null
+++ b/tests/fake-editor.sh
@@ -0,0 +1,3 @@
+#!/bin/sh
+sleep 5
+echo "new desc" >> $1
diff --gi
[PATCH] templater: handle division by zero in arithmetic
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1476025760 25200
# Sun Oct 09 08:09:20 2016 -0700
# Node ID ad830281f2cd1a6fd2249813a8a1ceddf3d0d2e8
# Parent 8e42dfde93d10e099040e9b57c70b7774235d883
templater: handle division by zero in arithmetic
For now, just turn it to an abort.
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -439,7 +439,10 @@
_('arithmetic only defined on integers'))
right = evalinteger(context, mapping, right,
_('arithmetic only defined on integers'))
-return func(left, right)
+try:
+return func(left, right)
+except ZeroDivisionError:
+raise error.Abort(_('division by zero is not defined'))
def buildfunc(exp, context):
n = getsymbol(exp[1])
@@ -741,12 +744,8 @@
# i18n: "mod" is a keyword
raise error.ParseError(_("mod expects two arguments"))
-left = evalinteger(context, mapping, args[0],
- _('arithmetic only defined on integers'))
-right = evalinteger(context, mapping, args[1],
-_('arithmetic only defined on integers'))
-
-return left % right
+func = lambda a, b: a % b
+return runarithmetic(context, mapping, (func, args[0], args[1]))
@templatefunc('relpath(path)')
def relpath(context, mapping, args):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH v2] templater: provide arithmetic operations on integers
On 09/10/2016 16:51, Yuya Nishihara wrote:
On Sun, 9 Oct 2016 05:53:25 -0700, Simon Farnsworth wrote:
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1476017464 25200
# Sun Oct 09 05:51:04 2016 -0700
# Node ID 2e2c959de0fe2c17bf6c5f47c01035a36f13c593
# Parent dbcef8918bbdd8a64d9f79a37bcfa284a26f3a39
templater: provide arithmetic operations on integers
Nice. Queued slight modified version, thanks.
We'll need to catch ZeroDivisionError as a follow-up.
Follow-up incoming to turn ZeroDivisionError into an abort (and to reuse
runarithmetic() in mod()).
+But negate binds closer still:
+
+ $ hg debugtemplate -r0 -v '{1-3|stringify}\n'
+ (template
+(-
+ ('integer', '1')
+ (|
+('integer', '3')
+('symbol', 'stringify')))
+('string', '\n'))
+ hg: parse error: arithmetic only defined on integers
+ [255]
For the record, this fails because '3' is taken as a keyword (for backward
compatibility), and evaluated to ''.
Indeed - the parse tree is the important bit here, not the failure, as
it shows that we've parsed '1-3' as a subtraction with the right
precedence, not as '1' then '-3'.
--
Simon Farnsworth
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH v2] templater: provide arithmetic operations on integers
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1476017464 25200
# Sun Oct 09 05:51:04 2016 -0700
# Node ID 2e2c959de0fe2c17bf6c5f47c01035a36f13c593
# Parent dbcef8918bbdd8a64d9f79a37bcfa284a26f3a39
templater: provide arithmetic operations on integers
The termwidth template keyword is of limited use without some way to ensure
that margins are respected.
Provide a full set of arithmetic operators (four basic operations plus the
mod function, defined to match Python's // for division), so that you can
create termwidth based layouts that match the user's terminal size
diff --git a/mercurial/help/templates.txt b/mercurial/help/templates.txt
--- a/mercurial/help/templates.txt
+++ b/mercurial/help/templates.txt
@@ -43,6 +43,12 @@
.. functionsmarker
+We provide a limited set of infix arithmetic operations on integers:
+ + for addition
+ - for subtraction
+ * for multiplication
+ / for floor division (division rounded to integer nearest -infinity)
+Division fulfils the law x = x / y + mod(x, y).
Also, for any expression that returns a list, there is a list operator::
expr % "{template}"
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -33,6 +33,10 @@
"|": (5, None, None, ("|", 5), None),
"%": (6, None, None, ("%", 6), None),
")": (0, None, None, None, None),
+"+": (3, None, None, ("+", 3), None),
+"-": (3, None, ("negate", 10), ("-", 3), None),
+"*": (4, None, None, ("*", 4), None),
+"/": (4, None, None, ("/", 4), None),
"integer": (0, "integer", None, None, None),
"symbol": (0, "symbol", None, None, None),
"string": (0, "string", None, None, None),
@@ -48,7 +52,7 @@
c = program[pos]
if c.isspace(): # skip inter-token whitespace
pass
-elif c in "(,)%|": # handle simple operators
+elif c in "(,)%|+-*/": # handle simple operators
yield (c, None, pos)
elif c in '"\'': # handle quoted templates
s = pos + 1
@@ -70,13 +74,8 @@
pos += 1
else:
raise error.ParseError(_("unterminated string"), s)
-elif c.isdigit() or c == '-':
+elif c.isdigit():
s = pos
-if c == '-': # simply take negate operator as part of integer
-pos += 1
-if pos >= end or not program[pos].isdigit():
-raise error.ParseError(_("integer literal without digits"), s)
-pos += 1
while pos < end:
d = program[pos]
if not d.isdigit():
@@ -420,6 +419,28 @@
# If so, return the expanded value.
yield i
+def buildnegate(exp, context):
+arg = compileexp(exp[1], context, exprmethods)
+return (runnegate, arg)
+
+def runnegate(context, mapping, data):
+data = evalinteger(context, mapping, data,
+_('negation needs an integer argument'))
+return -data
+
+def buildarithmetic(exp, context, func):
+left = compileexp(exp[1], context, exprmethods)
+right = compileexp(exp[2], context, exprmethods)
+return (runarithmetic, (func, left, right))
+
+def runarithmetic(context, mapping, data):
+func, left, right = data
+left = evalinteger(context, mapping, left,
+_('arithmetic only defined on integers'))
+right = evalinteger(context, mapping, right,
+_('arithmetic only defined on integers'))
+return func(left, right)
+
def buildfunc(exp, context):
n = getsymbol(exp[1])
args = [compileexp(x, context, exprmethods) for x in getlist(exp[2])]
@@ -713,6 +734,20 @@
tzoffset = util.makedate()[1]
return (date[0], tzoffset)
+@templatefunc('mod(a, b)')
+def mod(context, mapping, args):
+"""Calculate a mod b such that a / b + a mod b == a"""
+if not len(args) == 2:
+# i18n: "mod" is a keyword
+raise error.ParseError(_("mod expects two arguments"))
+
+left = evalinteger(context, mapping, args[0],
+_('arithmetic only defined on integers'))
+right = evalinteger(context, mapping, args[1],
+_('arithmetic only defined on integers'))
+
+return left % right
+
@templatefunc('revset(query[, formatargs...])')
def revset(context, mapping, args):
"""Execute a revision set query. See
@@ -906,6 +941,7 @@
# methods to interpret function arguments or inner expressions (e.g. {_(x)})
exprmethods = {
"integer": lambda e, c: (runinteger, e[1]),
+"negate": lambda e, c: (run
Re: [PATCH RFC] templater: provide ring operations on integers
This is RFC because I have no idea what I'm doing in the parser. If
someone at the sprint has 5 minutes available to educate me, I can
update this to a better version.
On 08/10/2016 18:24, Simon Farnsworth wrote:
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1475943860 25200
# Sat Oct 08 09:24:20 2016 -0700
# Node ID e89699ba5c9f0bf883bfae7c485e50219b90b2f9
# Parent 91a3c58ecf938ed675f5364b88f0d663f12b0047
templater: provide ring operations on integers
The termwidth template keyword is of limited use without some way to ensure
that margins are respected. Provide the three core ring operations -
addition, subtraction and multiplication. We can extend to division and
modulus later if necessary.
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -33,6 +33,9 @@
"|": (5, None, None, ("|", 5), None),
"%": (6, None, None, ("%", 6), None),
")": (0, None, None, None, None),
+"+": (10, None, None, ("+", 10), None),
+"-": (10, None, ("negate", 10), ("-", 10), None),
+"*": (12, None, None, ("*", 12), None),
"integer": (0, "integer", None, None, None),
"symbol": (0, "symbol", None, None, None),
"string": (0, "string", None, None, None),
@@ -48,7 +51,7 @@
c = program[pos]
if c.isspace(): # skip inter-token whitespace
pass
-elif c in "(,)%|": # handle simple operators
+elif c in "(,)%|+-*": # handle simple operators
yield (c, None, pos)
elif c in '"\'': # handle quoted templates
s = pos + 1
@@ -70,7 +73,7 @@
pos += 1
else:
raise error.ParseError(_("unterminated string"), s)
-elif c.isdigit() or c == '-':
+elif c.isdigit():
s = pos
if c == '-': # simply take negate operator as part of integer
pos += 1
@@ -420,6 +423,29 @@
# If so, return the expanded value.
yield i
+def buildnegate(exp, context):
+arg = compileexp(exp[1], context, exprmethods)
+return (runnegate, arg)
+
+def runnegate(context, mapping, data):
+data = evalinteger(context, mapping, data,
+_('negation needs an integer argument'))
+return -data
+
+
+def buildarithmetic(exp, context, func):
+left = compileexp(exp[1], context, exprmethods)
+right = compileexp(exp[2], context, exprmethods)
+return (runarithmetic, (func, left, right))
+
+def runarithmetic(context, mapping, data):
+func, left, right = data
+left = evalinteger(context, mapping, left,
+_('arithmetic only defined on integers'))
+right = evalinteger(context, mapping, right,
+_('arithmetic only defined on integers'))
+return func(left, right)
+
def buildfunc(exp, context):
n = getsymbol(exp[1])
args = [compileexp(x, context, exprmethods) for x in getlist(exp[2])]
@@ -906,6 +932,7 @@
# methods to interpret function arguments or inner expressions (e.g. {_(x)})
exprmethods = {
"integer": lambda e, c: (runinteger, e[1]),
+"negate": lambda e, c: (runinteger, e[1]),
"string": lambda e, c: (runstring, e[1]),
"symbol": lambda e, c: (runsymbol, e[1]),
"template": buildtemplate,
@@ -914,6 +941,10 @@
"|": buildfilter,
"%": buildmap,
"func": buildfunc,
+"+": lambda e, c: buildarithmetic(e, c, lambda a, b: a + b),
+"-": lambda e, c: buildarithmetic(e, c, lambda a, b: a - b),
+"negate": buildnegate,
+"*": lambda e, c: buildarithmetic(e, c, lambda a, b: a * b),
}
# methods to interpret top-level template (e.g. {x}, {x|_}, {x % "y"})
diff --git a/tests/test-command-template.t b/tests/test-command-template.t
--- a/tests/test-command-template.t
+++ b/tests/test-command-template.t
@@ -5,6 +5,8 @@
$ echo line 1 > b
$ echo line 2 >> b
$ hg commit -l b -d '100 0' -u 'User Name <user@hostname>'
+ $ hg log -T '{date(date, "%s") + 1} {date(date, "%s") - 2 * 1}\n'
+ 101 98
$ hg add b
$ echo other 1 > c
@@ -2890,14 +2892,15 @@
$ hg debugtemplate -v '{(-4)}\n'
(template
(group
- ('integer', '-4'))
+ (negate
+('integer', '4')))
('string', '\n'))
-4
$ hg debugtemplate '{(-)}\n'
- hg: parse error at 2: integer literal without digits
+ hg: parse error at 3: not a prefix: )
[255]
$ hg debugtemplate '{(-a)}\n'
- hg: parse error at 2: integer literal without digits
+ hg: parse error: negat
[PATCH RFC] templater: provide ring operations on integers
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1475943860 25200
# Sat Oct 08 09:24:20 2016 -0700
# Node ID e89699ba5c9f0bf883bfae7c485e50219b90b2f9
# Parent 91a3c58ecf938ed675f5364b88f0d663f12b0047
templater: provide ring operations on integers
The termwidth template keyword is of limited use without some way to ensure
that margins are respected. Provide the three core ring operations -
addition, subtraction and multiplication. We can extend to division and
modulus later if necessary.
diff --git a/mercurial/templater.py b/mercurial/templater.py
--- a/mercurial/templater.py
+++ b/mercurial/templater.py
@@ -33,6 +33,9 @@
"|": (5, None, None, ("|", 5), None),
"%": (6, None, None, ("%", 6), None),
")": (0, None, None, None, None),
+"+": (10, None, None, ("+", 10), None),
+"-": (10, None, ("negate", 10), ("-", 10), None),
+"*": (12, None, None, ("*", 12), None),
"integer": (0, "integer", None, None, None),
"symbol": (0, "symbol", None, None, None),
"string": (0, "string", None, None, None),
@@ -48,7 +51,7 @@
c = program[pos]
if c.isspace(): # skip inter-token whitespace
pass
-elif c in "(,)%|": # handle simple operators
+elif c in "(,)%|+-*": # handle simple operators
yield (c, None, pos)
elif c in '"\'': # handle quoted templates
s = pos + 1
@@ -70,7 +73,7 @@
pos += 1
else:
raise error.ParseError(_("unterminated string"), s)
-elif c.isdigit() or c == '-':
+elif c.isdigit():
s = pos
if c == '-': # simply take negate operator as part of integer
pos += 1
@@ -420,6 +423,29 @@
# If so, return the expanded value.
yield i
+def buildnegate(exp, context):
+arg = compileexp(exp[1], context, exprmethods)
+return (runnegate, arg)
+
+def runnegate(context, mapping, data):
+data = evalinteger(context, mapping, data,
+_('negation needs an integer argument'))
+return -data
+
+
+def buildarithmetic(exp, context, func):
+left = compileexp(exp[1], context, exprmethods)
+right = compileexp(exp[2], context, exprmethods)
+return (runarithmetic, (func, left, right))
+
+def runarithmetic(context, mapping, data):
+func, left, right = data
+left = evalinteger(context, mapping, left,
+_('arithmetic only defined on integers'))
+right = evalinteger(context, mapping, right,
+_('arithmetic only defined on integers'))
+return func(left, right)
+
def buildfunc(exp, context):
n = getsymbol(exp[1])
args = [compileexp(x, context, exprmethods) for x in getlist(exp[2])]
@@ -906,6 +932,7 @@
# methods to interpret function arguments or inner expressions (e.g. {_(x)})
exprmethods = {
"integer": lambda e, c: (runinteger, e[1]),
+"negate": lambda e, c: (runinteger, e[1]),
"string": lambda e, c: (runstring, e[1]),
"symbol": lambda e, c: (runsymbol, e[1]),
"template": buildtemplate,
@@ -914,6 +941,10 @@
"|": buildfilter,
"%": buildmap,
"func": buildfunc,
+"+": lambda e, c: buildarithmetic(e, c, lambda a, b: a + b),
+"-": lambda e, c: buildarithmetic(e, c, lambda a, b: a - b),
+"negate": buildnegate,
+"*": lambda e, c: buildarithmetic(e, c, lambda a, b: a * b),
}
# methods to interpret top-level template (e.g. {x}, {x|_}, {x % "y"})
diff --git a/tests/test-command-template.t b/tests/test-command-template.t
--- a/tests/test-command-template.t
+++ b/tests/test-command-template.t
@@ -5,6 +5,8 @@
$ echo line 1 > b
$ echo line 2 >> b
$ hg commit -l b -d '100 0' -u 'User Name <user@hostname>'
+ $ hg log -T '{date(date, "%s") + 1} {date(date, "%s") - 2 * 1}\n'
+ 101 98
$ hg add b
$ echo other 1 > c
@@ -2890,14 +2892,15 @@
$ hg debugtemplate -v '{(-4)}\n'
(template
(group
- ('integer', '-4'))
+ (negate
+('integer', '4')))
('string', '\n'))
-4
$ hg debugtemplate '{(-)}\n'
- hg: parse error at 2: integer literal without digits
+ hg: parse error at 3: not a prefix: )
[255]
$ hg debugtemplate '{(-a)}\n'
- hg: parse error at 2: integer literal without digits
+ hg: parse error: negation needs an integer argument
[255]
top-level integer literal is interpreted as symbol (i.e. variable name):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2] evolve: indent cmdnext and cmdprev ready for locking change (issue5244)
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1475939661 25200
# Sat Oct 08 08:14:21 2016 -0700
# Node ID ee284d7c5faa5d9f17853f51c17883844b985c58
# Parent 5383671ef612a1764bbbed13a7ef2d339d0a9c2d
evolve: indent cmdnext and cmdprev ready for locking change (issue5244)
The locking change I'm about to introduce forces an indentation shift. Do
the indentation change with no code change now, to make the next change
easier to review
diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -2213,57 +2213,58 @@
"""update to parent revision
Displays the summary line of the destination for clarity."""
-wkctx = repo[None]
-wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
-if len(wparents) != 1:
-raise error.Abort('merge in progress')
-if not opts['merge']:
-try:
-cmdutil.bailifchanged(repo)
-except error.Abort as exc:
-exc.hint = _('do you want --merge?')
-raise
-
-parents = wparents[0].parents()
-topic = getattr(repo, 'currenttopic', '')
-if topic and not opts.get("no_topic", False):
-parents = [ctx for ctx in parents if ctx.topic() == topic]
-displayer = cmdutil.show_changeset(ui, repo, {'template': shorttemplate})
-if not parents:
-ui.warn(_('no parent in topic "%s"\n') % topic)
-ui.warn(_('(do you want --no-topic)\n'))
-elif len(parents) == 1:
-p = parents[0]
-bm = bmactive(repo)
-shouldmove = opts.get('move_bookmark') and bm is not None
-if dryrunopt:
-ui.write(('hg update %s;\n' % p.rev()))
-if shouldmove:
-ui.write(('hg bookmark %s -r %s;\n' % (bm, p.rev(
+if True:
+wkctx = repo[None]
+wparents = wkctx.parents()
+dryrunopt = opts['dry_run']
+if len(wparents) != 1:
+raise error.Abort('merge in progress')
+if not opts['merge']:
+try:
+cmdutil.bailifchanged(repo)
+except error.Abort as exc:
+exc.hint = _('do you want --merge?')
+raise
+
+parents = wparents[0].parents()
+topic = getattr(repo, 'currenttopic', '')
+if topic and not opts.get("no_topic", False):
+parents = [ctx for ctx in parents if ctx.topic() == topic]
+displayer = cmdutil.show_changeset(ui, repo, {'template':
shorttemplate})
+if not parents:
+ui.warn(_('no parent in topic "%s"\n') % topic)
+ui.warn(_('(do you want --no-topic)\n'))
+elif len(parents) == 1:
+p = parents[0]
+bm = bmactive(repo)
+shouldmove = opts.get('move_bookmark') and bm is not None
+if dryrunopt:
+ui.write(('hg update %s;\n' % p.rev()))
+if shouldmove:
+ui.write(('hg bookmark %s -r %s;\n' % (bm, p.rev(
+else:
+ret = hg.update(repo, p.rev())
+if not ret:
+tr = lock = None
+wlock = repo.wlock()
+try:
+lock = repo.lock()
+tr = repo.transaction('previous')
+if shouldmove:
+repo._bookmarks[bm] = p.node()
+repo._bookmarks.recordchange(tr)
+else:
+bmdeactivate(repo)
+tr.close()
+finally:
+lockmod.release(tr, lock, wlock)
+displayer.show(p)
+return 0
else:
-ret = hg.update(repo, p.rev())
-if not ret:
-tr = lock = None
-wlock = repo.wlock()
-try:
-lock = repo.lock()
-tr = repo.transaction('previous')
-if shouldmove:
-repo._bookmarks[bm] = p.node()
-repo._bookmarks.recordchange(tr)
-else:
-bmdeactivate(repo)
-tr.close()
-finally:
-lockmod.release(tr, lock, wlock)
-displayer.show(p)
-return 0
-else:
-for p in parents:
-displayer.show(p)
-ui.warn(_('multiple parents, explicitly update to one\n'))
-return 1
+for p in parents:
+displayer.show(p)
+ui.warn(_('multiple parents, explicitly update to one\n'))
+return 1
@command('^next',
[('B', 'move-bookmark', False,
@@ -2281,91 +2282,92 @@
Displays the summary line of the destination for clarity.
"""
-wkctx = repo[None]
-wparents = wkctx.parents()
-dryrunopt
[PATCH 2 of 2] evolve: lock the working copy early in next and prev (issue5244)
# HG changeset patch
# User Simon Farnsworth <simon...@fb.com>
# Date 1475939634 25200
# Sat Oct 08 08:13:54 2016 -0700
# Node ID 8a0c9c0158b3e9574a4571af3dce9978844b825d
# Parent ee284d7c5faa5d9f17853f51c17883844b985c58
evolve: lock the working copy early in next and prev (issue5244)
Both next and prev depend on a consistent working copy, but were waiting to
take the lock until they were ready to alter the working copy.
Take the lock before reading the working copy state, and do not release it
until we're definitely not going to change the working copy.
diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -2213,10 +2213,13 @@
"""update to parent revision
Displays the summary line of the destination for clarity."""
-if True:
+wlock = None
+dryrunopt = opts['dry_run']
+if not dryrunopt:
+wlock = repo.wlock()
+try:
wkctx = repo[None]
wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
if len(wparents) != 1:
raise error.Abort('merge in progress')
if not opts['merge']:
@@ -2246,7 +2249,6 @@
ret = hg.update(repo, p.rev())
if not ret:
tr = lock = None
-wlock = repo.wlock()
try:
lock = repo.lock()
tr = repo.transaction('previous')
@@ -2258,13 +2260,22 @@
tr.close()
finally:
lockmod.release(tr, lock, wlock)
+wlock = None
+else:
+lockmod.release(wlock)
+wlock = None
+
displayer.show(p)
return 0
else:
+lockmod.release(wlock)
+wlock = None
for p in parents:
displayer.show(p)
ui.warn(_('multiple parents, explicitly update to one\n'))
return 1
+finally:
+lockmod.release(wlock)
@command('^next',
[('B', 'move-bookmark', False,
@@ -2282,10 +2293,13 @@
Displays the summary line of the destination for clarity.
"""
-if True:
+wlock = None
+dryrunopt = opts['dry_run']
+if not dryrunopt:
+wlock = repo.wlock()
+try:
wkctx = repo[None]
wparents = wkctx.parents()
-dryrunopt = opts['dry_run']
if len(wparents) != 1:
raise error.Abort('merge in progress')
if not opts['merge']:
@@ -2315,7 +2329,6 @@
ret = hg.update(repo, c.rev())
if not ret:
lock = tr = None
-wlock = repo.wlock()
try:
lock = repo.lock()
tr = repo.transaction('next')
@@ -2327,15 +2340,23 @@
tr.close()
finally:
lockmod.release(tr, lock, wlock)
+wlock = None
+else:
+lockmod.release(wlock)
+wlock = None
displayer.show(c)
result = 0
elif children:
+lockmod.release(wlock)
+wlock = None
ui.warn(_("ambigious next changeset:\n"))
for c in children:
displayer.show(c)
ui.warn(_('explicitly update to one of them\n'))
result = 1
else:
+lockmod.release(wlock)
+wlock = None
aspchildren = _aspiringchildren(repo, [repo['.'].rev()])
if topic:
filtered.extend(repo[c] for c in children
@@ -2368,6 +2389,8 @@
return result
return 1
return result
+finally:
+lockmod.release(wlock)
def _reachablefrombookmark(repo, revs, bookmarks):
"""filter revisions and bookmarks reachable from the given bookmark
diff --git a/tests/fake-editor.sh b/tests/fake-editor.sh
new file mode 100755
--- /dev/null
+++ b/tests/fake-editor.sh
@@ -0,0 +1,3 @@
+#!/bin/sh
+sleep 5
+echo "new desc" >> $1
diff --git a/tests/test-prev-next.t b/tests/test-prev-next.t
--- a/tests/test-prev-next.t
+++ b/tests/test-prev-next.t
@@ -206,3 +206,34 @@
move:[5] added d
atop:[6] added b (3)
working directory is now at 47ea25be8aea
+
+prev and next should lock properly against other commands
+
+ $ hg init repo
+ $ cd repo
+ $ HGEDITOR=${TESTDIR}/fake-editor.sh
+ $ echo hi > foo
+ $ hg ci -Am 'one'
+ adding foo
+ $ echo bye > foo
+ $ hg ci -Am 'two'
+
+ $ hg amend --edit &
+ $ sleep 1
+ $ hg prev
+ waiting for lock on working directory of $TESTTMP/repo held by process '*'
on host '*' (glob)
+ got lock after [4-6] seconds (re)
+ 1 files updated, 0 files merged, 0 files removed, 0 files
Re: [PATCH] merge: add conflict labels to merge command
On 08/10/2016 11:37, Pierre-Yves David wrote: On 10/08/2016 11:35 AM, Simon Farnsworth wrote: On 08/10/2016 11:31, Pierre-Yves David wrote: On 10/08/2016 10:26 AM, Simon Farnsworth wrote: # HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1475855510 25200 # Fri Oct 07 08:51:50 2016 -0700 # Node ID 9293c425580b06b3ab10f9648b35e4a982cc5c67 # Parent 91a3c58ecf938ed675f5364b88f0d663f12b0047 merge: add conflict labels to merge command Pushed, thanks, test-subrepo-git.t and test-subrepo.t says hi. Cheers, They pass for me without fixups - I'm running git v2.9.3 and running the test suite with ./run-tests.py -l -j24. I see a failure in test-paths.t, because zeroconf assumes I have IPv4 locally (I don't). Even if I run those two tests individually, I don't see issues. I'm running 2.9.3. Can you check the changesets I pushed on hg-committed? They look OK - I can't work out what's triggering the discrepancy between tests on my system and tests on yours. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] merge: add conflict labels to merge command
On 08/10/2016 11:31, Pierre-Yves David wrote: On 10/08/2016 10:26 AM, Simon Farnsworth wrote: # HG changeset patch # User Simon Farnsworth <simon...@fb.com> # Date 1475855510 25200 # Fri Oct 07 08:51:50 2016 -0700 # Node ID 9293c425580b06b3ab10f9648b35e4a982cc5c67 # Parent 91a3c58ecf938ed675f5364b88f0d663f12b0047 merge: add conflict labels to merge command Pushed, thanks, test-subrepo-git.t and test-subrepo.t says hi. Cheers, They pass for me without fixups - I'm running git v2.9.3 and running the test suite with ./run-tests.py -l -j24. I see a failure in test-paths.t, because zeroconf assumes I have IPv4 locally (I don't). Even if I run those two tests individually, I don't see issues. -- Simon Farnsworth ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
