[PATCH V2] tests: add diff color trailing whitespace test
# HG changeset patch # User Sune Foldager # Date 1531318293 -7200 # Wed Jul 11 16:11:33 2018 +0200 # Node ID 95fc4453e33a79acca36f978019a05ff9f4ae04a # Parent 4d5fb4062f0bb159230062701461fa6cab9b539b tests: add diff color trailing whitespace test diff --git a/tests/test-diff-color.t b/tests/test-diff-color.t --- a/tests/test-diff-color.t +++ b/tests/test-diff-color.t @@ -51,6 +51,27 @@ default context a c +trailing whitespace + + $ cp a a.orig + $ sed 's/^dd$/dd \r/' a >a.new + $ mv a.new a + $ hg diff --nodates + \x1b[0;1mdiff -r cf9f4ba66af2 a\x1b[0m (esc) + \x1b[0;31;1m--- a/a\x1b[0m (esc) + \x1b[0;32;1m+++ b/a\x1b[0m (esc) + \x1b[0;35m@@ -2,7 +2,7 @@\x1b[0m (esc) + c + a + a + \x1b[0;31m-b\x1b[0m (esc) + \x1b[0;32m+dd\x1b[0m\x1b[0;1;41m \x1b[0m\r (esc) + a + a + c + + $ mv a.orig a + (check that 'ui.color=yes' match '--color=auto') $ hg diff --nodates --config ui.formatted=no ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] patch: don't separate \r and \n when colorizing diff output
> On 11 Jul 2018, at 17.21, Augie Fackler wrote: > > > >> On Jul 11, 2018, at 10:26, Sune Foldager via Mercurial-devel >> wrote: >> >>> Also please fix the check-code failure. >> >> Hm what’s wrong with -i? At any rate, I don’t have time to do this today. > > -i was added with different behaviors on BSD and GNU sed. It's incompatible > between those seds. Right, although it seems plain -i would work identically on both BSD and GNU, at least from reading the man pages on macOS and Linux. They both edit in-place, but the GNU version can take an optional argument after -i, for backup. Anyway, I changed the patch. /Sun ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] patch: don't separate \r and \n when colorizing diff output
> On 11 Jul 2018, at 16.22, Yuya Nishihara wrote: > >> On Wed, 11 Jul 2018 16:10:32 +0200, Sune Foldager wrote: >> It’s not in the repo as far as I can see, but ok then... > > Maybe you aren't based on the hg-committed repo? > > https://www.mercurial-scm.org/repo/hg-committed/ Never heard of it, so no :p. > >>>> +trailing whitespace >>>> + >>>> + $ sed -i 's/^dd$/dd \r/' a >>> >>> + tests/test-diff-color.t:56: >>> + > $ sed -i 's/^dd$/dd \r/' a >>> + don't use 'sed -i', use a temporary file >>> + tests/test-diff-color.t:71: >>> + > $ sed -i 's/\s*\r$//' a >>> + don't use 'sed -i', use a temporary file >>> + [1] > > Also please fix the check-code failure. Hm what’s wrong with -i? At any rate, I don’t have time to do this today. /Sune ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] tests: add diff color trailing whitespace test
# HG changeset patch # User Sune Foldager # Date 1531318293 -7200 # Wed Jul 11 16:11:33 2018 +0200 # Node ID 47e87ae492c852e773e19a896d08385eeedd9e35 # Parent 4d5fb4062f0bb159230062701461fa6cab9b539b tests: add diff color trailing whitespace test diff --git a/tests/test-diff-color.t b/tests/test-diff-color.t --- a/tests/test-diff-color.t +++ b/tests/test-diff-color.t @@ -51,6 +51,25 @@ default context a c +trailing whitespace + + $ sed -i 's/^dd$/dd \r/' a + $ hg diff --nodates + \x1b[0;1mdiff -r cf9f4ba66af2 a\x1b[0m (esc) + \x1b[0;31;1m--- a/a\x1b[0m (esc) + \x1b[0;32;1m+++ b/a\x1b[0m (esc) + \x1b[0;35m@@ -2,7 +2,7 @@\x1b[0m (esc) + c + a + a + \x1b[0;31m-b\x1b[0m (esc) + \x1b[0;32m+dd\x1b[0m\x1b[0;1;41m \x1b[0m\r (esc) + a + a + c + + $ sed -i 's/\s*\r$//' a + (check that 'ui.color=yes' match '--color=auto') $ hg diff --nodates --config ui.formatted=no ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] patch: don't separate \r and \n when colorizing diff output
It’s not in the repo as far as I can see, but ok then... > On 11 Jul 2018, at 15.59, Yuya Nishihara wrote: > >> On Wed, 11 Jul 2018 15:03:42 +0200, Sune Foldager via Mercurial-devel wrote: >> # HG changeset patch >> # User Sune Foldager >> # Date 1531314149 -7200 >> # Wed Jul 11 15:02:29 2018 +0200 >> # Node ID dc12175e9e5ba1bcb8ed7658a9218211727a9c49 >> # Parent 4d5fb4062f0bb159230062701461fa6cab9b539b >> patch: don't separate \r and \n when colorizing diff output > > V1 is already in. Can you send only the test? > >> +trailing whitespace >> + >> + $ sed -i 's/^dd$/dd \r/' a > > + tests/test-diff-color.t:56: > + > $ sed -i 's/^dd$/dd \r/' a > + don't use 'sed -i', use a temporary file > + tests/test-diff-color.t:71: > + > $ sed -i 's/\s*\r$//' a > + don't use 'sed -i', use a temporary file > + [1] ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH V2] patch: don't separate \r and \n when colorizing diff output
# HG changeset patch
# User Sune Foldager
# Date 1531314149 -7200
# Wed Jul 11 15:02:29 2018 +0200
# Node ID dc12175e9e5ba1bcb8ed7658a9218211727a9c49
# Parent 4d5fb4062f0bb159230062701461fa6cab9b539b
patch: don't separate \r and \n when colorizing diff output
When displaying diffs, \r at the end of a line is treated as trailing
whitespace. This causes an ANSI escape code to be inserted between \r and \n.
Some programs, such as less since version 530 (maybe earlier, but at least not
version 487) displays ^M when it encounters a lone \r. This causes a lot of
noise in diff output on Windows, where \r\n is used to terminate lines.
We avoid that by treating both \n and \r\n as end of line when considering
trailing whitespace.
A test case for trailing whitespace has also been added.
diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -2405,7 +2405,7 @@ def diffsinglehunk(hunklines):
"""yield tokens for a list of lines in a single hunk"""
for line in hunklines:
# chomp
-chompline = line.rstrip('\n')
+chompline = line.rstrip('\r\n')
# highlight tabs and trailing whitespace
stripline = chompline.rstrip()
if line.startswith('-'):
@@ -2473,6 +2473,9 @@ def diffsinglehunkinline(hunklines):
isendofline = token.endswith('\n')
if isendofline:
chomp = token[:-1] # chomp
+if chomp.endswith('\r'):
+chomp = chomp[:-1]
+endofline = token[len(chomp):]
token = chomp.rstrip() # detect spaces at the end
endspaces = chomp[len(token):]
# scan tabs
@@ -2488,7 +2491,7 @@ def diffsinglehunkinline(hunklines):
if isendofline:
if endspaces:
yield (endspaces, 'diff.trailingwhitespace')
-yield ('\n', '')
+yield (endofline, '')
nextisnewline = True
def difflabel(func, *args, **kw):
diff --git a/tests/test-diff-color.t b/tests/test-diff-color.t
--- a/tests/test-diff-color.t
+++ b/tests/test-diff-color.t
@@ -51,6 +51,25 @@ default context
a
c
+trailing whitespace
+
+ $ sed -i 's/^dd$/dd \r/' a
+ $ hg diff --nodates
+ \x1b[0;1mdiff -r cf9f4ba66af2 a\x1b[0m (esc)
+ \x1b[0;31;1m--- a/a\x1b[0m (esc)
+ \x1b[0;32;1m+++ b/a\x1b[0m (esc)
+ \x1b[0;35m@@ -2,7 +2,7 @@\x1b[0m (esc)
+ c
+ a
+ a
+ \x1b[0;31m-b\x1b[0m (esc)
+ \x1b[0;32m+dd\x1b[0m\x1b[0;1;41m \x1b[0m\r (esc)
+ a
+ a
+ c
+
+ $ sed -i 's/\s*\r$//' a
+
(check that 'ui.color=yes' match '--color=auto')
$ hg diff --nodates --config ui.formatted=no
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH STABLE] patch: don't separate \r and \n when colorizing diff output
On Jul 10, 2018, at 02:54 PM, Yuya Nishihara wrote: On Tue, 10 Jul 2018 13:20:34 +0200, Sune Foldager via Mercurial-devel wrote: # HG changeset patch # User Sune Foldager # Date 1531221514 -7200 # Tue Jul 10 13:18:34 2018 +0200 # Branch stable # Node ID 37b29ae6598324a8079f1d6d0cb973068a628ab7 # Parent 3a0f322af1926e52322d2cff90a71529f359f2d9 patch: don't separate \r and \n when colorizing diff output Queued for default since it's close to freeze and this isn't a critical issue. Yes, that's fine. I'll keep it in my patch queue for now anyway :) Thanks. Can you add some tests as a follow up? Yes, I suppose I can. I'll have a look. FWIW I miss this bug which helped me to notice unwanted CRLF line ending. Right. Typing -u in less (or giving it as command line option) shows CR as ^M again. It's just that less now shows isolated CR as ^M even without that option. It may be a bug in less or a feature, I don't know. At any rate, we shouldn't separate CR and LF. /Sune ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH STABLE] patch: don't separate \r and \n when colorizing diff output
# HG changeset patch
# User Sune Foldager
# Date 1531221514 -7200
# Tue Jul 10 13:18:34 2018 +0200
# Branch stable
# Node ID 37b29ae6598324a8079f1d6d0cb973068a628ab7
# Parent 3a0f322af1926e52322d2cff90a71529f359f2d9
patch: don't separate \r and \n when colorizing diff output
When displaying diffs, \r at the end of a line is treated as trailing
whitespace. This causes an ANSI escape code to be inserted between \r and \n.
Some programs, such as less since version 530 (maybe earlier, but at least not
version 487) displays ^M when it encounters a lone \r. This causes a lot of
noise in diff output on Windows, where \r\n is used to terminate lines.
We avoid that by treating both \n and \r\n as end of line when considering
trailing whitespace.
diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -2489,7 +2489,7 @@ def diffsinglehunk(hunklines):
"""yield tokens for a list of lines in a single hunk"""
for line in hunklines:
# chomp
-chompline = line.rstrip('\n')
+chompline = line.rstrip('\r\n')
# highlight tabs and trailing whitespace
stripline = chompline.rstrip()
if line[0] == '-':
@@ -2557,6 +2557,9 @@ def diffsinglehunkinline(hunklines):
isendofline = token.endswith('\n')
if isendofline:
chomp = token[:-1] # chomp
+if chomp.endswith('\r'):
+chomp = chomp[:-1]
+endofline = token[len(chomp):]
token = chomp.rstrip() # detect spaces at the end
endspaces = chomp[len(token):]
# scan tabs
@@ -2572,7 +2575,7 @@ def diffsinglehunkinline(hunklines):
if isendofline:
if endspaces:
yield (endspaces, 'diff.trailingwhitespace')
-yield ('\n', '')
+yield (endofline, '')
nextisnewline = True
def difflabel(func, *args, **kw):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH STABLE] windows: fix incorrect detection of broken pipe when writing to pager
Any comments? I am not really satisfied with the name "lasterrorwaspipeerror", but I couldn't find anything better. I also don't know if we want that in flush, but it seems it would be correct. The original code is many years old, so I'm not sure anyone remembers. I tried to find a way to extract the windows errorcode from an IOError, but that doesn't seem possible. Once it's mapped, I suppose it's discarded. The current method assumes that it's still valid, which isn't 100% satisfying somehow. We could also go for the simpler solution and simply detect EINVAL, but that might misdetect in some cases. We can also put it on default if you aren't comfortable with it on stable, but it _is_ a regression of sorts, at least from when the pager wasn't in core. The last version I tested (where I don't see it) is 4.0.1. -- Sune ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH STABLE] windows: fix incorrect detection of broken pipe when writing to pager
# HG changeset patch # User Sune Foldager # Date 1530706753 -7200 # Wed Jul 04 14:19:13 2018 +0200 # Branch stable # Node ID 3a0f322af1926e52322d2cff90a71529f359f2d9 # Parent 173cfdde0c8611fd86d26c0467b42aabf03bda0f windows: fix incorrect detection of broken pipe when writing to pager Paging e.g. hg incoming on Windows and quitting the pager before the output is consumed will print 'abort: Invalid argument'. This is because the windows error 0xE8 (ERROR_NO_DATA) is mapped to EINVAL even though it is documented as 'The pipe is being closed'. Note that this fix assumes that Windows' last error code is still valid in the exception handler. It works correctly in all my tests. A simpler fix would be to just map EINVAL to EPIPE, like was done is flush previously, but that would be less precise. This error was not observed previously, when pager was an extension. diff --git a/mercurial/win32.py b/mercurial/win32.py --- a/mercurial/win32.py +++ b/mercurial/win32.py @@ -44,6 +44,7 @@ from . import ( _ERROR_INVALID_PARAMETER = 87 _ERROR_BROKEN_PIPE = 109 _ERROR_INSUFFICIENT_BUFFER = 122 +_ERROR_NO_DATA = 232 # WPARAM is defined as UINT_PTR (unsigned type) # LPARAM is defined as LONG_PTR (signed type) @@ -406,6 +407,12 @@ def peekpipe(pipe): return avail.value +def lasterrorwaspipeerror(err): +if err.errno != errno.EINVAL: +return False +err = _kernel32.GetLastError() +return err == _ERROR_BROKEN_PIPE or err == _ERROR_NO_DATA + def testpid(pid): '''return True if pid is still running or unable to determine, False otherwise''' diff --git a/mercurial/windows.py b/mercurial/windows.py --- a/mercurial/windows.py +++ b/mercurial/windows.py @@ -172,7 +172,7 @@ class winstdout(object): self.fp.write(s[start:end]) start = end except IOError as inst: -if inst.errno != 0: +if inst.errno != 0 and not win32.lasterrorwaspipeerror(inst): raise self.close() raise IOError(errno.EPIPE, 'Broken pipe') @@ -181,7 +181,7 @@ class winstdout(object): try: return self.fp.flush() except IOError as inst: -if inst.errno != errno.EINVAL: +if not win32.lasterrorwaspipeerror(inst): raise raise IOError(errno.EPIPE, 'Broken pipe') ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] procutil: use unbuffered stdout on Windows
# HG changeset patch # User Sune Foldager # Date 1529937374 -7200 # Mon Jun 25 16:36:14 2018 +0200 # Node ID 936d9ec80550309fbebd1c182a7d3fe6eb3e1f56 # Parent c07424ec633c2541136a1b35dbcea1b3243dc1c1 procutil: use unbuffered stdout on Windows Windows doesn't support line buffering, treating it as fully buffered. This causes output of slow commands to stutter. We use unbuffered instead. diff -r c07424ec633c -r 936d9ec80550 mercurial/utils/procutil.py --- a/mercurial/utils/procutil.py Tue Jun 12 12:41:09 2018 -0700 +++ b/mercurial/utils/procutil.py Mon Jun 25 16:36:14 2018 +0200 @@ -41,9 +41,13 @@ # 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 +# buffering (or unbuffered, on Windows) if isatty(stdout): -stdout = os.fdopen(stdout.fileno(), r'wb', 1) +if pycompat.iswindows: +# Windows doesn't support line buffering +stdout = os.fdopen(stdout.fileno(), r'wb', 0) +else: +stdout = os.fdopen(stdout.fileno(), r'wb', 1) if pycompat.iswindows: from .. import windows as platform ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Output buffering on Windows 10
On 14 Jun 2018, at 17.08, Simon Farnsworth wrote: > 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. That makes sense, I figured it was something like that but didn’t look into your further changesets. > 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. Yes, I wasn’t sure whether we would prefer unbuffered over fully buffered for the pager on Windows. If unbuffered is what we want (now that line buffered isn’t available), I can make a fix for that and post it tomorrow. Thanks. — Sune ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Output buffering on Windows 10
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
[PATCH] hgweb: propagate http headers from ErrorResponse for web interface commands
# HG changeset patch
# User Sune Foldager
# Date 1528976682 -7200
# Thu Jun 14 13:44:42 2018 +0200
# Node ID d81af9ddf782d3761e89f778d42756b91514d1cc
# Parent c07424ec633c2541136a1b35dbcea1b3243dc1c1
hgweb: propagate http headers from ErrorResponse for web interface commands
This makes it possible for e.g. authorization hooks to provide appropriate
headers to make the web browser ask for credentials.
It's done in the same way as the existing code in wireprotoserver.py.
diff -r c07424ec633c -r d81af9ddf782 mercurial/hgweb/hgweb_mod.py
--- a/mercurial/hgweb/hgweb_mod.py Tue Jun 12 12:41:09 2018 -0700
+++ b/mercurial/hgweb/hgweb_mod.py Thu Jun 14 13:44:42 2018 +0200
@@ -441,6 +441,8 @@
res.headers['Content-Type'] = ctype
return rctx.sendtemplate('error', error=pycompat.bytestr(e))
except ErrorResponse as e:
+for k, v in e.headers:
+res.headers[k] = v
res.status = statusmessage(e.code, pycompat.bytestr(e))
res.headers['Content-Type'] = ctype
return rctx.sendtemplate('error', error=pycompat.bytestr(e))
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] parsers: fix invariant bug in find_deepest
Right, sorry about that. So you ended up un-rebasing the rebase I did before submitting :p. Always good to keep in shape, I suppose. Thanks. -Sune > On 14 Jul 2017, at 23.15, Augie Fackler <r...@durin42.com> wrote: > > >> On Jul 14, 2017, at 17:14, Sune Foldager <sune.folda...@me.com> wrote: >> >> Rebase? Didn’t I put in on stable? I realize the files are different on >> default, which is where I initially developed it. > > Ah, you didn't have --flag stable and I missed the branch. Next release is > 4.3 anyway, so it'll be there in any case. > >> >> As for the commit message change, I assume you mean the issue reference. >> Yeah, I didn’t think that would be correct ;). I’m out of the loop. >> >> -Sune >> >>>> On 14 Jul 2017, at 18.51, Augie Fackler <r...@durin42.com> wrote: >>>> >>>> On Fri, Jul 14, 2017 at 01:49:09PM +0200, Sune Foldager wrote: >>>> # HG changeset patch >>>> # User Sune Foldager <c...@cyanite.org> >>>> # Date 1500032897 -7200 >>>> # Fri Jul 14 13:48:17 2017 +0200 >>>> # Branch stable >>>> # Node ID 21158392118c6ffd8c60dd627a338f044726b9cb >>>> # Parent c1994c986d77f071e338de35a2c69d7bb87e39a3 >>>> parsers: fix invariant bug in find_deepest >>> >>> queued, thanks >>> >>> (Though with a pretty big rebase and a commit message tweak for the >>> bugzilla bot) > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V2] parsers: fix invariant bug in find_deepest
Rebase? Didn’t I put in on stable? I realize the files are different on default, which is where I initially developed it. As for the commit message change, I assume you mean the issue reference. Yeah, I didn’t think that would be correct ;). I’m out of the loop. -Sune > On 14 Jul 2017, at 18.51, Augie Fackler <r...@durin42.com> wrote: > >> On Fri, Jul 14, 2017 at 01:49:09PM +0200, Sune Foldager wrote: >> # HG changeset patch >> # User Sune Foldager <c...@cyanite.org> >> # Date 1500032897 -7200 >> # Fri Jul 14 13:48:17 2017 +0200 >> # Branch stable >> # Node ID 21158392118c6ffd8c60dd627a338f044726b9cb >> # Parent c1994c986d77f071e338de35a2c69d7bb87e39a3 >> parsers: fix invariant bug in find_deepest > > queued, thanks > > (Though with a pretty big rebase and a commit message tweak for the > bugzilla bot) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH V2] parsers: fix invariant bug in find_deepest
# HG changeset patch
# User Sune Foldager <c...@cyanite.org>
# Date 1500032897 -7200
# Fri Jul 14 13:48:17 2017 +0200
# Branch stable
# Node ID 21158392118c6ffd8c60dd627a338f044726b9cb
# Parent c1994c986d77f071e338de35a2c69d7bb87e39a3
parsers: fix invariant bug in find_deepest
find_deepest is used to find the "best" ancestors given a list. In the main
loop it keeps an invariant called 'ninteresting' which is supposed to contain
the number of non-zero entries in the 'interesting' array. This invariant is
incorrectly maintained, however, which leads the the algorithm returning an
empty result for certain graphs. This has been fixed.
Also, the 'interesting' array is supposed to fit 2^ancestors values, but is
incorrectly allocated to twice that size. This has been fixed as well.
The tests in test-ancestor.py compare the Python and C versions of the code,
and report the error correctly, since the Python version works correct. Even
so, I have added an additional test against the expected result, in the event
that both algorithms have an identical error in the future.
This fixes issue5623.
diff -r c1994c986d77 -r 21158392118c mercurial/parsers.c
--- a/mercurial/parsers.c Wed Jul 05 11:24:22 2017 -0400
+++ b/mercurial/parsers.c Fri Jul 14 13:48:17 2017 +0200
@@ -2040,7 +2040,7 @@
goto bail;
}
- interesting = calloc(sizeof(*interesting), 2 << revcount);
+ interesting = calloc(sizeof(*interesting), 1 << revcount);
if (interesting == NULL) {
PyErr_NoMemory();
goto bail;
@@ -2057,6 +2057,8 @@
interesting[b] = 1;
}
+ /* invariant: ninteresting is the number of non-zero entries in
+* interesting. */
ninteresting = (int)revcount;
for (v = maxrev; v >= 0 && ninteresting > 1; v--) {
@@ -2099,8 +2101,10 @@
continue;
seen[p] = nsp;
interesting[sp] -= 1;
- if (interesting[sp] == 0 && interesting[nsp] >
0)
+ if (interesting[sp] == 0)
ninteresting -= 1;
+ if (interesting[nsp] == 0)
+ ninteresting += 1;
interesting[nsp] += 1;
}
}
diff -r c1994c986d77 -r 21158392118c tests/test-ancestor.py
--- a/tests/test-ancestor.pyWed Jul 05 11:24:22 2017 -0400
+++ b/tests/test-ancestor.pyFri Jul 14 13:48:17 2017 +0200
@@ -212,14 +212,16 @@
# The C gca algorithm requires a real repo. These are textual descriptions of
-# DAGs that have been known to be problematic.
+# DAGs that have been known to be problematic, and, optionally, known pairs
+# of revisions and their expected ancestor list.
dagtests = [
-'+2*2*2/*3/2',
-'+3*3/*2*2/*4*4/*4/2*4/2*2',
+('+2*2*2/*3/2', {}),
+('+3*3/*2*2/*4*4/*4/2*4/2*2', {}),
+('+2*2*/2*4*/4*/3*2/4', {(6, 7): [3, 5]}),
]
def test_gca():
u = uimod.ui.load()
-for i, dag in enumerate(dagtests):
+for i, (dag, tests) in enumerate(dagtests):
repo = hg.repository(u, 'gca%d' % i, create=1)
cl = repo.changelog
if not util.safehasattr(cl.index, 'ancestors'):
@@ -230,15 +232,21 @@
# Compare the results of the Python and C versions. This does not
# include choosing a winner when more than one gca exists -- we make
# sure both return exactly the same set of gcas.
+# Also compare against expected results, if available.
for a in cl:
for b in cl:
cgcas = sorted(cl.index.ancestors(a, b))
pygcas = sorted(ancestor.ancestors(cl.parentrevs, a, b))
-if cgcas != pygcas:
+expected = None
+if (a, b) in tests:
+expected = tests[(a, b)]
+if cgcas != pygcas or (expected and cgcas != expected):
print("test_gca: for dag %s, gcas for %d, %d:"
% (dag, a, b))
print(" C returned: %s" % cgcas)
print(" Python returned: %s" % pygcas)
+if expected:
+print(" expected:%s" % expected)
def main():
seed = None
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
