[PATCH 8 of 9 V2] revlog: make _addrevision only accept rawtext
# HG changeset patch # User Jun Wu # Date 1490924283 25200 # Thu Mar 30 18:38:03 2017 -0700 # Node ID 3a4dd24ccf078c47722582f00872a88d33042ac3 # Parent 4bc37a3afb8b56b3d124188cc16ea8d83a0ba92d # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 3a4dd24ccf07 revlog: make _addrevision only accept rawtext All 3 users of _addrevision use raw: - addrevision: passing rawtext to _addrevision - addgroup: passing rawtext and raw=True to _addrevision - clone: passing rawtext to _addrevision There is no real user using _addrevision(raw=False). On the other hand, _addrevision is low-level code dealing with raw revlog deltas and rawtexts. It should not transform rawtext to non-raw text. This patch removes the "raw" parameter from "_addrevision", and does some rename and doc change to make it clear that "_addrevision" expects rawtext. Archeology shows 2df983125d37 added "raw" flag to "_addrevision", follow-ups e12c0fa1f65b and c1b7b2285522 seem to make the flag unnecessary. test-revlog-raw.py no longer complains. diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -1578,17 +1578,17 @@ class revlog(object): return True -def _addrevision(self, node, text, transaction, link, p1, p2, flags, - cachedelta, ifh, dfh, alwayscache=False, raw=False): +def _addrevision(self, node, rawtext, transaction, link, p1, p2, flags, + cachedelta, ifh, dfh, alwayscache=False): """internal function to add revisions to the log see addrevision for argument descriptions. + +note: "addrevision" takes non-raw text, "_addrevision" takes raw text. + invariants: -- text is optional (can be None); if not set, cachedelta must be set. +- rawtext is optional (can be None); if not set, cachedelta must be set. if both are set, they must correspond to each other. -- raw is optional; if set to True, it indicates the revision data is to - be treated by _processflags() as raw. It is usually set by changegroup - generation and debug commands. """ -btext = [text] +btext = [rawtext] def buildtext(): if btext[0] is not None: @@ -1608,9 +1608,9 @@ class revlog(object): else: fh = dfh -basetext = self.revision(self.node(baserev), _df=fh, raw=raw) +basetext = self.revision(self.node(baserev), _df=fh, raw=True) btext[0] = mdiff.patch(basetext, delta) try: -res = self._processflags(btext[0], flags, 'read', raw=raw) +res = self._processflags(btext[0], flags, 'read', raw=True) btext[0], validatehash = res if validatehash: @@ -1664,9 +1664,9 @@ class revlog(object): # full versions are inserted when the needed deltas # become comparable to the uncompressed text -if text is None: +if rawtext is None: textlen = mdiff.patchedsize(self.rawsize(cachedelta[0]), cachedelta[1]) else: -textlen = len(text) +textlen = len(rawtext) # should we try to build a delta? @@ -1709,6 +1709,6 @@ class revlog(object): dist, l, data, base, chainbase, chainlen, compresseddeltalen = delta else: -text = buildtext() -data = self.compress(text) +rawtext = buildtext() +data = self.compress(rawtext) l = len(data[1]) + len(data[0]) base = chainbase = curr @@ -1722,9 +1722,9 @@ class revlog(object): self._writeentry(transaction, ifh, dfh, entry, data, link, offset) -if alwayscache and text is None: -text = buildtext() +if alwayscache and rawtext is None: +rawtext = buildtext() -if type(text) == str: # only accept immutable objects -self._cache = (node, curr, text) +if type(rawtext) == str: # only accept immutable objects +self._cache = (node, curr, rawtext) self._chainbasecache[curr] = chainbase return node @@ -1848,6 +1848,5 @@ class revlog(object): p1, p2, flags, (baserev, delta), ifh, dfh, - alwayscache=bool(addrevisioncb), - raw=True) + alwayscache=bool(addrevisioncb)) if addrevisioncb: diff --git a/tests/test-revlog-raw.py.out b/tests/test-revlog-raw.py.out --- a/tests/test-revlog-raw.py.out +++ b/tests/test-revlog-raw.py.out @@ -2,3 +2,3 @@ local test passed addgroupcopy test passed clone test passed -abort: crashed: integrity check
[PATCH 6 of 9 V2] revlog: use raw revisions in revdiff
# HG changeset patch # User Jun Wu # Date 1490923407 25200 # Thu Mar 30 18:23:27 2017 -0700 # Node ID 272495735d544f8ab6053752db4e0a648f07415d # Parent 290c65aad38108cc59c7e8fb7f3b37a7c7a7573b # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 272495735d54 revlog: use raw revisions in revdiff See the added comment. revdiff is meant to output the raw delta that will be written to revlog. It should use raw. test-revlog-raw.py now shows "addgroupcopy test passed", but there is more to fix. diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -1240,10 +1240,14 @@ class revlog(object): def revdiff(self, rev1, rev2): -"""return or calculate a delta between two revisions""" +"""return or calculate a delta between two revisions + +The delta calculated is in binary form and is intended to be written to +revlog data directly. So this function needs raw revision data. +""" if rev1 != nullrev and self.deltaparent(rev2) == rev1: return bytes(self._chunk(rev2)) -return mdiff.textdiff(self.revision(rev1), - self.revision(rev2)) +return mdiff.textdiff(self.revision(rev1, raw=True), + self.revision(rev2, raw=True)) def revision(self, nodeorrev, _df=None, raw=False): diff --git a/tests/test-revlog-raw.py.out b/tests/test-revlog-raw.py.out --- a/tests/test-revlog-raw.py.out +++ b/tests/test-revlog-raw.py.out @@ -1,2 +1,3 @@ local test passed +addgroupcopy test passed abort: crashed: invalid patch ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 7 of 9 V2] revlog: use raw revisions in clone
# HG changeset patch # User Jun Wu # Date 1490923463 25200 # Thu Mar 30 18:24:23 2017 -0700 # Node ID 4bc37a3afb8b56b3d124188cc16ea8d83a0ba92d # Parent 272495735d544f8ab6053752db4e0a648f07415d # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 4bc37a3afb8b revlog: use raw revisions in clone test-revlog-raw.py now shows "clone test passed", but there is more to fix. diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -2085,5 +2085,5 @@ class revlog(object): # the revlog chunk is a delta. cachedelta = None -text = None +rawtext = None if populatecachedelta: dp = self.deltaparent(rev) @@ -2092,5 +2092,5 @@ class revlog(object): if not cachedelta: -text = self.revision(rev) +rawtext = self.revision(rev, raw=True) ifh = destrevlog.opener(destrevlog.indexfile, 'a+', @@ -2100,5 +2100,5 @@ class revlog(object): dfh = destrevlog.opener(destrevlog.datafile, 'a+') try: -destrevlog._addrevision(node, text, tr, linkrev, p1, p2, +destrevlog._addrevision(node, rawtext, tr, linkrev, p1, p2, flags, cachedelta, ifh, dfh) finally: diff --git a/tests/test-revlog-raw.py.out b/tests/test-revlog-raw.py.out --- a/tests/test-revlog-raw.py.out +++ b/tests/test-revlog-raw.py.out @@ -1,3 +1,4 @@ local test passed addgroupcopy test passed -abort: crashed: invalid patch +clone test passed +abort: crashed: integrity check failed on _destrevlog.i:5 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 5 of 9 V2] revlog: use raw content when building delta
# HG changeset patch # User Jun Wu # Date 1490921883 25200 # Thu Mar 30 17:58:03 2017 -0700 # Node ID 290c65aad38108cc59c7e8fb7f3b37a7c7a7573b # Parent 173d73e93cc25abdcac23fcd858a4e94d29e7461 # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 290c65aad381 revlog: use raw content when building delta Using external content provided by flagprocessor when building revlog delta is wrong, because deltas are applied to raw contents in revlog. This patch fixes the above issue by adding "raw=True". test-revlog-raw.py now shows "local test passed", but there is more to fix. diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -1636,5 +1636,5 @@ class revlog(object): else: fh = dfh -ptext = self.revision(self.node(rev), _df=fh) +ptext = self.revision(self.node(rev), _df=fh, raw=True) delta = mdiff.textdiff(ptext, t) header, data = self.compress(delta) diff --git a/tests/test-revlog-raw.py.out b/tests/test-revlog-raw.py.out --- a/tests/test-revlog-raw.py.out +++ b/tests/test-revlog-raw.py.out @@ -1,1 +1,2 @@ -abort: crashed: integrity check failed on _testrevlog.i:11 +local test passed +abort: crashed: invalid patch ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 9 of 9 V2] revlog: add a fast path for revision(raw=False)
# HG changeset patch # User Jun Wu # Date 1490934075 25200 # Thu Mar 30 21:21:15 2017 -0700 # Node ID 70c6b2eecf4d580d38404f157ef99da237593a68 # Parent 3a4dd24ccf078c47722582f00872a88d33042ac3 # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 70c6b2eecf4d revlog: add a fast path for revision(raw=False) If cache hit and flags are empty, no flag processor runs and "text" equals to "rawtext". So we check flags, and return rawtext. This resolves performance issue introduced by a previous patch. diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -1275,4 +1275,11 @@ class revlog(object): if raw: return self._cache[2] +# duplicated, but good for perf +if rev is None: +rev = self.rev(node) +# no extra flags set, no flag processor runs, text = rawtext +if self.flags(rev) == REVIDX_DEFAULT_FLAGS: +return self._cache[2] + cachedrev = self._cache[1] ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 9 V2] revlog: add a stronger test for raw processing
# HG changeset patch
# User Jun Wu
# Date 1490932137 25200
# Thu Mar 30 20:48:57 2017 -0700
# Node ID 4a3f09c00a850667225b56d4aa0069e8e8233281
# Parent dea2a17cbfd00bf08ee87b3e44b1c71499189f89
# Available At https://bitbucket.org/quark-zju/hg-draft
# hg pull https://bitbucket.org/quark-zju/hg-draft -r 4a3f09c00a85
revlog: add a stronger test for raw processing
There are some issues about revlog raw processing (flag processor). The test
is relatively strong covering many cases. It will verify fixes.
diff --git a/tests/test-revlog-raw.py b/tests/test-revlog-raw.py
new file mode 100644
--- /dev/null
+++ b/tests/test-revlog-raw.py
@@ -0,0 +1,290 @@
+# test revlog interaction about raw data (flagprocessor)
+
+from __future__ import absolute_import, print_function
+
+import sys
+
+from mercurial import (
+encoding,
+node,
+revlog,
+transaction,
+vfs,
+)
+
+# TESTTMP is optional. This makes it convenient to run without run-tests.py
+tvfs = vfs.vfs(encoding.environ.get('TESTTMP', b'/tmp'))
+
+# Enable generaldelta otherwise revlog won't use delta as expected by the test
+tvfs.options = {'generaldelta': True, 'revlogv1': True, 'revlogv1': True}
+
+# The test wants to control whether to use delta explicitly, based on
+# "storedeltachains".
+revlog.revlog._isgooddelta = lambda self, d, textlen: self.storedeltachains
+
+def abort(msg):
+print('abort: %s' % msg)
+# Return 0 so run-tests.py could compare the output.
+sys.exit()
+
+# Register a revlog processor for flag EXTSTORED.
+#
+# It simply prepends a fixed header, and replaces '1' to 'i'. So it has
+# insertion and replacement, and may be interesting to test revlog's line-based
+# deltas.
+_extheader = b'E\n'
+
+def readprocessor(self, rawtext):
+# True: the returned text could be used to verify hash
+text = rawtext[len(_extheader):].replace(b'i', b'1')
+return text, True
+
+def writeprocessor(self, text):
+# False: the returned rawtext shouldn't be used to verify hash
+rawtext = _extheader + text.replace(b'1', b'i')
+return rawtext, False
+
+def rawprocessor(self, rawtext):
+# False: do not verify hash. Only the content returned by "readprocessor"
+# can be used to verify hash.
+return False
+
+revlog.addflagprocessor(revlog.REVIDX_EXTSTORED,
+(readprocessor, writeprocessor, rawprocessor))
+
+# Utilities about reading and appending revlog
+
+def newtransaction():
+# A transaction is required to write revlogs
+report = lambda msg: None
+return transaction.transaction(report, tvfs, {'plain': tvfs}, b'journal')
+
+def newrevlog(name=b'_testrevlog.i', recreate=False):
+if recreate:
+tvfs.tryunlink(name)
+rlog = revlog.revlog(tvfs, name)
+return rlog
+
+def appendrev(rlog, text, tr, isext=False, isdelta=True):
+'''Append a revision. If isext is True, set the EXTSTORED flag so flag
+processor will be used (and rawtext is different from text). If isdelta is
+True, force the revision to be a delta, otherwise it's full text.
+'''
+nextrev = len(rlog)
+p1 = rlog.node(nextrev - 1)
+p2 = node.nullid
+if isext:
+flags = revlog.REVIDX_EXTSTORED
+else:
+flags = revlog.REVIDX_DEFAULT_FLAGS
+# Change storedeltachains temporarily, to override revlog's delta decision
+rlog.storedeltachains = isdelta
+try:
+rlog.addrevision(text, tr, nextrev, p1, p2, flags=flags)
+return nextrev
+except Exception as ex:
+abort('rev %d: failed to append: %s' % (nextrev, ex))
+finally:
+# Restore storedeltachains. It is always True, see revlog.__init__
+rlog.storedeltachains = True
+
+def addgroupcopy(rlog, tr, destname=b'_destrevlog.i', optimaldelta=True):
+'''Copy revlog to destname using revlog.addgroup. Return the copied revlog.
+
+This emulates push or pull. They use changegroup. Changegroup requires
+repo to work. We don't have a repo, so a dummy changegroup is used.
+
+If optimaldelta is True, use optimized delta parent, so the destination
+revlog could probably reuse it. Otherwise it builds sub-optimal delta, and
+the destination revlog needs more work to use it.
+
+This exercises some revlog.addgroup (and revlog._addrevision(text=None))
+code path, which is not covered by "appendrev" alone.
+'''
+class dummychangegroup(object):
+@staticmethod
+def deltachunk(pnode):
+pnode = pnode or node.nullid
+parentrev = rlog.rev(pnode)
+r = parentrev + 1
+if r >= len(rlog):
+return {}
+if optimaldelta:
+deltaparent = parentrev
+else:
+# suboptimal deltaparent
+deltaparent = min(0, parentrev)
+return {'node': rlog.node(r), 'p1': pnode, 'p2': node.nullid,
+'cs': rlog.node(rlog.linkrev(r)), 'flags': rlog.flags(r),
+
[PATCH 3 of 9 V2] revlog: rename some "text"s to "rawtext"
# HG changeset patch
# User Jun Wu
# Date 1490910969 25200
# Thu Mar 30 14:56:09 2017 -0700
# Node ID 3fb26d049c52534835fd397ceb70b0f83068695a
# Parent 31aa38709dbd983b87302a0f24f4cf78a138ae83
# Available At https://bitbucket.org/quark-zju/hg-draft
# hg pull https://bitbucket.org/quark-zju/hg-draft -r 3fb26d049c52
revlog: rename some "text"s to "rawtext"
This makes code easier to understand. "_addrevision" is left untouched - it
will be changed in a later patch.
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -1272,5 +1272,5 @@ class revlog(object):
# look up what we need to read
-text = None
+rawtext = None
if rev is None:
rev = self.rev(node)
@@ -1278,5 +1278,5 @@ class revlog(object):
chain, stopped = self._deltachain(rev, stoprev=cachedrev)
if stopped:
-text = self._cache[2]
+rawtext = self._cache[2]
# drop cache to save memory
@@ -1284,12 +1284,12 @@ class revlog(object):
bins = self._chunks(chain, df=_df)
-if text is None:
-text = bytes(bins[0])
+if rawtext is None:
+rawtext = bytes(bins[0])
bins = bins[1:]
-text = mdiff.patches(text, bins)
+rawtext = mdiff.patches(rawtext, bins)
-text, validatehash = self._processflags(text, self.flags(rev), 'read',
-raw=raw)
+text, validatehash = self._processflags(rawtext, self.flags(rev),
+'read', raw=raw)
if validatehash:
self.checkhash(text, node, rev=rev)
@@ -1452,23 +1452,22 @@ class revlog(object):
node = node or self.hash(text, p1, p2)
-newtext, validatehash = self._processflags(text, flags, 'write')
+rawtext, validatehash = self._processflags(text, flags, 'write')
# If the flag processor modifies the revision data, ignore any provided
# cachedelta.
-if newtext != text:
+if rawtext != text:
cachedelta = None
-text = newtext
-if len(text) > _maxentrysize:
+if len(rawtext) > _maxentrysize:
raise RevlogError(
_("%s: size of %d bytes exceeds maximum revlog storage of
2GiB")
-% (self.indexfile, len(text)))
+% (self.indexfile, len(rawtext)))
-node = node or self.hash(text, p1, p2)
+node = node or self.hash(rawtext, p1, p2)
if node in self.nodemap:
return node
if validatehash:
-self.checkhash(text, node, p1=p1, p2=p2)
+self.checkhash(rawtext, node, p1=p1, p2=p2)
dfh = None
@@ -1477,5 +1476,5 @@ class revlog(object):
ifh = self.opener(self.indexfile, "a+", checkambig=self._checkambig)
try:
-return self._addrevision(node, text, transaction, link, p1, p2,
+return self._addrevision(node, rawtext, transaction, link, p1, p2,
flags, cachedelta, ifh, dfh)
finally:
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 4 of 9 V2] revlog: fix _cache usage in revision()
# HG changeset patch # User Jun Wu # Date 1490913248 25200 # Thu Mar 30 15:34:08 2017 -0700 # Node ID 173d73e93cc25abdcac23fcd858a4e94d29e7461 # Parent 3fb26d049c52534835fd397ceb70b0f83068695a # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 173d73e93cc2 revlog: fix _cache usage in revision() As documented at revlog.__init__, revlog._cache stores raw text. The current read and write usage of "_cache" in revlog.revision lacks of raw=True check. This patch fixes that by adding check about raw, and storing rawtext explicitly in _cache. Note: it may slow down cache hit code path when raw=False and flags=0. That performance issue will be fixed in a later patch. test-revlog-raw now points us to a new problem. diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -1268,5 +1268,7 @@ class revlog(object): if self._cache: if self._cache[0] == node: -return self._cache[2] +# _cache only stores rawtext +if raw: +return self._cache[2] cachedrev = self._cache[1] @@ -1295,5 +1297,5 @@ class revlog(object): self.checkhash(text, node, rev=rev) -self._cache = (node, rev, text) +self._cache = (node, rev, rawtext) return text diff --git a/tests/test-revlog-raw.py.out b/tests/test-revlog-raw.py.out --- a/tests/test-revlog-raw.py.out +++ b/tests/test-revlog-raw.py.out @@ -1,1 +1,1 @@ -abort: rev 5: wrong text +abort: crashed: integrity check failed on _testrevlog.i:11 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 9 V2] revlog: clarify flagprocessor documentation
# HG changeset patch # User Jun Wu # Date 1490885988 25200 # Thu Mar 30 07:59:48 2017 -0700 # Node ID 31aa38709dbd983b87302a0f24f4cf78a138ae83 # Parent 4a3f09c00a850667225b56d4aa0069e8e8233281 # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 31aa38709dbd revlog: clarify flagprocessor documentation The words "text", "newtext", "bool" could be confusing. Use explicit "text" or "rawtext" and document more about the "bool". diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -89,9 +89,15 @@ def addflagprocessor(flag, processor): - flagprocessors must be 3-tuples of functions (read, write, raw) with the following signatures: - - (read) f(self, text) -> newtext, bool - - (write) f(self, text) -> newtext, bool - - (raw) f(self, text) -> bool + - (read) f(self, rawtext) -> text, bool + - (write) f(self, text) -> rawtext, bool + - (raw) f(self, rawtext) -> bool + "text" is presented to the user. "rawtext" is stored in revlog data, not + directly visible to the user. The boolean returned by these transforms is used to determine whether - 'newtext' can be used for hash integrity checking. + the returned text can be used for hash integrity checking. For example, + if "write" returns False, then "text" is used to generate hash. If + "write" returns True, that basically means "rawtext" returned by "write" + should be used to generate hash. Usually, "write" and "read" return + different booleans. And "raw" returns a same boolean as "write". Note: The 'raw' transform is used for changegroup generation and in some ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH V6] hgweb: expose a followlines UI in filerevision view
On Thu, Mar 30, 2017 at 1:29 AM, Denis Laxalde wrote:
> # HG changeset patch
> # User Denis Laxalde
> # Date 1490819176 -7200
> # Wed Mar 29 22:26:16 2017 +0200
> # Node ID 1c360ab5640bc326dd6ff70bdee78e4da006dcef
> # Parent e540846c67e0f838bcdb1db567a57df28d92491c
> # Available At http://hg.logilab.org/users/dlaxalde/hg
> # hg pull http://hg.logilab.org/users/dlaxalde/hg -r
> 1c360ab5640b
> # EXP-Topic linerange-log/hgweb-filelog
> hgweb: expose a followlines UI in filerevision view
>
> In filerevision view (/file//) we add some event listeners on
> mouse clicks of elements in the block.
> Those listeners will capture a range of lines selected between two mouse
> clicks and a box inviting to follow the history of selected lines will then
> show up. Selected lines (i.e. the block of lines) get a CSS class which
> make
> them highlighted. Selection can be cancelled (and restarted) by either
> clicking on the cancel ("x") button in the invite box or clicking on any
> other
> source line. Also clicking twice on the same line will abort the selection
> and
> reset event listeners to restart the process.
>
> As a first step, this action is only advertised by the "cursor: cell" CSS
> rule
> on source lines elements as any other mechanisms would make the code
> significantly more complicated. This might be improved later.
>
> All JavaScript code lives in a new "linerangelog.js" file, sourced in
> filerevision template (only in "paper" style for now).
>
This patch is great! As far as functionality goes, I think it is almost
good enough to queue. There are still some usability improvements (such as
an aid to let users know that clicking a line will allow them to follow
that line). But those features can be implemented as follow-ups IMO.
>
> diff --git a/contrib/wix/templates.wxs b/contrib/wix/templates.wxs
> --- a/contrib/wix/templates.wxs
> +++ b/contrib/wix/templates.wxs
> @@ -225,6 +225,7 @@
>
>
>
> +
>
>
>
> diff --git a/mercurial/templates/paper/filerevision.tmpl
> b/mercurial/templates/paper/filerevision.tmpl
> --- a/mercurial/templates/paper/filerevision.tmpl
> +++ b/mercurial/templates/paper/filerevision.tmpl
> @@ -71,8 +71,11 @@
>
> line wrap: class="linewraplink" href="javascript:toggleLinewrap()">on
> line source
> -{text%fileline}
> + data-logurl="{url|urlescape}log/{symrev}/{file|urlescape}"
> >{text%fileline}
>
> +
> +
> +
>
>
>
> diff --git a/mercurial/templates/static/linerangelog.js
> b/mercurial/templates/static/linerangelog.js
> new file mode 100644
> --- /dev/null
> +++ b/mercurial/templates/static/linerangelog.js
> @@ -0,0 +1,137 @@
> +// Copyright 2017 Logilab SA
> +//
> +// This software may be used and distributed according to the terms
> +// of the GNU General Public License, incorporated herein by reference.
>
This license header is slightly different from what is used elsewhere. In
other files, we use "version 2 or any later version." I see this file's
version is used by .c files and a few other random places. But since it is
in the minority, let's stick to the "version 2 or any later version"
language, as seen in e.g. mercurial/__init__.py.
+
> +//** Install event listeners for line block selection and followlines
> action */
> +function installLineSelect() {
> +var sourcelines = document.getElementsByClassName('sourcelines')[0];
> +if (typeof sourcelines === 'undefined') {
> +return;
> +}
> +// URL to complement with "linerange" query parameter
> +var targetUri = sourcelines.dataset.logurl;
> +if (typeof targetUri === 'undefined') {
> +return;
> +}
> +
> +var lineSelectedCSSClass = 'followlines-selected';
> +
> +//** add CSS class on element in `from`-`to` line range */
> +function addSelectedCSSClass(from, to) {
> +var spans = sourcelines.getElementsByTagName('span');
> +for (var i = from; i <= to; i++) {
> +spans.item(i).classList.add(lineSelectedCSSClass);
> +}
> +}
> +
> +//** remove CSS class from previously selected lines */
> +function removeSelectedCSSClass() {
> +var nodes = sourcelines.getElementsByClassName(
> +lineSelectedCSSClass);
> +while (nodes.length) {
> +nodes[0].classList.remove(lineSelectedCSSClass);
> +}
> +}
> +
> +// add event listener to the whole block to
> have
> +// it available in all children
> +sourcelines.addEventListener('click', lineSelectStart);
>
I know this works, but could you please move this until after the
definition of lineSelectStart() because it feels weird to see the reference
to a symbol before it is declared.
> +
> +//** event handler for "click" on the first line of a block */
> +function lineSelectStart(e) {
> +var startElement = e.target;
> +if (startElement.tagName !== 'SPAN') {
> +// not a (ma
[PATCH] sslutil: clarify internal documentation
# HG changeset patch # User Matt Harbison # Date 1490795674 14400 # Wed Mar 29 09:54:34 2017 -0400 # Node ID 9505a8771bb00e56230e4c4b265e8369e659a54f # Parent 2632df096fc0ac7582382b1f94ea4b9ad0bce8f2 sslutil: clarify internal documentation I ran into this python issue with an incomplete certificate chain on Windows recently, and this is the clarification that came from that experimenting. The comment I left on the bug tracker [1] with a reference to the CPython code [2] indicates that the original problem I had is a different bug, but happened to be mentioned under issue20916 on the Python bug tracker. [1] https://bz.mercurial-scm.org/show_bug.cgi?id=5313#c7 [2] https://hg.python.org/cpython/file/v2.7.12/Modules/_ssl.c#l628 diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py --- a/mercurial/sslutil.py +++ b/mercurial/sslutil.py @@ -414,8 +414,10 @@ # a hint to the user. # Only modern ssl module exposes SSLContext.get_ca_certs() so we can # only show this warning if modern ssl is available. -# The exception handler is here because of -# https://bugs.python.org/issue20916. +# The exception handler is here to handle bugs around cert attributes: +# https://bugs.python.org/issue20916#msg213479. (See issues5313.) +# When the main 20916 bug occurs, 'sslcontext.get_ca_certs()' is a +# non-empty list, but the following conditional is otherwise True. try: if (caloaded and settings['verifymode'] == ssl.CERT_REQUIRED and modernssl and not sslcontext.get_ca_certs()): ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2 V3] serve: make the URL the same for `hg serve` and `hg serve -S`
On Tue, 28 Mar 2017 10:02:34 -0400, Yuya Nishihara wrote: On Sun, 26 Mar 2017 23:04:31 -0400, Matt Harbison wrote: # HG changeset patch # User Matt Harbison # Date 1488146777 18000 # Sun Feb 26 17:06:17 2017 -0500 # Node ID d584ca4bc33bd2ebeaf9a7bd86440b3cdcecc138 # Parent 0ff9bef3e0f67422cf29c200fa4a671d861d060b serve: make the URL the same for `hg serve` and `hg serve -S` It's perfectly workable to serve up the parent repo without the -S for push and pull, as long as there are no subrepo changes in play. Therefore, having a different URL for the main repo based on the presence of this option seems like it would get annoying. This looks good, but more detailed commit message will help since some changes in hgweb/hgwebdir are really subtle. Other than noting that this change allows repos to be hosted at "", is there something else to mention on this one? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2 V3] serve: add support for Mercurial subrepositories
On Tue, 28 Mar 2017 10:11:15 -0400, Yuya Nishihara wrote:
On Sun, 26 Mar 2017 23:04:30 -0400, Matt Harbison wrote:
# HG changeset patch
# User Matt Harbison
# Date 1488146743 18000
# Sun Feb 26 17:05:43 2017 -0500
# Node ID 0ff9bef3e0f67422cf29c200fa4a671d861d060b
# Parent c60091fa1426892552dd6c0dd4b9c49e3c3da045
serve: add support for Mercurial subrepositories
+def addwebdirpath(repo, serverpath, webconf):
+webconf[serverpath] = repo.root
+repo.ui.debug('adding %s = %s\n' % (serverpath, repo.root))
+
+for r in repo.revs('filelog("path:.hgsub")'):
+ctx = repo[r]
+for subpath in ctx.substate:
+ctx.sub(subpath).addwebdirpath(serverpath, webconf)
[...]
--- a/mercurial/server.py
+++ b/mercurial/server.py
@@ -15,6 +15,7 @@
from . import (
chgserver,
+cmdutil,
commandserver,
error,
hgweb,
@@ -130,11 +131,24 @@
baseui = ui
webconf = opts.get('web_conf') or opts.get('webdir_conf')
if webconf:
+if opts.get('subrepos'):
+raise error.Abort(_('--web-conf cannot be used with
--subrepos'))
+
# load server settings (e.g. web.port) to "copied" ui, which
allows
# hgwebdir to reload webconf cleanly
servui = ui.copy()
servui.readconfig(webconf, sections=['web'])
alluis.add(servui)
+elif opts.get('subrepos'):
+servui = ui.copy()
+alluis.add(servui)
No need to make a copy of ui since nothing loaded into servui.
+@annotatesubrepoerror
+def addwebdirpath(self, serverpath, webconf):
+# The URL request contains the subrepo source path, not the
local
+# subrepo path. The distinction matters for 'foo = ../foo'
type
+# entries. It isn't possible to serve up 'foo = http://..'
type
+# entries, because the server path is relative to this local
server.
+src = self._state[0]
+if util.url(src).islocal():
+path = util.normpath(serverpath + src)
+cmdutil.addwebdirpath(self._repo, path + '/', webconf)
What happens if src is '../../escape_from_web_root' ?
I don't think it's correct to lay out subrepositories by peer URL since
we're
exporting a _local_ clone. If you do "hg clone sub1 sub2", you'll see
sub1
in local-path layout. "hg serve -S" just allows us to see sub1 over http.
That was actually how I first coded it, but switched it because it wasn't
handling the test in test-subrepo-deep-nested-change.t properly. I forget
what exactly the problem was. I'll take another look this weekend.
Maybe it's okay to add all repositories found under repo.root to webconf.
For strictness, we could check if a subrepo path exists in .hgsub of any
revision.
I'd like to avoid searching through the directory tree, if possible.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 07 of 10 shelve-ext v4] shelve: add shelve type saving and loading
On 21/03/2017 21:55, Augie Fackler wrote:
> On Sat, Mar 11, 2017 at 01:00:26PM -0800, Kostia Balytskyi wrote:
>> # HG changeset patch
>> # User Kostia Balytskyi
>> # Date 1489250294 28800
>> # Sat Mar 11 08:38:14 2017 -0800
>> # Node ID 5179d6be9d6a33071c8a18cc48166a3f1d5ceec5
>> # Parent 2dbff12d9b22281c8d84328704095d72b3151388
>> shelve: add shelve type saving and loading
> It's not called out anywhere, so I'll ask: do things work okay if you
> have a mix of bundled shelves and obsolete-marker shelves? It kind of
> looks like the answer is yes, but I wanted to make sure.
They are for the most part. But I've started looking into whether I have
test for every case and found a bug. Will add a patch in the initial
series and a separate test patch in test series.
>
>> We need shelve type to be stored in .hg/shelvedstate in order
>> to be able to run abort or continue action properly. If the shelve
>> is obsbased, those actions should create markes, if it is traditional,
>> the actions should strip commits.
>>
>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>> --- a/hgext/shelve.py
>> +++ b/hgext/shelve.py
>> @@ -183,6 +183,8 @@ class shelvedstate(object):
>> _filename = 'shelvedstate'
>> _keep = 'keep'
>> _nokeep = 'nokeep'
>> +_obsbased = 'obsbased'
>> +_traditional = 'traditional'
>>
>> def __init__(self, ui, repo):
>> self.ui = ui
>> @@ -204,6 +206,7 @@ class shelvedstate(object):
>> nodestoprune = [nodemod.bin(h) for h in fp.readline().split()]
>> branchtorestore = fp.readline().strip()
>> keep = fp.readline().strip() == cls._keep
>> +obsshelve = fp.readline().strip() == cls._obsbased
>> except (ValueError, TypeError) as err:
>> raise error.CorruptedState(str(err))
>> finally:
>> @@ -218,6 +221,7 @@ class shelvedstate(object):
>> obj.nodestoprune = nodestoprune
>> obj.branchtorestore = branchtorestore
>> obj.keep = keep
>> +obj.obsshelve = obsshelve
>> except error.RepoLookupError as err:
>> raise error.CorruptedState(str(err))
>>
>> @@ -225,7 +229,7 @@ class shelvedstate(object):
>>
>> @classmethod
>> def save(cls, repo, name, originalwctx, pendingctx, nodestoprune,
>> - branchtorestore, keep=False):
>> + branchtorestore, keep=False, obsshelve=False):
>> fp = repo.vfs(cls._filename, 'wb')
>> fp.write('%i\n' % cls._version)
>> fp.write('%s\n' % name)
>> @@ -237,6 +241,7 @@ class shelvedstate(object):
>>' '.join([nodemod.hex(n) for n in nodestoprune]))
>> fp.write('%s\n' % branchtorestore)
>> fp.write('%s\n' % (cls._keep if keep else cls._nokeep))
>> +fp.write('%s\n' % (cls._obsbased if obsshelve else
>> cls._traditional))
>> fp.close()
>>
>> @classmethod
>> ___
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 2 V2] verify: document corner cases
# HG changeset patch # User Jun Wu # Date 1490823901 25200 # Wed Mar 29 14:45:01 2017 -0700 # Node ID 17b41390f4912a4c18538d778837bc2cf4a1be92 # Parent dea2a17cbfd00bf08ee87b3e44b1c71499189f89 # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r 17b41390f491 verify: document corner cases It seems a good idea to list all kinds of "surprises" and expected behavior to make the upcoming changes easier to understand. Note: the comment added does not reflect the actual behavior of the current code. diff --git a/mercurial/verify.py b/mercurial/verify.py --- a/mercurial/verify.py +++ b/mercurial/verify.py @@ -380,5 +380,51 @@ class verifier(object): del filenodes[f][n] -# verify contents +# Verify contents. 4 cases to care about: +# +# common: the most common case +# rename: with a rename +# meta: file content starts with b'\1\n', the metadata +# header defined in filelog.py, but without a rename +# ext: largefiles. content stored elsewhere +# +# More formally, their differences are shown below: +# +# | common | rename | meta | ext +# --- +# flags() | 0 | 0 | 0 | not 0 +# renamed() | False | True | False | ? +# rawtext[0:2]=='\1\n'| False | True | True | ? +# +# "rawtext" means the raw text stored in revlog data, which +# could be retrieved by "revision(rev, raw=True)". "text" +# mentioned below is "revision(rev, raw=False)". +# +# There are 3 different lengths stored physically: +# 1. L1: rawsize, stored in revlog index +# 2. L2: len(rawtext), stored in revlog data +# 3. L3: len(text), stored in revlog data if flags==0, or +# possibly somewhere else if flags!=0 +# +# L1 should be equal to L2. L3 could be different from them. +# "text" may or may not affect commit hash depending on flag +# processors (see revlog.addflagprocessor). +# +# | common | rename | meta | ext +# - +#rawsize() | L1 | L1 | L1| L1 +# size() | L1 | L2-LM | L1(*) | L1 (?) +# len(rawtext) | L2 | L2 | L2| L2 +#len(text) | L2 | L2 | L2| L3 +# len(read()) | L2 | L2-LM | L2-LM | L3 (?) +# +# LM: length of metadata, depending on rawtext +# (*): not ideal, see comment in filelog.size +# (?): could be "- len(meta)" if the resolved content has +# rename metadata +# +# Checks needed to be done: +# 1. length check: L1 == L2, in all cases. +# 2. hash check: depending on flag processor, we may need to +# use either "text" (external), or "rawtext" (in revlog). try: l = len(fl.read(n)) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 2 V2] verify: fix length check
# HG changeset patch
# User Jun Wu
# Date 1490824154 25200
# Wed Mar 29 14:49:14 2017 -0700
# Node ID 35902a0e3f38c76a66d0dfbf76ec72091832
# Parent 17b41390f4912a4c18538d778837bc2cf4a1be92
# Available At https://bitbucket.org/quark-zju/hg-draft
# hg pull https://bitbucket.org/quark-zju/hg-draft -r 35902a0e3f38
verify: fix length check
According to the document added above, we should check L1 == L2, and the
only way to get L1 in all cases is to call "rawsize()", and the only way to
get L2 is to call "revision(raw=True)". Therefore the fix.
Meanwhile there are still a lot of things about flagprocessor broken in
revlog.py. Tests will be added after revlog.py gets fixed.
diff --git a/mercurial/verify.py b/mercurial/verify.py
--- a/mercurial/verify.py
+++ b/mercurial/verify.py
@@ -431,5 +431,6 @@ class verifier(object):
rp = fl.renamed(n)
if l != fl.size(i):
-if len(fl.revision(n)) != fl.size(i):
+# the "L1 == L2" check
+if len(fl.revision(n, raw=True)) != fl.rawsize(i):
self.err(lr, _("unpacked size is %s, %s expected")
%
(l, fl.size(i)), f)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Unifying sparse and narrow "profiles"
On 3/30/17 2:10 PM, Martin von Zweigbergk wrote: On Thu, Mar 30, 2017 at 1:15 PM, Durham Goode wrote: If we make some basic assumptions about the UI (i.e. 'hg sparse' is a good command name, and it will have 'include', 'exclude', 'enable-profile', and 'disable-profile' flags; and .hg/sparse is a good spot to store the client config), I'd almost say let's ship the current sparse extension in hgext/ and satisfy mode 0. Then once the narrow-sparse unification has occurred (assuming it implements the same hg sparse UI for mode 0 operations), we can delete the old sparse code. I'm not ready to make those assumption (and I'm not sure you were saying we should). I think we need to discuss UI for the three modes Augie mentioned and make sure that they feel consistent. Yea, I wasn't suggesting we use that exact UI. I'm just pointing out that the UI surface area of sparse is small enough that if we can decide on a reasonable UI, we could deliver a mode 0 implementation with relatively little effort. That way we can have sparse in 4.3, and there's no pressure to prioritize the narrow refactor until we feel it's necessary. That opens room for potential future refactors in narrowhg as Facebook moves towards a lazy changelog strategy which could influence the public facing narrowness strategy. For sparse and/or narrow in core, I think we'll need to pass matchers around a bit more. We probably will also need to work on matcher composition, so we can create a matcher with the user's patterns ANDed together with the sparse/narrow matcher. Do you have that problem already in the sparse extension? Once we have both sparse and narrow, it seems likely we will need to compose those two matchers too. Yea, sparse hacks around matcher composition at the moment by creating bare bones union and negation matcher classes. https://bitbucket.org/facebook/hg-experimental/src/b5df1d5f3e303a75c550f8e8770afad9945edd86/hgext3rd/sparse.py?at=default&fileviewer=file-view-default#sparse.py-938 It's a bit scary, but actual works remarkably well (though it needs an update to handle visitdir for treemanifest). (Also, are there any docs I should read about your sparse stuff and profiles?) `hg sparse --help` is about all we have right now but it's pretty good. Just pay attention to the last sentence in each argument description, since that tells you when the sparse change is applied (i.e. applied when the command is run, or applied when the commit is made) One other thing, the sparse extension is smart about temporarily pulling in files necessary for merge conflicts (like during a rebase, we pull in files that are being rebased until the rebase is over). This is very useful logic and would need to be ported to narrow as well. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Unifying sparse and narrow "profiles"
On Thu, Mar 30, 2017 at 1:15 PM, Durham Goode wrote: > On 3/29/17 7:47 PM, Augie Fackler wrote: >> >> (+martinvonz, durham) >> >>> On Mar 29, 2017, at 1:05 PM, Gregory Szorc >>> wrote: >>> >>> Mozilla has added tens of thousands of files to the Firefox repo in >>> the past few months and we have plans to add tens of thousands more >>> shortly. Working directory update times (especially in automation, >>> which has to do fresh checkouts with a somewhat high frequency >>> since we rely on ephemeral compute instances) were borderline >>> tolerable before. With the addition of tens of thousands of new >>> files, working directory updates are starting to put noticeable >>> strain on systems. >>> >>> Mozilla can make due with sparse checkouts - we don't yet have so >>> much repo data that we need narrow clone, although narrow would be >>> useful. >>> >>> Facebook's sparse checkout extension has existed for years and my >>> understanding is it gets the job done. When I asked at the sprint >>> why sparse isn't part of the core distribution, someone mentioned >>> it is because sparse and narrow have different, competing concepts >>> for defining a sparse/narrow profile and these will need to be >>> reconciled before either can be accepted into core. >> >> >> I think we’ve mostly stopped caring about profiles for our users - >> Martin, can you confirm that? > > > While Google might not need sparse profiles, I think the feature is critical > for making sparse usable in a multi-user environment. I think the lack of it > is one of the reasons Git's sparse implementation is not more widely used > (along with its unpolished UI). > >> >>> Is there a timeline for unifying the profiles and adding sparse to >>> the core distribution? >> >> >> This is mostly on me, I think, to clean up narrow and make it so that >> it can satisfy the varying modes of operation: >> 0) narrow working directory only >> 1) narrow working directory and history, but no eliding of irrelevant >> changes >> 2) “full narrow”, including elision of irrelevant changes >> >> What we’re using at Google is mode 2, but that’s also the most >> server-expensive and the most likely to have bugs. It shouldn’t be >> /too/ much work to reconcile our implementation with sparse, add >> profiles support, and default to mode 0 or 1. I agree about this. I'd also like to point out that mode 1 and 2 with treemanifests places unusual requirements on matching, namely that the client and server will agree on which directories are needed (the match.visitdir() method), given a set of patterns. For example, if I include "glob:foo/*/bar", the client will not know what which subdirectories in foo/ have a bar (sub)subdirectory, so the server will need to send all of them (i.e. the server can not skip subdirectories of foo/ that don't have any 'bar's). It also means that the match.visitdir() has to match between client and server, which places BC constraints on adding optimizations to match.visitdir(). So for mode 1 and 2, we should probably make the server restrictive about what kinds of patterns it allows, at least when using treemanifests. >> >> Durham, do you have opinions on this? Is it a fair assumption on my >> part that you’d rather we maintain this horror than you? > > > If we make some basic assumptions about the UI (i.e. 'hg sparse' is a good > command name, and it will have 'include', 'exclude', 'enable-profile', and > 'disable-profile' flags; and .hg/sparse is a good spot to store the client > config), I'd almost say let's ship the current sparse extension in hgext/ > and satisfy mode 0. Then once the narrow-sparse unification has occurred > (assuming it implements the same hg sparse UI for mode 0 operations), we can > delete the old sparse code. I'm not ready to make those assumption (and I'm not sure you were saying we should). I think we need to discuss UI for the three modes Augie mentioned and make sure that they feel consistent. > > Heck, it may even be beneficial to have the 'hg sparse' command control the > working copy oriented sparseness, and a separate command to control the > storage sparseness. So introducing them separately may make even more sense. Possibly. > > That way we can have sparse in 4.3, and there's no pressure to prioritize > the narrow refactor until we feel it's necessary. That opens room for > potential future refactors in narrowhg as Facebook moves towards a lazy > changelog strategy which could influence the public facing narrowness > strategy. For sparse and/or narrow in core, I think we'll need to pass matchers around a bit more. We probably will also need to work on matcher composition, so we can create a matcher with the user's patterns ANDed together with the sparse/narrow matcher. Do you have that problem already in the sparse extension? Once we have both sparse and narrow, it seems likely we will need to compose those two matchers too. > >> >> (Also, are there any docs I should read about your sparse stuff and >> profil
Re: [PATCH 7 of 7 shelve-ext v5] shelve: add some forwards compatibility to shelve operations
On 30/03/2017 10:31, Ryan McElroy wrote:
> On 3/29/17 2:18 PM, Kostia Balytskyi wrote:
>> # HG changeset patch
>> # User Kostia Balytskyi
>> # Date 1490790691 25200
>> # Wed Mar 29 05:31:31 2017 -0700
>> # Node ID 0953ca0a71a0e1f05078b0f3c255459e1ba56a4e
>> # Parent 9ce0a366a6316d1cffc2a875e7dc6b4a3227c851
>> shelve: add some forwards compatibility to shelve operations
>>
>> This patch handles cases where someone enabled an obs-based
>> shelve in their repo, then disabled it while having made some
>> shelves. Approach of this patch is described below:
>> - assume this is what .hg/shelved looks like:
>>- sh1.patch (created 1s ago)
>>- sh1.oshelve (created 1s ago)
> Would it make sense to have a "compatibility mode" where we also
> create the bundle, so that unshelving is actually backwards-compatible
> (if this option is enabled)?
We chatted in person and agreed on the following.
Despite my initial idea of making 'experimental.obsshelve' a clear line
between Mercurial knowing about obs-shelves and not knowing about them,
there seem to be no harm in letting list and unshelve know how to deal
with obsshelves. This means that current patch can be dropped entirely
and I will need to change a bit of unshelve's behavior. Effectively, it
will work like this:
- I run 'hg shelve --config experimental.obsshelve=on --name myshelve',
my changes are obs-shelved
- I run 'hg shelve --list --config experimental.obsshelve=off',
'myshelve' is shown among other shelves
- I run 'hg unshelve --config experimental.obsshelve=off myshelve', and
the operation is successful
I will modify shelve to behave like this in the next series unless
someone objects bofore.
>
>>- sh2.patch (created 2s ago)
>>- sh2.hg (created 2s ago)
>> - when obs-based shelve is enabled, 'hg shelve --list' shows both
>>sh1 and s2 and is able to unshelve both. 'hg unshelbe' (without
>> --name)
>
> s/unshelbe/unshelve
>
>>will unshelve sh1.
>> - when obs-based shelve is disabled, 'hg shelve --list' only shows
>>sh2, prints a warning that it found an obs-based shelve and is
>>only able to unshelve a traditional shelve. 'hg unshelve'
>>(without --name) will unshelve sh2.
>>
>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>> --- a/hgext/shelve.py
>> +++ b/hgext/shelve.py
>> @@ -66,9 +66,15 @@ testedwith = 'ships-with-hg-core'
>> backupdir = 'shelve-backup'
>> shelvedir = 'shelved'
>> -shelvefileextensions = ['hg', 'patch', 'oshelve']
>> # universal extension is present in all types of shelves
>> patchextension = 'patch'
>> +# extension used for bundle-based traditional shelves
>> +traditionalextension = 'hg'
>> +# extension used for obs-based shelves
>> +obsshelveextension = 'oshelve'
>> +shelvefileextensions = [traditionalextension,
>> +patchextension,
>> +obsshelveextension]
>> # we never need the user, so we use a
>> # generic user for all shelve operations
>> @@ -530,7 +536,12 @@ def deletecmd(ui, repo, pats):
>> raise error.Abort(_("shelved change '%s' not found") %
>> name)
>> def listshelves(repo):
>> -"""return all shelves in repo as list of (time, filename)"""
>> +"""return a tuple of (allshelves, obsshelvespresent)
>> +where
>> +'allshelves' is a list of (time, filename) for shelves available
>> +in current repo configuration
>> +'obsshelvespresent' is a bool indicating whether repo has
>> +any obs-based shelves"""
>> try:
>> names = repo.vfs.readdir(shelvedir)
>> except OSError as err:
>> @@ -538,13 +549,22 @@ def listshelves(repo):
>> raise
>> return []
>> info = []
>> +obsshelvedisabled = not isobsshelve(repo, repo.ui)
>> +obsshelvespresent = False
>> for (name, _type) in names:
>> pfx, sfx = name.rsplit('.', 1)
>> +if sfx == obsshelveextension:
>> +obsshelvespresent = True
>> if not pfx or sfx != patchextension:
>> continue
>> +traditionalfpath = repo.vfs.join(shelvedir,
>> + pfx + '.' +
>> traditionalextension)
>> +if obsshelvedisabled and not repo.vfs.exists(traditionalfpath):
>> +# this is not a traditional shelve
>> +continue
>> st = shelvedfile(repo, name).stat()
>> info.append((st.st_mtime, shelvedfile(repo, pfx).filename()))
>> -return sorted(info, reverse=True)
>> +return sorted(info, reverse=True), obsshelvespresent
>> def listcmd(ui, repo, pats, opts):
>> """subcommand that displays the list of shelves"""
>> @@ -554,7 +574,13 @@ def listcmd(ui, repo, pats, opts):
>> width = ui.termwidth()
>> namelabel = 'shelve.newest'
>> ui.pager('shelve')
>> -for mtime, name in listshelves(repo):
>> +availableshelves, obsshelvespresent = listshelves(repo)
>> +if (obsshelvespresent and not
Re: [PATCH 1 of 4 V2] obsolete: track node versions
I think this is a very nice approach to move forward. There are some behavior changes. But I've discussed with Durham and I'm happy about the new behaviors. The "node version" approach achieves "unhide" in a fast and more conservative way. The root-based hidden seems to require some non-trivial changes so I'll send a new version of "node version" patches solely to solve the "visibility" issue. Without a more general purposed API targeting future possibilities like exchange etc. Excerpts from Durham Goode's message of 2017-03-30 11:28:32 -0700: > Let's step back a moment and think about what obsmarkers are used for. > They are used to hide commits, and to automatically perform rebases. The > concerns around obscycles is that allowing cycles (without a perfect > version scheme) could affect those two uses. For hiding, it could > result in hiding commits from the user that they didn't mean to be > hidden. For rebasing, it means we could rebase onto the wrong successor. > > I think the underlying issue is that we're trying to do too much magic > for the user, and we're unable to find a perfect design to make that > magic safe and understandable. I think finding the right answer here may > be impossible. > > Proposal > === > > I propose a series of changes to reduce the magic and remove the need > for obs versioning, while maintaining the power and increasing the > understandability of these workflows. It has two parts: > > > 1. Never hide a commit during hg pull. Only hide commits when the user > does an action (strip/rebase/amend/histedit/evolve) > --- > > One of the recurring issues with obsmarkers is that commits may > magically disappear when you pull from another repo. This has two > issues: A) we have no good way of explaining it to the user, and B) we > can't even be sure the user wants those commits to disappear. > > I propose we *never* hide a commit when doing hg pull. When the user > pulls, we download the obsmarkers like normal, but the commits don't > disappear. Instead, we show the "[amended|rebased] to HASH" text in a > log/smartlog output so the user can know the old commits are dead, then > they can strip them at their leisure. This has the added benefit of > making it user visible what obsmarker changes the pull brought in. > > This proposal has two optional side features: > > 1a. *Do* hide commits during pull if the pulled version is public. > 1b. Add an option to strip/prune to auto hide any visible commits that > have visible successors and don't have visible descendants. So "hg pull > && hg strip/prune --obsolete-commits" would be roughly equivalent to hg > pull today. > > This proposal requires a notable change to core: > > - Hidden must be separated from obsmarkers storage and mutable outside > obsmarkers. This is possible with Jun's proposed phaseroot-esque hidden > storage. > > > 2. Auto rebase uses "visible successors" instead of "latest successor" > --- > > Today auto rebase (i.e. evolve) uses the latest successor as the > destination of the rebases. I propose we change that to "visible > successor" instead, where visible successor is defined as "any successor > commit that is not hidden". This means, regardless of the current > obsmarkers setup, the destination of the auto rebase is whatever > successor is currently visible. Which is probably what the user wanted > anyway. > > If there are multiple visible successors, auto rebase fails and shows a > list of the potential visible successors. Each item in the list can have > the "[amended|rebased] to HASH" string next to it so users can > understand at a glance the ordering and make a decision. They can either > use -d on rebase to specify a solution to the conflict, or they can use > the normal visibility commands (hg strip/prune) to remove any > undesirable commits. > > The presence of cycles does not affect this at all, and there is no need > for marker versioning. Auto rebase still uses whatever successors are > visible, even if the successor is "older" than it, because of a cycle. > The user can use the same rebase -d or strip/prune resolutions to > resolve the conflict. > > Summary of what these two changes achieve > === > > From a single, non-exchanging user's perspective the above proposal > does not affect current UX around when things are hidden (like during > rebase/amend/etc), but does allows all the benefits of the obscycle > discussion (allowing unhiding) without the need for obsmarker versioning. > > From an exchange user's perspective, this makes exchange much more > deterministic for the user (nothing is magically removed from the repo, > and what new obs information from the pull is explained via smartlog), > while still enabling auto rebase workflows. It also makes obsmarker > conflict (divergence/etc) easier to understand and resolve by allowing > users to resolve obsmarker conflicts using tools they're already > familiar with (rebase -d, st
Re: Unifying sparse and narrow "profiles"
On 3/29/17 7:47 PM, Augie Fackler wrote: (+martinvonz, durham) On Mar 29, 2017, at 1:05 PM, Gregory Szorc wrote: Mozilla has added tens of thousands of files to the Firefox repo in the past few months and we have plans to add tens of thousands more shortly. Working directory update times (especially in automation, which has to do fresh checkouts with a somewhat high frequency since we rely on ephemeral compute instances) were borderline tolerable before. With the addition of tens of thousands of new files, working directory updates are starting to put noticeable strain on systems. Mozilla can make due with sparse checkouts - we don't yet have so much repo data that we need narrow clone, although narrow would be useful. Facebook's sparse checkout extension has existed for years and my understanding is it gets the job done. When I asked at the sprint why sparse isn't part of the core distribution, someone mentioned it is because sparse and narrow have different, competing concepts for defining a sparse/narrow profile and these will need to be reconciled before either can be accepted into core. I think we’ve mostly stopped caring about profiles for our users - Martin, can you confirm that? While Google might not need sparse profiles, I think the feature is critical for making sparse usable in a multi-user environment. I think the lack of it is one of the reasons Git's sparse implementation is not more widely used (along with its unpolished UI). Is there a timeline for unifying the profiles and adding sparse to the core distribution? This is mostly on me, I think, to clean up narrow and make it so that it can satisfy the varying modes of operation: 0) narrow working directory only 1) narrow working directory and history, but no eliding of irrelevant changes 2) “full narrow”, including elision of irrelevant changes What we’re using at Google is mode 2, but that’s also the most server-expensive and the most likely to have bugs. It shouldn’t be /too/ much work to reconcile our implementation with sparse, add profiles support, and default to mode 0 or 1. Durham, do you have opinions on this? Is it a fair assumption on my part that you’d rather we maintain this horror than you? If we make some basic assumptions about the UI (i.e. 'hg sparse' is a good command name, and it will have 'include', 'exclude', 'enable-profile', and 'disable-profile' flags; and .hg/sparse is a good spot to store the client config), I'd almost say let's ship the current sparse extension in hgext/ and satisfy mode 0. Then once the narrow-sparse unification has occurred (assuming it implements the same hg sparse UI for mode 0 operations), we can delete the old sparse code. Heck, it may even be beneficial to have the 'hg sparse' command control the working copy oriented sparseness, and a separate command to control the storage sparseness. So introducing them separately may make even more sense. That way we can have sparse in 4.3, and there's no pressure to prioritize the narrow refactor until we feel it's necessary. That opens room for potential future refactors in narrowhg as Facebook moves towards a lazy changelog strategy which could influence the public facing narrowness strategy. (Also, are there any docs I should read about your sparse stuff and profiles?) `hg sparse --help` is about all we have right now but it's pretty good. Just pay attention to the last sentence in each argument description, since that tells you when the sparse change is applied (i.e. applied when the command is run, or applied when the commit is made) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 4 V2] obsolete: track node versions
On 3/30/17 11:28 AM, Durham Goode wrote: 1. Never hide a commit during hg pull. Only hide commits when the user does an action (strip/rebase/amend/histedit/evolve) 2. Auto rebase uses "visible successors" instead of "latest successor" To elaborate on how I see this obs cycle series affecting this proposal, I think the end result of this proposal is that obs versioning won't matter anymore. Or at the very least versioning only becomes a rough heuristic to suggest to the user which visible successor is likely to be their desired auto rebase destination. But, this series would be a fast way to introduce the concept of unhiding into core, which would let us start developing user experiences that benefit from unhiding, until we have truly separate hidden storage. So if we think my proposal is a good idea and where we want to be in the long run, I think we take this obscycle series, and don't worry about date being an imperfect heuristic since it won't matter in the long run. And we don't worry about the edge cases where it's unclear if X should be visible or Y, because in the future visibility will only ever be changed by explicit user action, and never by deducing it from obsmarker. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 4 V2] obsolete: track node versions
Let's step back a moment and think about what obsmarkers are used for. They are used to hide commits, and to automatically perform rebases. The concerns around obscycles is that allowing cycles (without a perfect version scheme) could affect those two uses. For hiding, it could result in hiding commits from the user that they didn't mean to be hidden. For rebasing, it means we could rebase onto the wrong successor. I think the underlying issue is that we're trying to do too much magic for the user, and we're unable to find a perfect design to make that magic safe and understandable. I think finding the right answer here may be impossible. Proposal === I propose a series of changes to reduce the magic and remove the need for obs versioning, while maintaining the power and increasing the understandability of these workflows. It has two parts: 1. Never hide a commit during hg pull. Only hide commits when the user does an action (strip/rebase/amend/histedit/evolve) --- One of the recurring issues with obsmarkers is that commits may magically disappear when you pull from another repo. This has two issues: A) we have no good way of explaining it to the user, and B) we can't even be sure the user wants those commits to disappear. I propose we *never* hide a commit when doing hg pull. When the user pulls, we download the obsmarkers like normal, but the commits don't disappear. Instead, we show the "[amended|rebased] to HASH" text in a log/smartlog output so the user can know the old commits are dead, then they can strip them at their leisure. This has the added benefit of making it user visible what obsmarker changes the pull brought in. This proposal has two optional side features: 1a. *Do* hide commits during pull if the pulled version is public. 1b. Add an option to strip/prune to auto hide any visible commits that have visible successors and don't have visible descendants. So "hg pull && hg strip/prune --obsolete-commits" would be roughly equivalent to hg pull today. This proposal requires a notable change to core: - Hidden must be separated from obsmarkers storage and mutable outside obsmarkers. This is possible with Jun's proposed phaseroot-esque hidden storage. 2. Auto rebase uses "visible successors" instead of "latest successor" --- Today auto rebase (i.e. evolve) uses the latest successor as the destination of the rebases. I propose we change that to "visible successor" instead, where visible successor is defined as "any successor commit that is not hidden". This means, regardless of the current obsmarkers setup, the destination of the auto rebase is whatever successor is currently visible. Which is probably what the user wanted anyway. If there are multiple visible successors, auto rebase fails and shows a list of the potential visible successors. Each item in the list can have the "[amended|rebased] to HASH" string next to it so users can understand at a glance the ordering and make a decision. They can either use -d on rebase to specify a solution to the conflict, or they can use the normal visibility commands (hg strip/prune) to remove any undesirable commits. The presence of cycles does not affect this at all, and there is no need for marker versioning. Auto rebase still uses whatever successors are visible, even if the successor is "older" than it, because of a cycle. The user can use the same rebase -d or strip/prune resolutions to resolve the conflict. Summary of what these two changes achieve === From a single, non-exchanging user's perspective the above proposal does not affect current UX around when things are hidden (like during rebase/amend/etc), but does allows all the benefits of the obscycle discussion (allowing unhiding) without the need for obsmarker versioning. From an exchange user's perspective, this makes exchange much more deterministic for the user (nothing is magically removed from the repo, and what new obs information from the pull is explained via smartlog), while still enabling auto rebase workflows. It also makes obsmarker conflict (divergence/etc) easier to understand and resolve by allowing users to resolve obsmarker conflicts using tools they're already familiar with (rebase -d, strip/prune). ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 7 shelve-ext v5] shelve: add obs-based shelve functionality
As long as evolve does not have a way to unhide a commit. I think we do need
to use strip on repos that with obsstore enabled. So I'd say the patch
probably needs to be deferred until we can unhide a commit.
Excerpts from Kostia Balytskyi's message of 2017-03-30 10:11:55 +:
>
> On 30/03/2017 10:15, Ryan McElroy wrote:
> >
> >
> > On 3/29/17 2:18 PM, Kostia Balytskyi wrote:
> >> # HG changeset patch
> >> # User Kostia Balytskyi
> >> # Date 1490790691 25200
> >> # Wed Mar 29 05:31:31 2017 -0700
> >> # Node ID fc1144e5993a5c85060b913e2d92cd4b1b61772e
> >> # Parent 0aa864184c9d78c11d18980cf0faa10828445ff5
> >> shelve: add obs-based shelve functionality
> >>
> >> Obsolescense-based shelve works in a following way:
> >
> > s/ a/ the
> sigh :/ Will fix.
> >
> >> 1. In order to shelve some changes, it creates a commit, records its
> >> node into a .oshelve file and prunes created commit.
> >> 2. In order to finish a shelve operation, transaction is just
> >> closed and not aborted.
> >
> > s/transaction/the transaction
> >
> >>
> >> diff --git a/hgext/shelve.py b/hgext/shelve.py
> >> --- a/hgext/shelve.py
> >> +++ b/hgext/shelve.py
> >> @@ -380,10 +380,15 @@ def _nothingtoshelvemessaging(ui, repo,
> >> else:
> >> ui.status(_("nothing changed\n"))
> >> -def _shelvecreatedcommit(repo, node, name):
> >> -bases = list(mutableancestors(repo[node]))
> >> -shelvedfile(repo, name, 'hg').writebundle(bases, node)
> >> -cmdutil.export(repo, [node],
> >> +def _shelvecreatedcommit(ui, repo, node, name, tr):
> >> +if isobsshelve(repo, ui):
> >> +shelvedfile(repo, name, 'oshelve').writeobsshelveinfo({
> >> +'node': nodemod.hex(node)
> >> +})
> >> +else:
> >> +bases = list(mutableancestors(repo[node]))
> >> +shelvedfile(repo, name, 'hg').writebundle(bases, node)
> >> +cmdutil.export(repo.unfiltered(), [node],
> >> fp=shelvedfile(repo, name,
> >> patchextension).opener('wb'),
> >> opts=mdiff.diffopts(git=True))
> >
> > I'm a latecomer to paying attention to this series, so please push
> > back on me if this has already been discussed, but I think it would be
> > "cleaner" to not mix old shelve and logic, and instead have just have
> > the main function _shelvecreatedcommit() call out to
> > _obsshelvecreatedcommit() or _abortshelvecreatedcommit() (or whatever
> > we call the traditional version).
> >
> > Again, feel free to push back if this has already been discussed or
> > discarded as a direction, or if there are other reasons to not do it.
> I agree that this can be made better. Sean Farley (if I'm not mistaken)
> suggested that this should be a class. I do not mind refactrong it
> later, but I want to get it in first as a functionality. Also,
> currently, if I understand you correctly, you want to split an 11 line
> function into two. I don't think it's worth it at the moment.
> >
> >> @@ -394,8 +399,13 @@ def _includeunknownfiles(repo, pats, opt
> >> extra['shelve_unknown'] = '\0'.join(s.unknown)
> >> repo[None].add(s.unknown)
> >> -def _finishshelve(repo):
> >> -_aborttransaction(repo)
> >> +def _finishshelve(ui, repo, tr, node):
> >> +if isobsshelve(repo, ui):
> >> +obsolete.createmarkers(repo, [(repo.unfiltered()[node], ())])
> >> +tr.close()
> >> +tr.release()
> >> +else:
> >> +_aborttransaction(repo)
> >> def _docreatecmd(ui, repo, pats, opts):
> >> wctx = repo[None]
> >> @@ -417,9 +427,12 @@ def _docreatecmd(ui, repo, pats, opts):
> >> try:
> >> lock = repo.lock()
> >> -# use an uncommitted transaction to generate the bundle to
> >> avoid
> >> -# pull races. ensure we don't print the abort message to
> >> stderr.
> >> -tr = repo.transaction('commit', report=lambda x: None)
> >> +# depending on whether shelve is traditional or
> >> +# obsolescense-based, we either abort or commit this
> >> +# transaction in the end. If we abort it, we don't
> >> +# want to print anything to stderr
> >> +report = None if isobsshelve(repo, ui) else (lambda x: None)
> >> +tr = repo.transaction('commit', report=report)
> >> interactive = opts.get('interactive', False)
> >> includeunknown = (opts.get('unknown', False) and
> >> @@ -447,16 +460,19 @@ def _docreatecmd(ui, repo, pats, opts):
> >> _nothingtoshelvemessaging(ui, repo, pats, opts)
> >> return 1
> >> -_shelvecreatedcommit(repo, node, name)
> >> +_shelvecreatedcommit(ui, repo, node, name, tr)
> >> if ui.formatted():
> >> desc = util.ellipsis(desc, ui.termwidth())
> >> ui.status(_('shelved as %s\n') % name)
> >> -hg.update(repo, parent.node())
> >> +# current wc parent may be already obsolete becuase
> >> +# it might have been created previousl
Re: [PATCH RFC] repo: add an ability to hide nodes in an appropriate way
On Thu, Mar 30, 2017 at 10:08 AM, Ryan McElroy wrote: > On 3/30/17 3:15 PM, Pierre-Yves David wrote: >> >> On 03/27/2017 07:24 PM, Augie Fackler wrote: >>> >>> On Mon, Mar 27, 2017 at 10:27:33AM +0200, Pierre-Yves David wrote: On 03/27/2017 12:32 AM, Kostia Balytskyi wrote: > > # HG changeset patch > # User Kostia Balytskyi > # Date 1490567500 25200 > # Sun Mar 26 15:31:40 2017 -0700 > # Node ID 43cd56f38c1ca2ad1920ed6804e1a90a5e263c0d > # Parent c60091fa1426892552dd6c0dd4b9c49e3c3da045 > repo: add an ability to hide nodes in an appropriate way > > Potentially frequent usecase is to hide nodes in the most appropriate > for current repo configuration way. Examples of things which use this > are rebase (strip or obsolete old nodes), shelve (strip of obsolete > temporary nodes) and probably others. > Jun Wu had an idea of having one place in core where this functionality > is implemented so that others can utilize it. This patch > implements this idea. I do not think this is a step in the right direction, creating obsolescence marker is not just about hiding changeset. >>> >>> >>> But we're using them for that today, all over the place. >> >> >> Actually, we do not use it all over the place. Current markers usages in >> core are: >> >> amend: >> - record evolution history >> - prune the temporary amend commit >> (that I would rather see disappear all together) >> >> rebase: >> - record evolution history on successful completion >> >> histedit: >> - record evolution history on successful completion >> - prune temporary nodes on successsful completion >>(This ones used to be stripped even with evolution) >> >> (let us talk about shelve later) >> >> >> So they are mainly used to record evolution history on successful >> operation (their intended usage) between "real-changesets". This is the >> intended usage of obsolescence markers. >> >> In addition, they are also used in a couple of place to hide temporary >> changesets (in the "utility-space") after an operation succeeed. >> This a bit an unorthodox uses of the obsolescence markers, but is "okay" >> to use them in these case because we know: >> 1) this is utility-space so user never interact with them. >> 2) we only create them for completed successful operation so we'll never >> need them ever again. >> Strictly speaking we could just strip these temporary commits (and we have >> done so in the past) and this will not change anything for the user. Any >> other hiding mechanism aimed at "internal temporary" commit would do the job >> just fine. The internal commit never needs (and actually should ever) to >> leave the user repository. >> In practice, we could use in memory commit for most of these temporary >> commit (if not all) and never have to deal with hiding them. >> >>> We're also having pretty productive (I think!) discussions about >>> non-obsmarker >>> based mechanisms for hiding things that are implementation details of >>> a sort (amend changesets that get folded immediately, shelve changes, >>> etc). >> >> >> (Let me be at be long to try to be clear and comprehensive) >> >> I can see three categories of things we want to hide: >> >> :obsolete changeset (evolution semantic): >> >> A rewrite operation has created/recorded a new version of this >> changesets. that old version no longer needs to be shown to users (if >> possible). There is a strong semantic associated with such changesets and >> the property will be propagated to other repositories >> >> :temporary/internal changesets: >> >> Such changesets are created as a side effect of other operation (amend, >> histedit, etc). They have the following properties (once we are done using >> them) >> >> * They are irrelevant to the users, >> * We should never-ever base anything on them, >> * We should never-ever access them ever again, >> * They should never-ever leave the repository. >> * They never stop being internal-changesets >> >> :locally-hidden changeset: >> >> An hypothetically (not used) yet local-only hidden mechanism (similar to >> strip but without actual repo stripping). This could be used for local >> hiding/masking of changeset in the "real-changeset" space. Such data would >> not be exchanged. Commit reappears when "recreated" or re-pulled. >> >> --- >> >> To take practical examples: >> >> :amend: >> - amend source: "obsolete-changeset" >> - temporary changeset: "internal-changeset" >> >> :rebase: >> - rebase source, on success: "obsolete-changeset" >> - rebase result, on abort: "local-hiding" >> >> >> :histedit: >> - histedit source, on success: "obsolete-changeset" >> - histedit temporary node, on success: "internal-changeset" >> - histedit result, on abort: "local-hiding" >> - histedit temporary node, on abort: "local-hiding (internal-node)" >> >> extra notes: >> * If local hiding (strip) would take
Re: [PATCH] test-check-code: prevent files being added to the root directory
Excerpts from Ryan McElroy's message of 2017-03-30 09:24:38 +0100: > On 3/29/17 8:14 PM, Jun Wu wrote: > > # HG changeset patch > > # User Jun Wu > > # Date 1490814860 25200 > > # Wed Mar 29 12:14:20 2017 -0700 > > # Node ID 4b4345a62fbd9fb0f4610d013c4ee5c9c06287e0 > > # Parent cda83a1bfb3ac3a23cfa158c407be93755c1018e > > test-check-code: prevent files being added to the root directory > > This looks good to me. Marked as pre-reviewed in patchwork. > > > Adding new files in the root directory is probably a mistake, and is usually > > discouraged [1]. The test catches it to avoid mistakes like [2]. > > > > Modify the test if files need to be added in the root. > > > > [1]: > > https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-July/086442.html > > [2]: > > https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-March/095836.html > > OMG I hate proofpoint. I want it to die in a fire. This is why I always > remove the footers and the "available at" URLs haha. I didn't see proofpoint? This is an advertisement of sup [1]. [1]: https://github.com/quark-zju/sup/ ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2] verify: document corner cases
On 3/30/17 5:01 PM, Jun Wu wrote: Excerpts from Ryan McElroy's message of 2017-03-30 09:55:50 +0100: On 3/29/17 10:51 PM, Jun Wu wrote: # HG changeset patch # User Jun Wu # Date 1490823901 25200 # Wed Mar 29 14:45:01 2017 -0700 # Node ID 1f7890370b5437466534cbd0a313d21671dade03 # Parent e9fda3b8614a8b701bd48041afa8b709e1227f27 verify: document corner cases It seems a good idea to list all kinds of "surprises" and expected behavior to make the upcoming changes easier to understand. Note: the comment added do not reflect the actual behavior of the current grammar-nit: s/do/does code. diff --git a/mercurial/verify.py b/mercurial/verify.py --- a/mercurial/verify.py +++ b/mercurial/verify.py @@ -381,4 +381,44 @@ class verifier(object): # verify contents +# +# Define 4 cases to help understand corner cases: +# +# | common | rename | meta | ext Can you define these four words? The first table is supposed to define them precisely. I guess it didn't for me, which is why I asked and thought "english descriptions" would be useful. common = common case? rename = file was just copied from somewhere else? meta = ??? "rename" is called "metadata" in filelog.py, so it means metadata without rename. When would this happen? It's still not clear to me. ext = extension that adds a flags processor? +# --- +# flags() | 0 | 0 | 0 | not 0 +# renamed() | False | True | False | ? +# rawtext[0:2]=='\1\n'| False | True | True | ? +# +# "rawtext" means the raw text stored in revlog data, which +# could be retrieved by "revision(rev, raw=True)". "text" +# mentioned below is "revision(rev, raw=False)". +# +# There are 3 different lengths stored physically: +# 1. L1: rawsize, stored in revlog index +# 2. L2: len(rawtext), stored in revlog data +# 3. L3: len(text), stored in revlog data if flags=0, or +# possibly somewhere else if flags!=0 +# +# L1 should be equal to L2. L3 is unrelated to L1 or L2. But How about "L3 can be different than L1 and L2" -- it would be the same if there are no flags, right? Yes. I notice the minor difference between the language. Thanks for the suggestion! +# text may or may not affect commit hash depending on flag +# processors (see revlog.addflagprocessor). +# +# | common | rename | meta | ext +# - +#rawsize() | L1 | L1 | L1| L1 +# size() | L1 | L2-LM | L1(*) | L1 (?) +# len(rawtext) | L2 | L2 | L2| L2 +#len(text) | L2 | L2 | L2| L3 +# len(read()) | L2 | L2-LM | L2-LM | L3 +# +# LM: length of metadata, depending on rawtext +# (*): not ideal, see comment in filelog.size +# (?): could be "- len(meta)" if the resolved content has +# rename metadata +# +# Checks needed to be done: +# 1. length check: L1 == L2, in all cases. +# 2. hash check: depending on flag processor, we may need to +# use either "text" (external), or "rawtext" (in revlog). try: l = len(fl.read(n)) Overall this is pretty easy to follow and adds a lot of clarity. Just a couple of questions inline. ___ 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
Excerpts from Augie Fackler's message of 2017-03-30 12:43:31 -0400: > On Thu, Mar 30, 2017 at 12:40 PM, Jun Wu wrote: > > I'll drop it from patchwork and send a new version with better documentation > > and some planned fixes. The next version has a same core idea, but the > > interface is more formalized and is made future proof in mind. The interface > > changes are by myself and haven't been discussed here. > > > Actually, based on a quick discussion I had with Durham, can you check > with him before sending another volley? It'll help the reviewers if we > can keep the patch traffic on this topic at a lower rate until we > actually get some better understanding of where we want/need to go > architecturally. > > Thanks! I'll chat with him today. Thanks! By the way, for series about histedit - I agree that marmoute was right, and rolling back is the right choice for now. So everything about histedit should be considered as "settled for now". A better solution about histedit *depends on* the "node versions" approach. So I may restart the histedit change if we have "node versions". The most interesting discussion to me is so-called "obscycles" (aka this series), but it's more precisely "node versions" as the first patch title suggests. "cycles" are just a subset of problems it solves <- this paragraph is for marmoute who might think I don't even understand my own patches. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH RFC] repo: add an ability to hide nodes in an appropriate way
On 3/30/17 3:15 PM, Pierre-Yves David wrote: On 03/27/2017 07:24 PM, Augie Fackler wrote: On Mon, Mar 27, 2017 at 10:27:33AM +0200, Pierre-Yves David wrote: On 03/27/2017 12:32 AM, Kostia Balytskyi wrote: # HG changeset patch # User Kostia Balytskyi # Date 1490567500 25200 # Sun Mar 26 15:31:40 2017 -0700 # Node ID 43cd56f38c1ca2ad1920ed6804e1a90a5e263c0d # Parent c60091fa1426892552dd6c0dd4b9c49e3c3da045 repo: add an ability to hide nodes in an appropriate way Potentially frequent usecase is to hide nodes in the most appropriate for current repo configuration way. Examples of things which use this are rebase (strip or obsolete old nodes), shelve (strip of obsolete temporary nodes) and probably others. Jun Wu had an idea of having one place in core where this functionality is implemented so that others can utilize it. This patch implements this idea. I do not think this is a step in the right direction, creating obsolescence marker is not just about hiding changeset. But we're using them for that today, all over the place. Actually, we do not use it all over the place. Current markers usages in core are: amend: - record evolution history - prune the temporary amend commit (that I would rather see disappear all together) rebase: - record evolution history on successful completion histedit: - record evolution history on successful completion - prune temporary nodes on successsful completion (This ones used to be stripped even with evolution) (let us talk about shelve later) So they are mainly used to record evolution history on successful operation (their intended usage) between "real-changesets". This is the intended usage of obsolescence markers. In addition, they are also used in a couple of place to hide temporary changesets (in the "utility-space") after an operation succeeed. This a bit an unorthodox uses of the obsolescence markers, but is "okay" to use them in these case because we know: 1) this is utility-space so user never interact with them. 2) we only create them for completed successful operation so we'll never need them ever again. Strictly speaking we could just strip these temporary commits (and we have done so in the past) and this will not change anything for the user. Any other hiding mechanism aimed at "internal temporary" commit would do the job just fine. The internal commit never needs (and actually should ever) to leave the user repository. In practice, we could use in memory commit for most of these temporary commit (if not all) and never have to deal with hiding them. We're also having pretty productive (I think!) discussions about non-obsmarker based mechanisms for hiding things that are implementation details of a sort (amend changesets that get folded immediately, shelve changes, etc). (Let me be at be long to try to be clear and comprehensive) I can see three categories of things we want to hide: :obsolete changeset (evolution semantic): A rewrite operation has created/recorded a new version of this changesets. that old version no longer needs to be shown to users (if possible). There is a strong semantic associated with such changesets and the property will be propagated to other repositories :temporary/internal changesets: Such changesets are created as a side effect of other operation (amend, histedit, etc). They have the following properties (once we are done using them) * They are irrelevant to the users, * We should never-ever base anything on them, * We should never-ever access them ever again, * They should never-ever leave the repository. * They never stop being internal-changesets :locally-hidden changeset: An hypothetically (not used) yet local-only hidden mechanism (similar to strip but without actual repo stripping). This could be used for local hiding/masking of changeset in the "real-changeset" space. Such data would not be exchanged. Commit reappears when "recreated" or re-pulled. --- To take practical examples: :amend: - amend source: "obsolete-changeset" - temporary changeset: "internal-changeset" :rebase: - rebase source, on success: "obsolete-changeset" - rebase result, on abort: "local-hiding" :histedit: - histedit source, on success: "obsolete-changeset" - histedit temporary node, on success: "internal-changeset" - histedit result, on abort: "local-hiding" - histedit temporary node, on abort: "local-hiding (internal-node)" extra notes: * If local hiding (strip) would take care of obsolescence marker, rebase and histedit could create them "as they go" instead of to the end: "on success". * In rebase --abort and histedit --abort, strip is used on freshly created changesets, so its performance impact is limited * we use "local hiding" (strip) on temporary nodes on histedit abort because we needs to be able to reuse them if histedit is rerun. So we cannot throw them in oblivion just yet. --- regarding implem
Re: [PATCH 4 of 4 V2] obsolete: allow cycles
On Thu, Mar 30, 2017 at 12:40 PM, Jun Wu wrote: > I'll drop it from patchwork and send a new version with better documentation > and some planned fixes. The next version has a same core idea, but the > interface is more formalized and is made future proof in mind. The interface > changes are by myself and haven't been discussed here. Actually, based on a quick discussion I had with Durham, can you check with him before sending another volley? It'll help the reviewers if we can keep the patch traffic on this topic at a lower rate until we actually get some better understanding of where we want/need to go architecturally. Thanks! ___ 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
Per discussion on IRC with marmoute, this series lacks of documentation. I'll drop it from patchwork and send a new version with better documentation and some planned fixes. The next version has a same core idea, but the interface is more formalized and is made future proof in mind. The interface changes are by myself and haven't been discussed here. Excerpts from Augie Fackler's message of 2017-03-23 18:05:45 -0400: > On Mon, Mar 13, 2017 at 11:57:08AM -0700, Pierre-Yves David wrote: > > > > > > On 03/13/2017 09:23 AM, Durham Goode wrote: > > >On 3/13/17 2:48 AM, Jun Wu wrote: > > >># HG changeset patch > > >># User Jun Wu > > >># Date 1489395002 25200 > > >># Mon Mar 13 01:50:02 2017 -0700 > > >># Node ID 6ae6d1069ba1d4089afaeb0bb8ef2411983a1292 > > >># Parent 0280ee091bd0ae33aa0a67b0c8a55ccffd2e0718 > > >># Available At > > >>https://bitbucket.org/quark-zju/hg-draft > > >> > > >># hg pull > > >>https://bitbucket.org/quark-zju/hg-draft > > >>-r 6ae6d1069ba1 > > >>obsolete: allow cycles > > >> > > >>Now we can handle cycles nicely, allow them to be created. Some practical > > >>examples: > > >> > > >> - To revive X, just create a marker X -> X, with a newer date. > > >> - To prune X again, just create a marker X -> (), with a newer date. > > >> - The above two could be repeated. > > >> > > >> - To unamend A -> B, just create a marker B -> A, with a newer date. > > >> > > >>It's now possible for "touch" and "unamend" to reuse hashes (therefore > > >>more > > >>user-friendly). And it's no longer necessary to write "*_source" in > > >>commit > > >>metadata to workarounds obs cycles. The hacky inhibit extension also > > >>becomes > > >>unnecessary. > > >> > > >>Finally. I have been wanting all these for a long time. > > > > > >Seems pretty elegant, though I haven't fully understood it yet. Maybe > > >you guys talked about this in person, but what effect (if any) does this > > >have on exchange? > > > > We did not really discuss this at the sprint :-/ > > > > This probably has effect on two aspects of pull and push: > > > > * Computation of the relevant markers for a None > > > > * Computation of "head replacement" when pushing branches obsoleting another > > one. > > > > The proposal have some very interesting aspect, I'll try to do a deep review > > of its impact on the general concept soon™ (probably not this week) > > I've had a look over the code, and it's surprisingly simple (though it > could probably use more comments for future code archaeologists.) I'm > wary (as is indygreg) of using the date field for this sorting, just > because clocks are known sources of evil and consternation. > > (I acknowledge we might not have a significantly better choice though, > although I'd like to give it some more thought.) > > > > > Cheers, > > > > -- > > Pierre-Yves David > > ___ > > Mercurial-devel mailing list > > Mercurial-devel@mercurial-scm.org > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 4 V2] obsolete: track node versions
Per discussion on IRC. I'll drop this series from patchwork and send a new version with better documentation and some planned fixes. Excerpts from Jun Wu's message of 2017-03-27 01:49:03 -0700: > Thanks for the detailed reply. I have replies on multiple subtopics. But I > deleted topics that I think are less important, to make the most important > topic clear, and make the discussion more efficient. If you think I need to > respond to more topics, feel free to point me at them. > > Excerpts from Pierre-Yves David's message of 2017-03-27 09:14:53 +0200: > > [snip] > > > Simple practical example with date: > > --- > > > > [time-1] repo-B: changeset C-A is pulled from repo-A > > [time-2] repo-A: changeset C-A is rewritten as C-B (time-2) > > [time-3] repo-B: changeset C-C is marked as superceeded by C-A (time-3) > > [time-4] repo-B: Changeset C-B (with marker) is pulled from repo-A > > > > The markers pointing from C-B to C-A have a version "time-2" but "C-A" > > now have a version of "time-3". "C-A" become wrongly un-obsoleted (and > > I think we basically disagree here. You said that "C-A" being visible does > not match "the meaning of the user action", could you define "the meaning of > the user action" formally ? > > In the "node version" approach, the "meaning of a user action", where action > is "creating a marker", is defined as below: > > When the user replace changeset X with Y, the marker X -> Y gets created, > they want Y to be visible, regardless of what previous (local or remote) > attempts hiding it. > > So the un-obsolete behavior is expected. > > > C-B is in strange state). The fact C-A is un-obsoleted here is a bug and > > Strictly speaking, C-B still has an obsoleted precursor of C-A. If we draw a > 2-D graph, where Y-axis is revision, X-axis is time, and markers are edges, > the above case could be represented like: > >^ rev >| C-C >|\ >| C-A C-A >| \ >| C-B >| >+---> time > > Note that there are 2 "C-A"s, the right one is visible, the left one is not. > We show users the right one. Internally, the code could distinguish the > different of 2 "C-A"s, if it wants. > > > do not match the meaning of the user action. This is a regression from > > what evolution is currently able to achieve. > > [snip] ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2] verify: document corner cases
Excerpts from Ryan McElroy's message of 2017-03-30 09:55:50 +0100: > On 3/29/17 10:51 PM, Jun Wu wrote: > > # HG changeset patch > > # User Jun Wu > > # Date 1490823901 25200 > > # Wed Mar 29 14:45:01 2017 -0700 > > # Node ID 1f7890370b5437466534cbd0a313d21671dade03 > > # Parent e9fda3b8614a8b701bd48041afa8b709e1227f27 > > verify: document corner cases > > > > It seems a good idea to list all kinds of "surprises" and expected behavior > > to make the upcoming changes easier to understand. > > > > Note: the comment added do not reflect the actual behavior of the current > grammar-nit: s/do/does > > > code. > > > > diff --git a/mercurial/verify.py b/mercurial/verify.py > > --- a/mercurial/verify.py > > +++ b/mercurial/verify.py > > @@ -381,4 +381,44 @@ class verifier(object): > > > > # verify contents > > +# > > +# Define 4 cases to help understand corner cases: > > +# > > +# | common | rename | meta | ext > > Can you define these four words? The first table is supposed to define them precisely. > common = common case? > rename = file was just copied from somewhere else? > meta = ??? "rename" is called "metadata" in filelog.py, so it means metadata without rename. > ext = extension that adds a flags processor? > > +# --- > > +# flags() | 0 | 0 | 0 | not 0 > > +# renamed() | False | True | False | ? > > +# rawtext[0:2]=='\1\n'| False | True | True | ? > > +# > > +# "rawtext" means the raw text stored in revlog data, which > > +# could be retrieved by "revision(rev, raw=True)". "text" > > +# mentioned below is "revision(rev, raw=False)". > > +# > > +# There are 3 different lengths stored physically: > > +# 1. L1: rawsize, stored in revlog index > > +# 2. L2: len(rawtext), stored in revlog data > > +# 3. L3: len(text), stored in revlog data if flags=0, or > > +# possibly somewhere else if flags!=0 > > +# > > +# L1 should be equal to L2. L3 is unrelated to L1 or L2. > > But > > How about "L3 can be different than L1 and L2" -- it would be the same > if there are no flags, right? Yes. I notice the minor difference between the language. Thanks for the suggestion! > > +# text may or may not affect commit hash depending on flag > > +# processors (see revlog.addflagprocessor). > > +# > > +# | common | rename | meta | ext > > +# - > > +#rawsize() | L1 | L1 | L1| L1 > > +# size() | L1 | L2-LM | L1(*) | L1 (?) > > +# len(rawtext) | L2 | L2 | L2| L2 > > +#len(text) | L2 | L2 | L2| L3 > > +# len(read()) | L2 | L2-LM | L2-LM | L3 > > +# > > +# LM: length of metadata, depending on rawtext > > +# (*): not ideal, see comment in filelog.size > > +# (?): could be "- len(meta)" if the resolved content has > > +# rename metadata > > +# > > +# Checks needed to be done: > > +# 1. length check: L1 == L2, in all cases. > > +# 2. hash check: depending on flag processor, we may need > > to > > +# use either "text" (external), or "rawtext" (in > > revlog). > > try: > > l = len(fl.read(n)) > > Overall this is pretty easy to follow and adds a lot of clarity. Just a > couple of questions inline. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 7 V5] hgweb: add a "patch" query parameter to filelog command
On Fri, 31 Mar 2017 00:37:22 +0900, Yuya Nishihara wrote:
> On Thu, 30 Mar 2017 17:10:28 +0200, Denis Laxalde wrote:
> > Yuya Nishihara a écrit :
> > > On Sat, 25 Mar 2017 10:20:57 +0100, Denis Laxalde wrote:
> > >> # HG changeset patch
> > >> # User Denis Laxalde
> > >> # Date 1489398073 -3600
> > >> # Mon Mar 13 10:41:13 2017 +0100
> > >> # Node ID a7b9bc1af8b81c332cf34960f581359e6942ca17
> > >> # Parent ffd1a15d456342caba744f5049b97862fcb697c7
> > >> # Available At https://hg.logilab.org/users/dlaxalde/hg
> > >> # hg pull https://hg.logilab.org/users/dlaxalde/hg -r
> > >> a7b9bc1af8b8
> > >> # EXP-Topic linerange-log/hgweb-filelog
> > >> hgweb: add a "patch" query parameter to filelog command
> > >
> > >> @@ -981,12 +985,27 @@ def filelog(web, req, tmpl):
> > >> repo = web.repo
> > >> revs = fctx.filelog().revs(start, end - 1)
> > >> entries = []
> > >> +
> > >> +diffstyle = web.config('web', 'style', 'paper')
> > >> +if 'style' in req.form:
> > >> +diffstyle = req.form['style'][0]
> > >> +
> > >> +def diff(fctx):
> > >> +ctx = fctx.changectx()
> > >> +basectx = ctx.p1()
> > >> +path = fctx.path()
> > >> +return webutil.diffs(web, tmpl, ctx, basectx, [path], diffstyle)
> > >> +
> > >> for i in revs:
> > >> iterfctx = fctx.filectx(i)
> > >> +diffs = None
> > >> +if patch:
> > >> +diffs = diff(iterfctx)
> > >
> > > Got an exception at hidden revision '5cb3b7d1f50f' which had been rebased
> > > (in
> > > my local repo.) Perhaps linkrev wouldn't be adjusted because we walk
> > > revision
> > > numbers, not a history of filelog.
> > >
> >
> > Maybe hidden revisions shouldn't be listed at all? I mean, without the
> > "patch" query parameter, you don't get a crash on the /log//
> > view,
> > but a 404 when following a link pointing to a hidden revision in the
> > table, so these log entries are arguably not very useful.
>
> Hidden revisions aren't listed.
I was wrong. Hidden revisions are listed. Maybe we'll need the logic which
"hg log FILE" uses.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 7 V5] hgweb: add a "patch" query parameter to filelog command
On Thu, 30 Mar 2017 17:10:28 +0200, Denis Laxalde wrote:
> Yuya Nishihara a écrit :
> > On Sat, 25 Mar 2017 10:20:57 +0100, Denis Laxalde wrote:
> >> # HG changeset patch
> >> # User Denis Laxalde
> >> # Date 1489398073 -3600
> >> # Mon Mar 13 10:41:13 2017 +0100
> >> # Node ID a7b9bc1af8b81c332cf34960f581359e6942ca17
> >> # Parent ffd1a15d456342caba744f5049b97862fcb697c7
> >> # Available At https://hg.logilab.org/users/dlaxalde/hg
> >> # hg pull https://hg.logilab.org/users/dlaxalde/hg -r
> >> a7b9bc1af8b8
> >> # EXP-Topic linerange-log/hgweb-filelog
> >> hgweb: add a "patch" query parameter to filelog command
> >
> >> @@ -981,12 +985,27 @@ def filelog(web, req, tmpl):
> >> repo = web.repo
> >> revs = fctx.filelog().revs(start, end - 1)
> >> entries = []
> >> +
> >> +diffstyle = web.config('web', 'style', 'paper')
> >> +if 'style' in req.form:
> >> +diffstyle = req.form['style'][0]
> >> +
> >> +def diff(fctx):
> >> +ctx = fctx.changectx()
> >> +basectx = ctx.p1()
> >> +path = fctx.path()
> >> +return webutil.diffs(web, tmpl, ctx, basectx, [path], diffstyle)
> >> +
> >> for i in revs:
> >> iterfctx = fctx.filectx(i)
> >> +diffs = None
> >> +if patch:
> >> +diffs = diff(iterfctx)
> >
> > Got an exception at hidden revision '5cb3b7d1f50f' which had been rebased
> > (in
> > my local repo.) Perhaps linkrev wouldn't be adjusted because we walk
> > revision
> > numbers, not a history of filelog.
> >
>
> Maybe hidden revisions shouldn't be listed at all? I mean, without the
> "patch" query parameter, you don't get a crash on the /log//
> view,
> but a 404 when following a link pointing to a hidden revision in the
> table, so these log entries are arguably not very useful.
Hidden revisions aren't listed. I suspect fctx.changectx() would return
a hidden changectx because fctx wasn't traversed from a visible parent and
bypassed linkrev adjustment.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 7 V5] hgweb: add a "patch" query parameter to filelog command
Yuya Nishihara a écrit :
On Sat, 25 Mar 2017 10:20:57 +0100, Denis Laxalde wrote:
# HG changeset patch
# User Denis Laxalde
# Date 1489398073 -3600
# Mon Mar 13 10:41:13 2017 +0100
# Node ID a7b9bc1af8b81c332cf34960f581359e6942ca17
# Parent ffd1a15d456342caba744f5049b97862fcb697c7
# Available At https://hg.logilab.org/users/dlaxalde/hg
# hg pull https://hg.logilab.org/users/dlaxalde/hg -r a7b9bc1af8b8
# EXP-Topic linerange-log/hgweb-filelog
hgweb: add a "patch" query parameter to filelog command
@@ -981,12 +985,27 @@ def filelog(web, req, tmpl):
repo = web.repo
revs = fctx.filelog().revs(start, end - 1)
entries = []
+
+diffstyle = web.config('web', 'style', 'paper')
+if 'style' in req.form:
+diffstyle = req.form['style'][0]
+
+def diff(fctx):
+ctx = fctx.changectx()
+basectx = ctx.p1()
+path = fctx.path()
+return webutil.diffs(web, tmpl, ctx, basectx, [path], diffstyle)
+
for i in revs:
iterfctx = fctx.filectx(i)
+diffs = None
+if patch:
+diffs = diff(iterfctx)
Got an exception at hidden revision '5cb3b7d1f50f' which had been rebased (in
my local repo.) Perhaps linkrev wouldn't be adjusted because we walk revision
numbers, not a history of filelog.
Maybe hidden revisions shouldn't be listed at all? I mean, without the
"patch" query parameter, you don't get a crash on the /log//
view,
but a 404 when following a link pointing to a hidden revision in the
table, so these log entries are arguably not very useful.
+ --- /dev/nullThu Jan 01 00:00:00 1970 +
We'll need a prefix to make 'id' unique.
That's true. Will work on a follow-up.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 07 of 11] revlog: fix many file handle opens to explicitly use bytes mode
On Wed, 29 Mar 2017 19:24:11 -0700, Gregory Szorc wrote: > On Mon, Mar 27, 2017 at 6:54 AM, Yuya Nishihara wrote: > > On Sun, 26 Mar 2017 18:36:41 -0400, Augie Fackler wrote: > > > # HG changeset patch > > > # User Augie Fackler > > > # Date 1490567324 14400 > > > # Sun Mar 26 18:28:44 2017 -0400 > > > # Node ID e7fe3ab60132647a9c3b86b2960b385e61b9dcf0 > > > # Parent 48144fe2d912b7d9fc300955d0c881aceead6930 > > > revlog: fix many file handle opens to explicitly use bytes mode > > > > > > This broke on Python 3, but we've probably been getting lucky that > > > this hasn't caused EOL corruption on Windows or something for > > > years. :( > > > > > > diff --git a/mercurial/revlog.py b/mercurial/revlog.py > > > --- a/mercurial/revlog.py > > > +++ b/mercurial/revlog.py > > > @@ -1467,8 +1467,8 @@ class revlog(object): > > > > > > dfh = None > > > if not self._inline: > > > -dfh = self.opener(self.datafile, "a+") > > > -ifh = self.opener(self.indexfile, "a+", > > checkambig=self._checkambig) > > > +dfh = self.opener(self.datafile, "a+b") > > > +ifh = self.opener(self.indexfile, "a+b", > > checkambig=self._checkambig) > > > > 'b' shouldn't be needed since vfs adds 'b' unless text=True is explicitly > > specified. > > > > That "text" argument always seemed weird to me because all it does is > reinvent the wheel for the "mode" argument to open(). I'd prefer we > deprecate the argument and use explicit mode values so the I/O code more > closely aligns with what Python does. Principal of least surprise. Perhaps it exists because 'b' is too subtle and unix programmers would easily miss it. I'd rather get rid of the text mode at all for Py3 compatibility. Optionally we can make vfs raise exception if 'b' isn't explicitly specified. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 4 of 4 NEW-CONCEPT] track-tags: write all tag changes to a file
# HG changeset patch
# User Pierre-Yves David
# Date 1490688902 -7200
# Tue Mar 28 10:15:02 2017 +0200
# Node ID 46005886092380991a30c2a02486c2876ecac341
# Parent 2dcaa99d6160561166269f192a89b25f84fd4667
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
# hg pull
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 460058860923
track-tags: write all tag changes to a file
The tag changes information we compute are not written to disk. This gives
hooks full acces to that data.
The format picked for that file use a 2 characters prefix for the action:
-R: tag removed
+A: tag added
-M: tag moved (old value)
+M: tag moved (new value)
This format allows hooks to easily select the line that matters to them without
having to post process the file too much. Here is
a couple of example:
* to select all newly tagged changeset, match "^+",
* to detect tag move, match "^.M",
* to detect tag deletion, match "-R".
Once again we rely on the fact the tag tests run through all possible
situations to test this change.
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1015,6 +1015,25 @@ class localrepository(object):
# and do not use caches as much as it could. The current focus is on
# the behavior of the feature so we disable it by default. The flag
# will be removed when we are happy with the performance impact.
+#
+# Once this feature is no longer experimental move the following
+# documentation to the appropriate help section:
+#
+# The ``HG_TAG_MOVED`` variable will be set if the transaction touched
+# tags (new or changed or deleted tags). In addition the details of
+# these changes are made available in a file at:
+# ``REPOROOT/.hg/changes/tags.changes``.
+# Make sure you check for HG_TAG_MOVED before reading that file as it
+# might exist from a previous transaction even if no tag were touched
+# in this one. Change are recorded in a line base format::
+#
+# \n
+#
+# Actions are defined as follow:
+# "-R": tag is removed,
+# "+A": tag is added,
+# "-M": tag is moved (old value),
+# "+M": tag is moved (new value),
tracktags = lambda x: None
# experimental config: experimental.hook-track-tags
shouldtracktags = self.ui.configbool('experimental', 'hook-track-tags',
@@ -1031,6 +1050,12 @@ class localrepository(object):
changes = tagsmod.difftags(repo.ui, repo, oldfnodes, newfnodes)
if changes:
tr2.hookargs['tag_moved'] = '1'
+with repo.vfs('changes/tags.changes', 'w',
+ atomictemp=True) as changesfile:
+# note: we do not register the file to the transaction
+# because we needs it to still exist on the transaction
+# is close (for txnclose hooks)
+tagsmod.writediff(changesfile, changes)
def validate(tr2):
"""will run pre-closing hooks"""
# XXX the transaction API is a bit lacking here so we take a hacky
diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -129,6 +129,44 @@ def difftags(ui, repo, oldfnodes, newfno
entries.sort()
return entries
+def writediff(fp, difflist):
+"""write tags diff information to a file.
+
+Data are stored with a line based format:
+
+ \n
+
+Action are defined as follow:
+ -R tag is removed,
+ +A tag is added,
+ -M tag is moved (old value),
+ +M tag is moved (new value),
+
+Example:
+
+ +A 875517b4806a848f942811a315a5bce30804ae85 t5
+
+See documentation of difftags output for details about the input.
+"""
+add = '+A %s %s\n'
+remove = '-R %s %s\n'
+updateold = '-M %s %s\n'
+updatenew = '+M %s %s\n'
+for tag, old, new in difflist:
+# translate to hex
+if old is not None:
+old = hex(old)
+if new is not None:
+new = hex(new)
+# write to file
+if old is None:
+fp.write(add % (new, tag))
+elif new is None:
+fp.write(remove % (old, tag))
+else:
+fp.write(updateold % (old, tag))
+fp.write(updatenew % (new, tag))
+
def findglobaltags(ui, repo):
'''Find global tags in a repo: return a tagsmap
diff --git a/tests/test-tag.t b/tests/test-tag.t
--- a/tests/test-tag.t
+++ b/tests/test-tag.t
@@ -11,6 +11,7 @@
> # file...
> if [ -n "\$HG_TAG_MOVED" ]; then
> echo 'hook: tag changes detected'
+ > sed 's/^/hook: /' .hg/changes/tags.changes
> fi
> EOF
$ chmod +x taghook.sh
@@ -37,6 +38,7
[PATCH 1 of 4 NEW-CONCEPT] tags: introduce a function to return a valid fnodes list from revs
# HG changeset patch # User Pierre-Yves David # Date 1490670416 -7200 # Tue Mar 28 05:06:56 2017 +0200 # Node ID f6e7b8a14f4a94db6742410331db752112c76b82 # Parent dea2a17cbfd00bf08ee87b3e44b1c71499189f89 # EXP-Topic tags # Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ # hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r f6e7b8a14f4a tags: introduce a function to return a valid fnodes list from revs This will get used to compare tags between two set of revisions during a transaction (pre and post heads). The end goal is to be able to track tags movement in transaction hooks. diff --git a/mercurial/tags.py b/mercurial/tags.py --- a/mercurial/tags.py +++ b/mercurial/tags.py @@ -78,6 +78,18 @@ from . import ( # The most recent changeset (in terms of revlog ordering for the head # setting it) for each tag is last. +def fnoderevs(ui, repo, revs): +"""return the list of '.hgtags' fnodes used in a set revisions + +This is returned as list of unique fnodes. We use a list instead of a set +because order matters when it comes to tags.""" +unfi = repo.unfiltered() +tonode = unfi.changelog.node +nodes = [tonode(r) for r in revs] +fnodes = _getfnodes(ui, repo, nodes[::-1]) # reversed help the cache +fnodes = _filterfnodes(fnodes, nodes) +return fnodes + def findglobaltags(ui, repo): '''Find global tags in a repo: return a tagsmap ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 4 NEW-CONCEPT] track-tags: introduce first bits of tags tracking during transaction
# HG changeset patch
# User Pierre-Yves David
# Date 1490675889 -7200
# Tue Mar 28 06:38:09 2017 +0200
# Node ID 25952f350daaae03506991d566c8de170b5db094
# Parent f6e7b8a14f4a94db6742410331db752112c76b82
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
# hg pull
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 25952f350daa
track-tags: introduce first bits of tags tracking during transaction
This changeset introduces detection of tags changes during transaction. When
this happens a 'tag_moved=1' argument is set for hooks, similar to what we do
for bookmarks and phases.
This code is disabled by default as there is still various performance
concerns. Some require a smarter use of our existing tag caches and some other
require rework around the transaction logic to skip execution when unneeded.
These performance improvement have been delayed, I would like to be able to
experiment and stabilize the feature behavior first.
Later changesets will push the concept further and provide a way for hooks to
knows what are the actual changes introduced by the transaction. Similar work
is needed for the other families of changes (bookmark, phase, obsolescence,
etc). Upgrade of the transaction logic will likely be performed at the same
time.
The current code can report some false positive when .hgtags file changes but
resulting tags are unchanged. This will be fixed in the next changeset.
For testing, we simply globally enable a hook in the tag test as all the
possible tag update cases should exist there. A couple of them show the false
positive mentionned above.
See in code documentation for more details.
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1001,8 +1001,54 @@ class localrepository(object):
vfsmap = {'plain': self.vfs} # root of .hg/
# we must avoid cyclic reference between repo and transaction.
reporef = weakref.ref(self)
-def validate(tr):
+# Code to track tag movement
+#
+# Since tags are all handled as file content, it is actually quite hard
+# to track these movement from a code perspective. So we fallback to a
+# tracking at the repository level. One could envision to track changes
+# to the '.hgtags' file through changegroup apply but that fails to
+# cope with case where transaction expose new heads without changegroup
+# being involved (eg: phase movement).
+#
+# For now, We gate the feature behind a flag since this likely comes
+# with performance impacts. The current code run more often than needed
+# and do not use caches as much as it could. The current focus is on
+# the behavior of the feature so we disable it by default. The flag
+# will be removed when we are happy with the performance impact.
+tracktags = lambda x: None
+# experimental config: experimental.hook-track-tags
+shouldtracktags = self.ui.configbool('experimental', 'hook-track-tags',
+ False)
+if desc != 'strip' and shouldtracktags:
+oldheads = self.changelog.headrevs()
+def tracktags(tr2):
+repo = reporef()
+oldfnodes = tagsmod.fnoderevs(repo.ui, repo, oldheads)
+newheads = repo.changelog.headrevs()
+newfnodes = tagsmod.fnoderevs(repo.ui, repo, newheads)
+# notes: we compare lists here.
+# As we do it only once buiding set would not be cheaper
+if oldfnodes != newfnodes:
+tr2.hookargs['tag_moved'] = '1'
+def validate(tr2):
"""will run pre-closing hooks"""
+# XXX the transaction API is a bit lacking here so we take a hacky
+# path for now
+#
+# We cannot add this as a "pending" hooks since the 'tr.hookargs'
+# dict is copied before these run. In addition we needs the data
+# available to in memory hooks too.
+#
+# Moreover, we also needs to make sure this runs before txnclose
+# hooks and there is no "pending" mechanism that would execute
+# logic only if hooks are about to run.
+#
+# Fixing this limitation of the transaction is also needed to track
+# other families of changes (bookmarks, phases, obsolescence).
+#
+# This will have to be fixed before we remove the experimental
+# gating.
+tracktags(tr2)
reporef().hook('pretxnclose', throw=True,
txnname=desc, **pycompat.strkwargs(tr.hookargs))
def releasefn(tr, success):
diff --git a/tests/test-tag.t b/tests/test-tag.t
--- a/tests/test-tag.t
+++ b/tests/test-tag.t
@@ -1,3 +1,19 @@
[PATCH 3 of 4 NEW-CONCEPT] track-tags: compute the actual differences between tags pre/post transaction
# HG changeset patch
# User Pierre-Yves David
# Date 1490688895 -7200
# Tue Mar 28 10:14:55 2017 +0200
# Node ID 2dcaa99d6160561166269f192a89b25f84fd4667
# Parent 25952f350daaae03506991d566c8de170b5db094
# EXP-Topic tags
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
# hg pull
https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r 2dcaa99d6160
track-tags: compute the actual differences between tags pre/post transaction
We now compute the proper actuall differences between tags before and after the
transaction. This catch a couple of false positives in the tests.
The compute the full difference since we are about to make this data available
to hooks in the next changeset.
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1028,7 +1028,8 @@ class localrepository(object):
newfnodes = tagsmod.fnoderevs(repo.ui, repo, newheads)
# notes: we compare lists here.
# As we do it only once buiding set would not be cheaper
-if oldfnodes != newfnodes:
+changes = tagsmod.difftags(repo.ui, repo, oldfnodes, newfnodes)
+if changes:
tr2.hookargs['tag_moved'] = '1'
def validate(tr2):
"""will run pre-closing hooks"""
diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -90,6 +90,45 @@ def fnoderevs(ui, repo, revs):
fnodes = _filterfnodes(fnodes, nodes)
return fnodes
+def _nulltonone(value):
+"""convert nullid to None
+
+For tag value, nullid means "deleted". This small utility function helps
+translating that to None."""
+if value == nullid:
+return None
+return value
+
+def difftags(ui, repo, oldfnodes, newfnodes):
+"""list differences between tags expressed in two set of file-nodes
+
+The list contains entries in the form: (tagname, oldvalue, new value).
+None is used to expressed missing value:
+('foo', None, 'abcd') is a new tag,
+('bar', 'ef01', None) is a deletion,
+('baz', 'abcd', 'ef01') is a tag movement.
+"""
+if oldfnodes == newfnodes:
+return []
+oldtags = _tagsfromfnodes(ui, repo, oldfnodes)
+newtags = _tagsfromfnodes(ui, repo, newfnodes)
+
+# list of (tag, old, new): None means missing
+entries = []
+for tag, (new, __) in newtags.items():
+new = _nulltonone(new)
+old, __ = oldtags.pop(tag, (None, None))
+old = _nulltonone(old)
+if old != new:
+entries.append((tag, old, new))
+# handle deleted tags
+for tag, (old, __) in oldtags.items():
+old = _nulltonone(old)
+if old is not None:
+entries.append((tag, old, None))
+entries.sort()
+return entries
+
def findglobaltags(ui, repo):
'''Find global tags in a repo: return a tagsmap
diff --git a/tests/test-tag.t b/tests/test-tag.t
--- a/tests/test-tag.t
+++ b/tests/test-tag.t
@@ -230,7 +230,6 @@ doesn't end with EOL
> f = file('.hgtags', 'w'); f.write(last); f.close()
> EOF
$ hg ci -m'broken manual edit of .hgtags'
- hook: tag changes detected
$ cat .hgtags; echo
acb14030fe0a21b60322c440ad2d20cf7685a376 foobar
$ hg tag newline
@@ -635,7 +634,6 @@ handle the loss of tags
$ printf '' > .hgtags
$ hg commit -m 'delete all tags'
created new head
- hook: tag changes detected
$ hg log -r 'max(t7::)'
changeset: 17:ffe462b50880
user:test
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 3 V2] hardlink: extract topic text logic of copyfiles
On Wed, 29 Mar 2017 12:42:16 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu
> # Date 1490815275 25200
> # Wed Mar 29 12:21:15 2017 -0700
> # Node ID b1ef68e4196e01f723b78746d752f60e46e33cc0
> # Parent cda83a1bfb3ac3a23cfa158c407be93755c1018e
> # Available At https://bitbucket.org/quark-zju/hg-draft
> # hg pull https://bitbucket.org/quark-zju/hg-draft -r
> b1ef68e4196e
> hardlink: extract topic text logic of copyfiles
Looks good, queued, thanks.
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -1130,8 +1130,7 @@ def copyfiles(src, dst, hardlink=None, p
> hardlink = (os.stat(src).st_dev ==
> os.stat(os.path.dirname(dst)).st_dev)
> -if hardlink:
> -topic = _('linking')
> -else:
> -topic = _('copying')
> +
> +gettopic = lambda: hardlink and _('linking') or _('copying')
> +topic = gettopic()
IIRC, we prefer not using and/or ternary. I would instead do:
topicmap = [_('copying'), _('linking')]
topicmap[hardlink]
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] hgweb: fix diff hunks filtering by line range in webutil.diffs()
> On Mar 30, 2017, at 01:27, Denis Laxalde wrote: > > Gregory Szorc a écrit : >>> On Wed, Mar 29, 2017 at 6:10 AM, Denis Laxalde wrote: >>> >>> # HG changeset patch >>> # User Denis Laxalde >>> # Date 1490782027 -7200 >>> # Wed Mar 29 12:07:07 2017 +0200 >>> # Node ID 28297d6c3ae842f4c26f878bf5b107619366fbd5 >>> # Parent e540846c67e0f838bcdb1db567a57df28d92491c >>> # Available At http://hg.logilab.org/users/dlaxalde/hg >>> # hg pull http://hg.logilab.org/users/dlaxalde/hg -r >>> 28297d6c3ae8 >>> # EXP-Topic linerange-log/hgweb-filelog >>> hgweb: fix diff hunks filtering by line range in webutil.diffs() >>> >>> The previous clause for filter out a diff hunk was too restrictive. We >>> need to >>> consider the following cases (assuming linerange=(lb, ub) and the @s2,l2 >>> hunkrange): >>> >>><-(s2)(s2+l2)-> >>> <-(lb)---(ub)-> >>> <-(lb)---(ub)-> >>> <-(lb)---(ub)-> >>> >>> previously on the first and last situations were considered. >>> >>> In test-hgweb-filelog.t, add a couple of lines at the beginning of file >>> "b" so >>> that the line range we will follow does not start at the beginning of file. >>> This covers the change in aforementioned diff hunk filter clause. >>> >> I think this is better. But there's still some oddity here. Specifically, a >> number of diffs render extra lines that don't appear to be relevant to the >> initial line range. For example, on mozilla-central, the request for >> /log/60d7a0496a36/layout/base/nsCSSFrameConstructor.cpp?patch=&linerange=519:534 >> has an initial diff covering 13,000+ lines (the entire file). It seems we >> went from showing too little diff (no diff) to too much diff (all diff). > > > As far as I can tell, in all diffs on this page, there are only hunks > (in general one, sometimes two) in which there are changes that concern > the initial line range (GetLastIBSplitSibling function, renamed as > GetLastSpecialSibling). Do you agree with that? > > It is true that some hunks spread far from the target line range (for > instance the second row "Bug 508665 - part 3 [...]" (75c14b62556e) or > "Fix crash bug 393517. [...]" (8d561edb0012)). This is because the > algorithm does not split hunks but only filter them. In other words, if > mdiff.allblocks() (or more likely bdiff.blocks()) produces "too big" > hunks, there's nothing we can do with the current implementation. Ahh, I understand now. The new behavior obviously makes sense given the underlying implementation of being hunk based instead of line based. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH RFC] repo: add an ability to hide nodes in an appropriate way
On 03/27/2017 07:24 PM, Augie Fackler wrote: On Mon, Mar 27, 2017 at 10:27:33AM +0200, Pierre-Yves David wrote: On 03/27/2017 12:32 AM, Kostia Balytskyi wrote: # HG changeset patch # User Kostia Balytskyi # Date 1490567500 25200 # Sun Mar 26 15:31:40 2017 -0700 # Node ID 43cd56f38c1ca2ad1920ed6804e1a90a5e263c0d # Parent c60091fa1426892552dd6c0dd4b9c49e3c3da045 repo: add an ability to hide nodes in an appropriate way Potentially frequent usecase is to hide nodes in the most appropriate for current repo configuration way. Examples of things which use this are rebase (strip or obsolete old nodes), shelve (strip of obsolete temporary nodes) and probably others. Jun Wu had an idea of having one place in core where this functionality is implemented so that others can utilize it. This patch implements this idea. I do not think this is a step in the right direction, creating obsolescence marker is not just about hiding changeset. But we're using them for that today, all over the place. Actually, we do not use it all over the place. Current markers usages in core are: amend: - record evolution history - prune the temporary amend commit (that I would rather see disappear all together) rebase: - record evolution history on successful completion histedit: - record evolution history on successful completion - prune temporary nodes on successsful completion (This ones used to be stripped even with evolution) (let us talk about shelve later) So they are mainly used to record evolution history on successful operation (their intended usage) between "real-changesets". This is the intended usage of obsolescence markers. In addition, they are also used in a couple of place to hide temporary changesets (in the "utility-space") after an operation succeeed. This a bit an unorthodox uses of the obsolescence markers, but is "okay" to use them in these case because we know: 1) this is utility-space so user never interact with them. 2) we only create them for completed successful operation so we'll never need them ever again. Strictly speaking we could just strip these temporary commits (and we have done so in the past) and this will not change anything for the user. Any other hiding mechanism aimed at "internal temporary" commit would do the job just fine. The internal commit never needs (and actually should ever) to leave the user repository. In practice, we could use in memory commit for most of these temporary commit (if not all) and never have to deal with hiding them. We're also having pretty productive (I think!) discussions about non-obsmarker based mechanisms for hiding things that are implementation details of a sort (amend changesets that get folded immediately, shelve changes, etc). (Let me be at be long to try to be clear and comprehensive) I can see three categories of things we want to hide: :obsolete changeset (evolution semantic): A rewrite operation has created/recorded a new version of this changesets. that old version no longer needs to be shown to users (if possible). There is a strong semantic associated with such changesets and the property will be propagated to other repositories :temporary/internal changesets: Such changesets are created as a side effect of other operation (amend, histedit, etc). They have the following properties (once we are done using them) * They are irrelevant to the users, * We should never-ever base anything on them, * We should never-ever access them ever again, * They should never-ever leave the repository. * They never stop being internal-changesets :locally-hidden changeset: An hypothetically (not used) yet local-only hidden mechanism (similar to strip but without actual repo stripping). This could be used for local hiding/masking of changeset in the "real-changeset" space. Such data would not be exchanged. Commit reappears when "recreated" or re-pulled. --- To take practical examples: :amend: - amend source: "obsolete-changeset" - temporary changeset: "internal-changeset" :rebase: - rebase source, on success: "obsolete-changeset" - rebase result, on abort: "local-hiding" :histedit: - histedit source, on success: "obsolete-changeset" - histedit temporary node, on success: "internal-changeset" - histedit result, on abort: "local-hiding" - histedit temporary node, on abort: "local-hiding (internal-node)" extra notes: * If local hiding (strip) would take care of obsolescence marker, rebase and histedit could create them "as they go" instead of to the end: "on success". * In rebase --abort and histedit --abort, strip is used on freshly created changesets, so its performance impact is limited * we use "local hiding" (strip) on temporary nodes on histedit abort because we needs to be able to reuse them if histedit is rerun. So we cannot throw them in oblivion just yet. --- regarding implementations "obsolete-changesets": we already have a
Re: [PATCH 3 of 3] minirst: remove redundant _admonitions set
On Thu, 30 Mar 2017 09:38:31 +0100, Ryan McElroy wrote: > On 3/30/17 4:19 AM, Gregory Szorc wrote: > > # HG changeset patch > > # User Gregory Szorc > > # Date 1490843966 25200 > > # Wed Mar 29 20:19:26 2017 -0700 > > # Node ID 191b0b4697cc979464ec5ad9f04ef1ffa0f2a6cb > > # Parent c4329b811738046b70661f5647df50d7d28b2362 > > minirst: remove redundant _admonitions set > > This series looks good to me and is a nice cleanup. Marked as > pre-reviewed in patchwork. Queued, many thanks. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2 py3] color: replace str() with pycompat.bytestr()
On Thu, 30 Mar 2017 01:21:38 +0530, Pulkit Goyal wrote: > # HG changeset patch > # User Pulkit Goyal <7895pul...@gmail.com> > # Date 1490779072 -19800 > # Wed Mar 29 14:47:52 2017 +0530 > # Node ID 7339bf21508131c90a44317ff16214b81ebe62c1 > # Parent 08aeba5bc7c623a700eea9011e0d904e3732134a > color: replace str() with pycompat.bytestr() Queued these, thanks. > --- a/mercurial/color.py Sun Mar 26 20:52:51 2017 +0530 > +++ b/mercurial/color.py Wed Mar 29 14:47:52 2017 +0530 > @@ -332,9 +332,10 @@ > stop = _effect_str(ui, 'none') > else: > activeeffects = _activeeffects(ui) > -start = [str(activeeffects[e]) for e in ['none'] + effects.split()] > +start = [pycompat.bytestr(activeeffects[e]) for e in ['none'] > ++ effects.split()] > start = '\033[' + ';'.join(start) + 'm' > -stop = '\033[' + str(activeeffects['none']) + 'm' > +stop = '\033[' + pycompat.bytestr(activeeffects['none']) + 'm' This could be '%d'. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH py3 V2] diff: slice over bytes to make sure conditions work normally
On Thu, 30 Mar 2017 01:16:07 +0530, Pulkit Goyal wrote: > # HG changeset patch > # User Pulkit Goyal <7895pul...@gmail.com> > # Date 1490541771 -19800 > # Sun Mar 26 20:52:51 2017 +0530 > # Node ID 08aeba5bc7c623a700eea9011e0d904e3732134a > # Parent 331cc4433efe0d897bb16ad4ff08a3fbe850869b > diff: slice over bytes to make sure conditions work normally Queued, thanks. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 7 V2] tags: extract fnode retrieval in its own function
On Tue, 28 Mar 2017 20:20:05 +0100, Ryan McElroy wrote: > On 3/28/17 1:03 PM, Pierre-Yves David wrote: > > # HG changeset patch > > # User Pierre-Yves David > > # Date 1490673691 -7200 > > # Tue Mar 28 06:01:31 2017 +0200 > > # Node ID a710d3c24acd544408fe77243102325514f5f697 > > # Parent e6fd7930cf0b37710e379de37b7d87d5c1ea5dc9 > > # EXP-Topic tags > > tags: extract fnode retrieval in its own function > s/in/into > > Other than this nitpick (which I missed the first time), this whole > series looks good to me and is a nice clean-up. Marked at pre-reviewed. Yeah, looks nice. Queued, thanks. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] hgweb: fix diff hunks filtering by line range in webutil.diffs()
On Wed, 29 Mar 2017 15:10:23 +0200, Denis Laxalde wrote: > # HG changeset patch > # User Denis Laxalde > # Date 1490782027 -7200 > # Wed Mar 29 12:07:07 2017 +0200 > # Node ID 28297d6c3ae842f4c26f878bf5b107619366fbd5 > # Parent e540846c67e0f838bcdb1db567a57df28d92491c > # Available At http://hg.logilab.org/users/dlaxalde/hg > # hg pull http://hg.logilab.org/users/dlaxalde/hg -r 28297d6c3ae8 > # EXP-Topic linerange-log/hgweb-filelog > hgweb: fix diff hunks filtering by line range in webutil.diffs() > > The previous clause for filter out a diff hunk was too restrictive. We need to > consider the following cases (assuming linerange=(lb, ub) and the @s2,l2 > hunkrange): > > <-(s2)(s2+l2)-> > <-(lb)---(ub)-> ><-(lb)---(ub)-> ><-(lb)---(ub)-> > > previously on the first and last situations were considered. > > In test-hgweb-filelog.t, add a couple of lines at the beginning of file "b" so > that the line range we will follow does not start at the beginning of file. > This covers the change in aforementioned diff hunk filter clause. > > diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py > --- a/mercurial/hgweb/webutil.py > +++ b/mercurial/hgweb/webutil.py > @@ -473,7 +473,7 @@ def diffs(web, tmpl, ctx, basectx, files > if linerange is not None and hunkrange is not None: > s1, l1, s2, l2 = hunkrange > lb, ub = linerange > -if not (lb <= s2 < ub or lb < s2 + l2 <= ub): > +if not (lb < s2 +l2 and ub > s2): Looks correct compared to mdiff.blocksinrange(). Queued, thanks. Maybe it'll be nice to add a helper (with docstring) that tests if a hunk range is included in line range. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 7 V5] hgweb: add a "patch" query parameter to filelog command
On Sat, 25 Mar 2017 10:20:57 +0100, Denis Laxalde wrote:
> # HG changeset patch
> # User Denis Laxalde
> # Date 1489398073 -3600
> # Mon Mar 13 10:41:13 2017 +0100
> # Node ID a7b9bc1af8b81c332cf34960f581359e6942ca17
> # Parent ffd1a15d456342caba744f5049b97862fcb697c7
> # Available At https://hg.logilab.org/users/dlaxalde/hg
> # hg pull https://hg.logilab.org/users/dlaxalde/hg -r
> a7b9bc1af8b8
> # EXP-Topic linerange-log/hgweb-filelog
> hgweb: add a "patch" query parameter to filelog command
> @@ -981,12 +985,27 @@ def filelog(web, req, tmpl):
> repo = web.repo
> revs = fctx.filelog().revs(start, end - 1)
> entries = []
> +
> +diffstyle = web.config('web', 'style', 'paper')
> +if 'style' in req.form:
> +diffstyle = req.form['style'][0]
> +
> +def diff(fctx):
> +ctx = fctx.changectx()
> +basectx = ctx.p1()
> +path = fctx.path()
> +return webutil.diffs(web, tmpl, ctx, basectx, [path], diffstyle)
> +
> for i in revs:
> iterfctx = fctx.filectx(i)
> +diffs = None
> +if patch:
> +diffs = diff(iterfctx)
Got an exception at hidden revision '5cb3b7d1f50f' which had been rebased (in
my local repo.) Perhaps linkrev wouldn't be adjusted because we walk revision
numbers, not a history of filelog.
Traceback (most recent call last):
File "mercurial/hgweb/server.py", line 100, in do_POST
self.do_write()
File "mercurial/hgweb/server.py", line 93, in do_write
self.do_hgweb()
File "mercurial/hgweb/server.py", line 161, in do_hgweb
for chunk in self.server.application(env, self._start_response):
File "mercurial/hgweb/hgweb_mod.py", line 315, in run_wsgi
for r in self._runwsgi(req, repo):
File "mercurial/util.py", line 867, in increasingchunks
for chunk in source:
File "mercurial/templater.py", line 1035, in _flatten
for j in _flatten(i):
File "mercurial/templater.py", line 1035, in _flatten
for j in _flatten(i):
File "mercurial/templater.py", line 1035, in _flatten
for j in _flatten(i):
File "mercurial/templater.py", line 1035, in _flatten
for j in _flatten(i):
File "mercurial/templater.py", line 1035, in _flatten
for j in _flatten(i):
File "mercurial/templater.py", line 1027, in _flatten
for i in thing:
File "mercurial/hgweb/webutil.py", line 467, in diffs
diffhunks = patch.diffhunks(repo, node1, node2, m, opts=diffopts)
File "mercurial/patch.py", line 2283, in diffhunks
ctx1 = repo[node1]
File "mercurial/localrepo.py", line 579, in __getitem__
return context.changectx(self, changeid)
File "mercurial/context.py", line 526, in __init__
_("unknown revision '%s'") % changeid)
RepoLookupError: unknown revision '5cb3b7d1f50f9857cc5316d3211789158deab238'
> + --- /dev/nullThu Jan 01 00:00:00
> 1970 +
We'll need a prefix to make 'id' unique.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 2 of 5 v2] rebase: test to show brokenness with requiredest
# HG changeset patch
# User Ryan McElroy
# Date 1490871010 25200
# Thu Mar 30 03:50:10 2017 -0700
# Node ID 8f154b4f7255427303aed7fff2d78ff84e49bfeb
# Parent 9f25299c45b39fbdfc34c0c3ef1c454bcd42ed02
rebase: test to show brokenness with requiredest
As shown in issue5513, --continue is broken when destination is required. This
adds a patch that demonstates this silly behavior, which will be fixed in a
future patch.
diff --git a/tests/test-rebase-dest.t b/tests/test-rebase-dest.t
--- a/tests/test-rebase-dest.t
+++ b/tests/test-rebase-dest.t
@@ -31,3 +31,30 @@ Require a destination
rebasing 2:279de9495438 "cc" (tip)
saved backup bundle to
$TESTTMP/repo/.hg/strip-backup/279de9495438-ab0a5128-backup.hg (glob)
+Requiring dest should not break continue or other rebase options
+ $ hg up 1 -q
+ $ echo d >> c
+ $ hg commit -qAm dc
+ $ hg log -G -T '{rev} {desc}'
+ @ 3 dc
+ |
+ | o 2 cc
+ |/
+ o 1 bb
+ |
+ o 0 aa
+
+ $ hg rebase -d 2
+ rebasing 3:0537f6b50def "dc" (tip)
+ merging c
+ warning: conflicts while merging c! (edit, then use 'hg resolve --mark')
+ unresolved conflicts (see hg resolve, then hg rebase --continue)
+ [1]
+ $ echo d > c
+ $ hg resolve --mark --all
+ (no more unresolved files)
+ continue: hg rebase --continue
+ $ hg rebase --continue
+ abort: you must specify a destination
+ (use: hg rebase -d REV)
+ [255]
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 1 of 5 v2] rebase: move destination test to new test file
# HG changeset patch # User Ryan McElroy # Date 1490871010 25200 # Thu Mar 30 03:50:10 2017 -0700 # Node ID 9f25299c45b39fbdfc34c0c3ef1c454bcd42ed02 # Parent 2632df096fc0ac7582382b1f94ea4b9ad0bce8f2 rebase: move destination test to new test file We'll be adding a lot more tests here, so it makes sense to have this in its own file now. diff --git a/tests/test-rebase-base.t b/tests/test-rebase-base.t --- a/tests/test-rebase-base.t +++ b/tests/test-rebase-base.t @@ -391,33 +391,3 @@ Multiple roots. Two children share two p / o 0: A -Require a destination - $ cat >> $HGRCPATH < [commands] - > rebase.requiredest = True - > EOF - $ hg init repo - $ cd repo - $ echo a >> a - $ hg commit -qAm aa - $ echo b >> b - $ hg commit -qAm bb - $ hg up ".^" - 0 files updated, 0 files merged, 1 files removed, 0 files unresolved - $ echo c >> c - $ hg commit -qAm cc - $ hg rebase - abort: you must specify a destination - (use: hg rebase -d REV) - [255] - $ hg rebase -d 1 - rebasing 2:5db65b93a12b "cc" (tip) - saved backup bundle to $TESTTMP/repo/.hg/strip-backup/5db65b93a12b-4fb789ec-backup.hg (glob) - $ hg rebase -d 0 -r . -q - $ HGPLAIN=1 hg rebase - rebasing 2:889b0bc6a730 "cc" (tip) - saved backup bundle to $TESTTMP/repo/.hg/strip-backup/889b0bc6a730-41ec4f81-backup.hg (glob) - $ hg rebase -d 0 -r . -q - $ hg --config commands.rebase.requiredest=False rebase - rebasing 2:279de9495438 "cc" (tip) - saved backup bundle to $TESTTMP/repo/.hg/strip-backup/279de9495438-ab0a5128-backup.hg (glob) diff --git a/tests/test-rebase-dest.t b/tests/test-rebase-dest.t new file mode 100644 --- /dev/null +++ b/tests/test-rebase-dest.t @@ -0,0 +1,33 @@ +Require a destination + $ cat >> $HGRCPATH < [extensions] + > rebase = + > [commands] + > rebase.requiredest = True + > EOF + $ hg init repo + $ cd repo + $ echo a >> a + $ hg commit -qAm aa + $ echo b >> b + $ hg commit -qAm bb + $ hg up ".^" + 0 files updated, 0 files merged, 1 files removed, 0 files unresolved + $ echo c >> c + $ hg commit -qAm cc + $ hg rebase + abort: you must specify a destination + (use: hg rebase -d REV) + [255] + $ hg rebase -d 1 + rebasing 2:5db65b93a12b "cc" (tip) + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/5db65b93a12b-4fb789ec-backup.hg (glob) + $ hg rebase -d 0 -r . -q + $ HGPLAIN=1 hg rebase + rebasing 2:889b0bc6a730 "cc" (tip) + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/889b0bc6a730-41ec4f81-backup.hg (glob) + $ hg rebase -d 0 -r . -q + $ hg --config commands.rebase.requiredest=False rebase + rebasing 2:279de9495438 "cc" (tip) + saved backup bundle to $TESTTMP/repo/.hg/strip-backup/279de9495438-ab0a5128-backup.hg (glob) + ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 4 of 5 v2] rebase: demonstrate behavior with requiredest and pull --rebase
# HG changeset patch # User Ryan McElroy # Date 1490871010 25200 # Thu Mar 30 03:50:10 2017 -0700 # Node ID 2b8f2a54323459455a1038ca3b5a98a03cf4fe0e # Parent 50219901327938be2f4d6df041b69d7498a7914d rebase: demonstrate behavior with requiredest and pull --rebase diff --git a/tests/test-rebase-dest.t b/tests/test-rebase-dest.t --- a/tests/test-rebase-dest.t +++ b/tests/test-rebase-dest.t @@ -57,3 +57,28 @@ Requiring dest should not break continue $ hg rebase --continue rebasing 3:0537f6b50def "dc" (tip) saved backup bundle to $TESTTMP/repo/.hg/strip-backup/0537f6b50def-be4c7386-backup.hg (glob) + + $ cd .. + +Check rebase.requiredest interaction with pull --rebase + $ hg clone repo clone + updating to branch default + 3 files updated, 0 files merged, 0 files removed, 0 files unresolved + $ cd repo + $ echo e > e + $ hg commit -qAm ee + $ cd .. + $ cd clone + $ echo f > f + $ hg commit -qAm ff + $ hg pull --rebase + pulling from $TESTTMP/repo + searching for changes + adding changesets + adding manifests + adding file changes + added 1 changesets with 2 changes to 2 files (+1 heads) + abort: you must specify a destination + (use: hg rebase -d REV) + [255] + ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 5 of 5 v2] rebase: abort hg pull --rebase if rebase.requiredest is set (issue5514)
# HG changeset patch
# User Ryan McElroy
# Date 1490871010 25200
# Thu Mar 30 03:50:10 2017 -0700
# Node ID 982ea504f855ed143d714e2923332e223b7feef4
# Parent 2b8f2a54323459455a1038ca3b5a98a03cf4fe0e
rebase: abort hg pull --rebase if rebase.requiredest is set (issue5514)
Previously, the pull would succeed, but the subsequent rebase would fail due
to the rebase.requiredest flag. Now abort earlier with a more useful error
message.
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1362,6 +1362,11 @@ def pullrebase(orig, ui, repo, *args, **
'Call rebase after pull if the latter has been invoked with --rebase'
ret = None
if opts.get('rebase'):
+if ui.configbool('commands', 'rebase.requiredest'):
+msg = _('rebase destination required by configuration')
+hint = _('use hg pull followed by hg rebase -d DEST')
+raise error.Abort(msg, hint=hint)
+
wlock = lock = None
try:
wlock = repo.wlock()
diff --git a/tests/test-rebase-dest.t b/tests/test-rebase-dest.t
--- a/tests/test-rebase-dest.t
+++ b/tests/test-rebase-dest.t
@@ -72,13 +72,7 @@ Check rebase.requiredest interaction wit
$ echo f > f
$ hg commit -qAm ff
$ hg pull --rebase
- pulling from $TESTTMP/repo
- searching for changes
- adding changesets
- adding manifests
- adding file changes
- added 1 changesets with 2 changes to 2 files (+1 heads)
- abort: you must specify a destination
- (use: hg rebase -d REV)
+ abort: rebase destination required by configuration
+ (use hg pull followed by hg rebase -d DEST)
[255]
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH 3 of 5 v2] rebase: allow destination-free continue and abort (issue5513)
# HG changeset patch
# User Ryan McElroy
# Date 1490871010 25200
# Thu Mar 30 03:50:10 2017 -0700
# Node ID 50219901327938be2f4d6df041b69d7498a7914d
# Parent 8f154b4f7255427303aed7fff2d78ff84e49bfeb
rebase: allow destination-free continue and abort (issue5513)
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -685,12 +685,6 @@ def rebase(ui, repo, **opts):
# Validate input and define rebasing points
destf = opts.get('dest', None)
-
-if ui.configbool('commands', 'rebase.requiredest'):
-if not destf:
-raise error.Abort(_('you must specify a destination'),
- hint=_('use: hg rebase -d REV'))
-
srcf = opts.get('source', None)
basef = opts.get('base', None)
revf = opts.get('rev', [])
@@ -776,6 +770,10 @@ def _definesets(ui, repo, destf=None, sr
cmdutil.checkunfinished(repo)
cmdutil.bailifchanged(repo)
+if ui.configbool('commands', 'rebase.requiredest') and not destf:
+raise error.Abort(_('you must specify a destination'),
+ hint=_('use: hg rebase -d REV'))
+
if destf:
dest = scmutil.revsingle(repo, destf)
diff --git a/tests/test-rebase-dest.t b/tests/test-rebase-dest.t
--- a/tests/test-rebase-dest.t
+++ b/tests/test-rebase-dest.t
@@ -55,6 +55,5 @@ Requiring dest should not break continue
(no more unresolved files)
continue: hg rebase --continue
$ hg rebase --continue
- abort: you must specify a destination
- (use: hg rebase -d REV)
- [255]
+ rebasing 3:0537f6b50def "dc" (tip)
+ saved backup bundle to
$TESTTMP/repo/.hg/strip-backup/0537f6b50def-be4c7386-backup.hg (glob)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 7 shelve-ext v5] shelve: add obs-based shelve functionality
On 30/03/2017 10:15, Ryan McElroy wrote:
>
>
> On 3/29/17 2:18 PM, Kostia Balytskyi wrote:
>> # HG changeset patch
>> # User Kostia Balytskyi
>> # Date 1490790691 25200
>> # Wed Mar 29 05:31:31 2017 -0700
>> # Node ID fc1144e5993a5c85060b913e2d92cd4b1b61772e
>> # Parent 0aa864184c9d78c11d18980cf0faa10828445ff5
>> shelve: add obs-based shelve functionality
>>
>> Obsolescense-based shelve works in a following way:
>
> s/ a/ the
sigh :/ Will fix.
>
>> 1. In order to shelve some changes, it creates a commit, records its
>> node into a .oshelve file and prunes created commit.
>> 2. In order to finish a shelve operation, transaction is just
>> closed and not aborted.
>
> s/transaction/the transaction
>
>>
>> diff --git a/hgext/shelve.py b/hgext/shelve.py
>> --- a/hgext/shelve.py
>> +++ b/hgext/shelve.py
>> @@ -380,10 +380,15 @@ def _nothingtoshelvemessaging(ui, repo,
>> else:
>> ui.status(_("nothing changed\n"))
>> -def _shelvecreatedcommit(repo, node, name):
>> -bases = list(mutableancestors(repo[node]))
>> -shelvedfile(repo, name, 'hg').writebundle(bases, node)
>> -cmdutil.export(repo, [node],
>> +def _shelvecreatedcommit(ui, repo, node, name, tr):
>> +if isobsshelve(repo, ui):
>> +shelvedfile(repo, name, 'oshelve').writeobsshelveinfo({
>> +'node': nodemod.hex(node)
>> +})
>> +else:
>> +bases = list(mutableancestors(repo[node]))
>> +shelvedfile(repo, name, 'hg').writebundle(bases, node)
>> +cmdutil.export(repo.unfiltered(), [node],
>> fp=shelvedfile(repo, name,
>> patchextension).opener('wb'),
>> opts=mdiff.diffopts(git=True))
>
> I'm a latecomer to paying attention to this series, so please push
> back on me if this has already been discussed, but I think it would be
> "cleaner" to not mix old shelve and logic, and instead have just have
> the main function _shelvecreatedcommit() call out to
> _obsshelvecreatedcommit() or _abortshelvecreatedcommit() (or whatever
> we call the traditional version).
>
> Again, feel free to push back if this has already been discussed or
> discarded as a direction, or if there are other reasons to not do it.
I agree that this can be made better. Sean Farley (if I'm not mistaken)
suggested that this should be a class. I do not mind refactrong it
later, but I want to get it in first as a functionality. Also,
currently, if I understand you correctly, you want to split an 11 line
function into two. I don't think it's worth it at the moment.
>
>> @@ -394,8 +399,13 @@ def _includeunknownfiles(repo, pats, opt
>> extra['shelve_unknown'] = '\0'.join(s.unknown)
>> repo[None].add(s.unknown)
>> -def _finishshelve(repo):
>> -_aborttransaction(repo)
>> +def _finishshelve(ui, repo, tr, node):
>> +if isobsshelve(repo, ui):
>> +obsolete.createmarkers(repo, [(repo.unfiltered()[node], ())])
>> +tr.close()
>> +tr.release()
>> +else:
>> +_aborttransaction(repo)
>> def _docreatecmd(ui, repo, pats, opts):
>> wctx = repo[None]
>> @@ -417,9 +427,12 @@ def _docreatecmd(ui, repo, pats, opts):
>> try:
>> lock = repo.lock()
>> -# use an uncommitted transaction to generate the bundle to
>> avoid
>> -# pull races. ensure we don't print the abort message to
>> stderr.
>> -tr = repo.transaction('commit', report=lambda x: None)
>> +# depending on whether shelve is traditional or
>> +# obsolescense-based, we either abort or commit this
>> +# transaction in the end. If we abort it, we don't
>> +# want to print anything to stderr
>> +report = None if isobsshelve(repo, ui) else (lambda x: None)
>> +tr = repo.transaction('commit', report=report)
>> interactive = opts.get('interactive', False)
>> includeunknown = (opts.get('unknown', False) and
>> @@ -447,16 +460,19 @@ def _docreatecmd(ui, repo, pats, opts):
>> _nothingtoshelvemessaging(ui, repo, pats, opts)
>> return 1
>> -_shelvecreatedcommit(repo, node, name)
>> +_shelvecreatedcommit(ui, repo, node, name, tr)
>> if ui.formatted():
>> desc = util.ellipsis(desc, ui.termwidth())
>> ui.status(_('shelved as %s\n') % name)
>> -hg.update(repo, parent.node())
>> +# current wc parent may be already obsolete becuase
>> +# it might have been created previously and shelve just
>> +# reuses it
>> +hg.update(repo.unfiltered(), parent.node())
>> if origbranch != repo['.'].branch() and not
>> _isbareshelve(pats, opts):
>> repo.dirstate.setbranch(origbranch)
>> -_finishshelve(repo)
>> +_finishshelve(ui, repo, tr, node)
>> finally:
>> _restoreactivebookmark(repo, activebookmark)
>> lockmod.release(tr, lock)
>>
>
> ___
Re: [PATCH 7 of 7 shelve-ext v5] shelve: add some forwards compatibility to shelve operations
On 3/29/17 2:18 PM, Kostia Balytskyi wrote:
# HG changeset patch
# User Kostia Balytskyi
# Date 1490790691 25200
# Wed Mar 29 05:31:31 2017 -0700
# Node ID 0953ca0a71a0e1f05078b0f3c255459e1ba56a4e
# Parent 9ce0a366a6316d1cffc2a875e7dc6b4a3227c851
shelve: add some forwards compatibility to shelve operations
This patch handles cases where someone enabled an obs-based
shelve in their repo, then disabled it while having made some
shelves. Approach of this patch is described below:
- assume this is what .hg/shelved looks like:
- sh1.patch (created 1s ago)
- sh1.oshelve (created 1s ago)
Would it make sense to have a "compatibility mode" where we also create
the bundle, so that unshelving is actually backwards-compatible (if this
option is enabled)?
- sh2.patch (created 2s ago)
- sh2.hg (created 2s ago)
- when obs-based shelve is enabled, 'hg shelve --list' shows both
sh1 and s2 and is able to unshelve both. 'hg unshelbe' (without --name)
s/unshelbe/unshelve
will unshelve sh1.
- when obs-based shelve is disabled, 'hg shelve --list' only shows
sh2, prints a warning that it found an obs-based shelve and is
only able to unshelve a traditional shelve. 'hg unshelve'
(without --name) will unshelve sh2.
diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -66,9 +66,15 @@ testedwith = 'ships-with-hg-core'
backupdir = 'shelve-backup'
shelvedir = 'shelved'
-shelvefileextensions = ['hg', 'patch', 'oshelve']
# universal extension is present in all types of shelves
patchextension = 'patch'
+# extension used for bundle-based traditional shelves
+traditionalextension = 'hg'
+# extension used for obs-based shelves
+obsshelveextension = 'oshelve'
+shelvefileextensions = [traditionalextension,
+patchextension,
+obsshelveextension]
# we never need the user, so we use a
# generic user for all shelve operations
@@ -530,7 +536,12 @@ def deletecmd(ui, repo, pats):
raise error.Abort(_("shelved change '%s' not found") % name)
def listshelves(repo):
-"""return all shelves in repo as list of (time, filename)"""
+"""return a tuple of (allshelves, obsshelvespresent)
+where
+'allshelves' is a list of (time, filename) for shelves available
+in current repo configuration
+'obsshelvespresent' is a bool indicating whether repo has
+any obs-based shelves"""
try:
names = repo.vfs.readdir(shelvedir)
except OSError as err:
@@ -538,13 +549,22 @@ def listshelves(repo):
raise
return []
info = []
+obsshelvedisabled = not isobsshelve(repo, repo.ui)
+obsshelvespresent = False
for (name, _type) in names:
pfx, sfx = name.rsplit('.', 1)
+if sfx == obsshelveextension:
+obsshelvespresent = True
if not pfx or sfx != patchextension:
continue
+traditionalfpath = repo.vfs.join(shelvedir,
+ pfx + '.' + traditionalextension)
+if obsshelvedisabled and not repo.vfs.exists(traditionalfpath):
+# this is not a traditional shelve
+continue
st = shelvedfile(repo, name).stat()
info.append((st.st_mtime, shelvedfile(repo, pfx).filename()))
-return sorted(info, reverse=True)
+return sorted(info, reverse=True), obsshelvespresent
def listcmd(ui, repo, pats, opts):
"""subcommand that displays the list of shelves"""
@@ -554,7 +574,13 @@ def listcmd(ui, repo, pats, opts):
width = ui.termwidth()
namelabel = 'shelve.newest'
ui.pager('shelve')
-for mtime, name in listshelves(repo):
+availableshelves, obsshelvespresent = listshelves(repo)
+if (obsshelvespresent and not isobsshelve(repo, repo.ui) and
+ui.configbool('shelve', 'showobswarning', True)):
New configs need to be documented.
+ui.warn(_('warning: obsolescense-based shelve is disabled, but '
+ 'obs-based shelve files are in the repo\n'
+ '(hint: set experimental.obsshelve=on in your hgrc)\n'))
+for mtime, name in availableshelves :
sname = util.split(name)[1]
if pats and sname not in pats:
continue
@@ -952,7 +978,7 @@ def _dounshelve(ui, repo, *shelved, **op
elif len(shelved) > 1:
raise error.Abort(_('can only unshelve one change at a time'))
elif not shelved:
-shelved = listshelves(repo)
+shelved, __ = listshelves(repo)
if not shelved:
raise error.Abort(_('no shelved changes to apply!'))
basename = util.split(shelved[0][1])[1]
I like the idea of warning users in this case -- thanks for thinking
about this case. I think that a backwards-compat mode might be a nice
option to allow safely testing this feature before diving in with both
feet. I know that would be a b
Re: [PATCH 6 of 7 shelve-ext v5] shelve: add obs-based unshelve functionality
On 3/29/17 2:18 PM, Kostia Balytskyi wrote:
# HG changeset patch
# User Kostia Balytskyi
# Date 1490790691 25200
# Wed Mar 29 05:31:31 2017 -0700
# Node ID 9ce0a366a6316d1cffc2a875e7dc6b4a3227c851
# Parent 743fea249a852994c5bd3e47cdfb617719f19a0a
shelve: add obs-based unshelve functionality
Obsolescense-based unshelve works as follows:
1. Instead of stripping temporary nodes, markers are created to
obsolete them.
2. Restoring commit is just finding it in an unfiltered repo.
3. '--keep' is only passed to rebase on traditional unshelves
(and thus traditional rebases), becuase we want markers to be
created fro obsolete-based rebases.
s/fro/for
4. 'hg unshelve' uses unfiltered repo to perform rebases
because we want rebase to be able to create markers between original
and new commits. 'rebaseskipobsolete' is disabled to make rebase not
skip the commit altogether.
5. As per sprint discussions, hiding commits with obsmarkers is
not ideal. This is okay for now, as long as obsshelve is an
experimental feature. In future, once a better approach to
s/In future/In the future
hiding things is implemented, we can change the way obsshelve
works (and also change its name to not refer to obsolescense).
This is a great comment.
Tests for obs-shelve are coming in a separate series.
Why?
diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -25,6 +25,7 @@ from __future__ import absolute_import
import collections
import errno
import itertools
+import time
from mercurial.i18n import _
from mercurial import (
@@ -254,8 +255,13 @@ class shelvedstate(object):
def prunenodes(self, ui, repo):
"""Cleanup temporary nodes from the repo"""
-repair.strip(ui, repo, self.nodestoprune, backup=False,
- topic='shelve')
+if self.obsshelve:
+unfi = repo.unfiltered()
+relations = [(unfi[n], ()) for n in self.nodestoprune]
+obsolete.createmarkers(repo, relations)
+else:
+repair.strip(ui, repo, self.nodestoprune, backup=False,
+ topic='shelve')
def cleanupoldbackups(repo):
vfs = vfsmod.vfs(repo.vfs.join(backupdir))
@@ -675,9 +681,14 @@ def unshelvecontinue(ui, repo, state, op
repo.vfs.rename('unshelverebasestate', 'rebasestate')
try:
-rebase.rebase(ui, repo, **{
-'continue' : True
-})
+# if shelve is obs-based, we want rebase to be able
+# to create markers to already-obsoleted commits
+_repo = repo.unfiltered() if state.obsshelve else repo
+with ui.configoverride({('experimental', 'rebaseskipobsolete'):
+'off'}, 'unshelve'):
+rebase.rebase(ui, _repo, **{
+'continue' : True,
+})
except Exception:
repo.vfs.rename('rebasestate', 'unshelverebasestate')
raise
@@ -717,31 +728,59 @@ def _commitworkingcopychanges(ui, repo,
with ui.configoverride({('ui', 'quiet'): True}):
node = cmdutil.commit(ui, repo, commitfunc, [], tempopts)
tmpwctx = repo[node]
+ui.debug("temporary working copy commit: %s:%s\n" %
+ (tmpwctx.rev(), nodemod.short(node)))
return tmpwctx, addedbefore
-def _unshelverestorecommit(ui, repo, basename):
+def _unshelverestorecommit(ui, repo, basename, obsshelve):
"""Recreate commit in the repository during the unshelve"""
with ui.configoverride({('ui', 'quiet'): True}):
-shelvedfile(repo, basename, 'hg').applybundle()
-shelvectx = repo['tip']
+if obsshelve:
+md = shelvedfile(repo, basename, 'oshelve').readobsshelveinfo()
+shelvenode = nodemod.bin(md['node'])
+repo = repo.unfiltered()
+try:
+shelvectx = repo[shelvenode]
+except error.RepoLookupError:
+m = _("shelved node %s not found in repo")
+raise error.Abort(m % md['node'])
+else:
+shelvedfile(repo, basename, 'hg').applybundle()
+shelvectx = repo['tip']
return repo, shelvectx
def _rebaserestoredcommit(ui, repo, opts, tr, oldtiprev, basename, pctx,
tmpwctx, shelvectx, branchtorestore,
- activebookmark):
+ activebookmark, obsshelve):
"""Rebase restored commit from its original location to a destination"""
# If the shelve is not immediately on top of the commit
# we'll be merging with, rebase it to be on top.
if tmpwctx.node() == shelvectx.parents()[0].node():
+# shelvectx is immediately on top of the tmpwctx
return shelvectx
+# we need a new commit extra every time we perform a rebase to ensure
+# that "nothing to rebase" does not happen with obs-based shelve
+# "noth
Re: [PATCH 2 of 7 shelve-ext v5] shelve: add a function to check whether obs-based shelve is enabled
On 3/29/17 2:18 PM, Kostia Balytskyi wrote:
# HG changeset patch
# User Kostia Balytskyi
# Date 1490790691 25200
# Wed Mar 29 05:31:31 2017 -0700
# Node ID 0aa864184c9d78c11d18980cf0faa10828445ff5
# Parent b5d6e501621808f330e46295d09526a5d1e9064a
shelve: add a function to check whether obs-based shelve is enabled
A central place to check whether code should use traditional or
obsolescense-based shelve behavior.
diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -41,6 +41,7 @@ from mercurial import (
mdiff,
merge,
node as nodemod,
+obsolete,
patch,
phases,
repair,
@@ -72,6 +73,18 @@ patchextension = 'patch'
# generic user for all shelve operations
shelveuser = 'shelve@localhost'
+def isobsshelve(repo, ui):
+"""Check whether obsolescense-based shelve is enabled"""
+obsshelve = ui.configbool('experimental', 'obsshelve')
Even though check-code doesn't require experimental flags to be
documented yet, we should be good citizens and add documentation for any
configuration we introduce.
+if not obsshelve:
+return False
+if not obsolete.isenabled(repo, obsolete.createmarkersopt):
+w = _('ignoring experimental.obsshelve because createmarkers option '
+ 'is disabled')
+ui.warn(w)
+return False
+return True
+
class shelvedfile(object):
"""Helper for the file storing a single shelve
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 7 shelve-ext v5] shelve: add obs-based shelve functionality
On 3/29/17 2:18 PM, Kostia Balytskyi wrote:
# HG changeset patch
# User Kostia Balytskyi
# Date 1490790691 25200
# Wed Mar 29 05:31:31 2017 -0700
# Node ID fc1144e5993a5c85060b913e2d92cd4b1b61772e
# Parent 0aa864184c9d78c11d18980cf0faa10828445ff5
shelve: add obs-based shelve functionality
Obsolescense-based shelve works in a following way:
s/ a/ the
1. In order to shelve some changes, it creates a commit, records its
node into a .oshelve file and prunes created commit.
2. In order to finish a shelve operation, transaction is just
closed and not aborted.
s/transaction/the transaction
diff --git a/hgext/shelve.py b/hgext/shelve.py
--- a/hgext/shelve.py
+++ b/hgext/shelve.py
@@ -380,10 +380,15 @@ def _nothingtoshelvemessaging(ui, repo,
else:
ui.status(_("nothing changed\n"))
-def _shelvecreatedcommit(repo, node, name):
-bases = list(mutableancestors(repo[node]))
-shelvedfile(repo, name, 'hg').writebundle(bases, node)
-cmdutil.export(repo, [node],
+def _shelvecreatedcommit(ui, repo, node, name, tr):
+if isobsshelve(repo, ui):
+shelvedfile(repo, name, 'oshelve').writeobsshelveinfo({
+'node': nodemod.hex(node)
+})
+else:
+bases = list(mutableancestors(repo[node]))
+shelvedfile(repo, name, 'hg').writebundle(bases, node)
+cmdutil.export(repo.unfiltered(), [node],
fp=shelvedfile(repo, name, patchextension).opener('wb'),
opts=mdiff.diffopts(git=True))
I'm a latecomer to paying attention to this series, so please push back
on me if this has already been discussed, but I think it would be
"cleaner" to not mix old shelve and logic, and instead have just have
the main function _shelvecreatedcommit() call out to
_obsshelvecreatedcommit() or _abortshelvecreatedcommit() (or whatever we
call the traditional version).
Again, feel free to push back if this has already been discussed or
discarded as a direction, or if there are other reasons to not do it.
@@ -394,8 +399,13 @@ def _includeunknownfiles(repo, pats, opt
extra['shelve_unknown'] = '\0'.join(s.unknown)
repo[None].add(s.unknown)
-def _finishshelve(repo):
-_aborttransaction(repo)
+def _finishshelve(ui, repo, tr, node):
+if isobsshelve(repo, ui):
+obsolete.createmarkers(repo, [(repo.unfiltered()[node], ())])
+tr.close()
+tr.release()
+else:
+_aborttransaction(repo)
def _docreatecmd(ui, repo, pats, opts):
wctx = repo[None]
@@ -417,9 +427,12 @@ def _docreatecmd(ui, repo, pats, opts):
try:
lock = repo.lock()
-# use an uncommitted transaction to generate the bundle to avoid
-# pull races. ensure we don't print the abort message to stderr.
-tr = repo.transaction('commit', report=lambda x: None)
+# depending on whether shelve is traditional or
+# obsolescense-based, we either abort or commit this
+# transaction in the end. If we abort it, we don't
+# want to print anything to stderr
+report = None if isobsshelve(repo, ui) else (lambda x: None)
+tr = repo.transaction('commit', report=report)
interactive = opts.get('interactive', False)
includeunknown = (opts.get('unknown', False) and
@@ -447,16 +460,19 @@ def _docreatecmd(ui, repo, pats, opts):
_nothingtoshelvemessaging(ui, repo, pats, opts)
return 1
-_shelvecreatedcommit(repo, node, name)
+_shelvecreatedcommit(ui, repo, node, name, tr)
if ui.formatted():
desc = util.ellipsis(desc, ui.termwidth())
ui.status(_('shelved as %s\n') % name)
-hg.update(repo, parent.node())
+# current wc parent may be already obsolete becuase
+# it might have been created previously and shelve just
+# reuses it
+hg.update(repo.unfiltered(), parent.node())
if origbranch != repo['.'].branch() and not _isbareshelve(pats, opts):
repo.dirstate.setbranch(origbranch)
-_finishshelve(repo)
+_finishshelve(ui, repo, tr, node)
finally:
_restoreactivebookmark(repo, activebookmark)
lockmod.release(tr, lock)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 5 of 5] perf: workaround check-code
On 3/30/17 12:57 AM, Jun Wu wrote: # HG changeset patch # User Jun Wu # Date 1490831529 25200 # Wed Mar 29 16:52:09 2017 -0700 # Node ID 5ca313b3da12d145f1d49a85dd8b753e22d51521 # Parent 265ea657d75905fb59a27194a75aaff49be94598 perf: workaround check-code The code int his series all looks good to me. I'll mark as pre-reviewed in patchwork. The only question I have is whether we prefer to leave check-code errors in place inside the test, or workaround it with hacks like this? I'd prefer to leave the "expected failures" inside the test because it documents that we intend to keep them (probably with a comment at the site of the "error"). Also, if the check-code regex gets smarter in the future, we don't have to go and change our hack again. However, I don't know what the rest of the community thinks about this. I don't have a strong enough opinion, so I guess I'm -0 on this patch, but +1 on the rest of the series. I'll be happy as long as the first four get in regardless of if patch 5 is included or not. The check-code suggests using rev instead of node for revlog.revision. But early Mercurial does not support that (see 9117c6561b0b). So let's just workaround check-code in perf.py diff --git a/contrib/perf.py b/contrib/perf.py --- a/contrib/perf.py +++ b/contrib/perf.py @@ -847,4 +847,5 @@ def perfrevlog(ui, repo, file_=None, sta def d(): r = cmdutil.openrevlog(repo, 'perfrevlog', file_, opts) +r2 = r # workaround check-code startrev = 0 @@ -857,5 +858,5 @@ def perfrevlog(ui, repo, file_=None, sta for x in xrange(startrev, endrev, dist): -r.revision(r.node(x)) +r2.revision(r.node(x)) timer(d) diff --git a/tests/test-check-code.t b/tests/test-check-code.t --- a/tests/test-check-code.t +++ b/tests/test-check-code.t @@ -10,7 +10,4 @@ New errors are not allowed. Warnings are $ hg locate -X contrib/python-zstandard -X hgext/fsmonitor/pywatchman | > sed 's-\\-/-g' | xargs "$check_code" --warnings --per-file=0 || false - contrib/perf.py:859: - > r.revision(r.node(x)) - don't covert rev to node before passing to revision(nodeorrev) Skipping i18n/polib.py it has no-che?k-code (glob) mercurial/demandimport.py:312: ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] test-verify: add a testcase where the file has magic meta header
On 3/29/17 9:45 PM, Jun Wu wrote: # HG changeset patch # User Jun Wu # Date 1490820077 25200 # Wed Mar 29 13:41:17 2017 -0700 # Node ID e9fda3b8614a8b701bd48041afa8b709e1227f27 # Parent cda83a1bfb3ac3a23cfa158c407be93755c1018e test-verify: add a testcase where the file has magic meta header We use a magic header "\1\n" to store metadata like renames. See filelog.py. The patch adds tests about files with the special header. Sure, but why? Just to prevent regressions of handling files with this data that "looks like" metadata in them? What's the motivation for adding this test? Just future safety or did you run into something? I'm not against it -- adding this test seems like a good idea, but I'd love more context on what led you to write it. diff --git a/tests/test-verify.t b/tests/test-verify.t --- a/tests/test-verify.t +++ b/tests/test-verify.t @@ -318,2 +318,18 @@ test revlog format 0 1 files, 1 changesets, 1 total revisions $ cd .. + +Files with the meta header (see comment in filelog.size) + + $ cat > $TESTTMP/writemeta.py < import sys + > with open(sys.argv[1], 'wb') as f: + > f.write(b'\x01\n\x01\n%s' % sys.argv[2]) + > EOF + + $ hg init c + $ cd c + $ $PYTHON $TESTTMP/writemeta.py a '' + $ $PYTHON $TESTTMP/writemeta.py b 'b' + $ hg ci -Aqm meta a b + $ hg verify -q + $ cd .. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 2] verify: fix length check
On 3/29/17 10:52 PM, Jun Wu wrote:
# HG changeset patch
# User Jun Wu
# Date 1490824154 25200
# Wed Mar 29 14:49:14 2017 -0700
# Node ID ab735d6798fb133efdc2be8a441b51e7ba4d547b
# Parent 1f7890370b5437466534cbd0a313d21671dade03
verify: fix length check
According to the document added above, we should check L1 == L2, and the
only way to get L1 in all cases is to call "rawsize()", and the only way to
get L2 is to call "revision(raw=True)". Therefore the fix.
Meanwhile there are still a lot of things about flagprocessor broken in
revlog.py. Tests will be added after revlog.py gets fixed.
diff --git a/mercurial/verify.py b/mercurial/verify.py
--- a/mercurial/verify.py
+++ b/mercurial/verify.py
@@ -425,5 +425,6 @@ class verifier(object):
rp = fl.renamed(n)
if l != fl.size(i):
-if len(fl.revision(n)) != fl.size(i):
+# the "L1 == L2" check
+if len(fl.revision(n, raw=True)) != fl.rawsize(i):
self.err(lr, _("unpacked size is %s, %s
expected") %
(l, fl.size(i)), f)
This fix looks good and important to me.
I'd suggest sending a v2 with the names in the table documented more
clearly (from patch 1), as well as the typo you mentioned in your reply.
I'll mark as "changes requested" in patchwork.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 2] verify: document corner cases
On 3/29/17 10:51 PM, Jun Wu wrote: # HG changeset patch # User Jun Wu # Date 1490823901 25200 # Wed Mar 29 14:45:01 2017 -0700 # Node ID 1f7890370b5437466534cbd0a313d21671dade03 # Parent e9fda3b8614a8b701bd48041afa8b709e1227f27 verify: document corner cases It seems a good idea to list all kinds of "surprises" and expected behavior to make the upcoming changes easier to understand. Note: the comment added do not reflect the actual behavior of the current grammar-nit: s/do/does code. diff --git a/mercurial/verify.py b/mercurial/verify.py --- a/mercurial/verify.py +++ b/mercurial/verify.py @@ -381,4 +381,44 @@ class verifier(object): # verify contents +# +# Define 4 cases to help understand corner cases: +# +# | common | rename | meta | ext Can you define these four words? common = common case? rename = file was just copied from somewhere else? meta = ??? ext = extension that adds a flags processor? +# --- +# flags() | 0 | 0 | 0 | not 0 +# renamed() | False | True | False | ? +# rawtext[0:2]=='\1\n'| False | True | True | ? +# +# "rawtext" means the raw text stored in revlog data, which +# could be retrieved by "revision(rev, raw=True)". "text" +# mentioned below is "revision(rev, raw=False)". +# +# There are 3 different lengths stored physically: +# 1. L1: rawsize, stored in revlog index +# 2. L2: len(rawtext), stored in revlog data +# 3. L3: len(text), stored in revlog data if flags=0, or +# possibly somewhere else if flags!=0 +# +# L1 should be equal to L2. L3 is unrelated to L1 or L2. But How about "L3 can be different than L1 and L2" -- it would be the same if there are no flags, right? +# text may or may not affect commit hash depending on flag +# processors (see revlog.addflagprocessor). +# +# | common | rename | meta | ext +# - +#rawsize() | L1 | L1 | L1| L1 +# size() | L1 | L2-LM | L1(*) | L1 (?) +# len(rawtext) | L2 | L2 | L2| L2 +#len(text) | L2 | L2 | L2| L3 +# len(read()) | L2 | L2-LM | L2-LM | L3 +# +# LM: length of metadata, depending on rawtext +# (*): not ideal, see comment in filelog.size +# (?): could be "- len(meta)" if the resolved content has +# rename metadata +# +# Checks needed to be done: +# 1. length check: L1 == L2, in all cases. +# 2. hash check: depending on flag processor, we may need to +# use either "text" (external), or "rawtext" (in revlog). try: l = len(fl.read(n)) Overall this is pretty easy to follow and adds a lot of clarity. Just a couple of questions inline. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 7 of 7 V5] hgweb: expose a followlines UI in filerevision view (RFC)
Gregory Szorc a écrit : On Sat, Mar 25, 2017 at 2:21 AM, Denis Laxalde wrote: # HG changeset patch # User Denis Laxalde # Date 1489594320 -3600 # Wed Mar 15 17:12:00 2017 +0100 # Node ID ec77aa4ff2993057b604bdffb449d2d179525a9f # Parent 2f75006a7f31c97d29fd6dd9b72fa7cc9e7ab840 # Available At https://hg.logilab.org/users/dlaxalde/hg # hg pull https://hg.logilab.org/users/dlaxalde/hg -r ec77aa4ff299 # EXP-Topic linerange-log/hgweb-filelog hgweb: expose a followlines UI in filerevision view (RFC) In filerevision view (/file//) we add some event listeners on mouse selection of elements in the block. Those listeners will capture the range of mouse-selected lines and, upon mouse release, a box inviting to follow the history of selected lines will show up. This action is advertised by a :after pseudo-element on file lines that shows up on hover and invite to "select a block of lines to follow its history". All JavaScript code lives in a new "linerangelog.js" file, sourced in filerevision template (only in "paper" style for now). This is proposal implementation, comments welcome on any aspects. This feature is frigging awesome!!! I can pretty much guarantee that some engineers at Mozilla will go completely crazy for this feature. As I'm playing with it locally, I found a few paper cuts. First, on the initial revision introducing lines (last revision as rendered in hgweb), the diff is often (always?) empty. I can also see empty diffs in the intermediate changesets. For example, run this on mercurial/exchange.py and ask it for the line range of the _pushdiscovery(pushop) function. For this function, I get 2 empty diffs (revisions 233623d58c9a and ddb56e7e1b92). Highlighting _pushdiscoverychangeset() yields only an empty initial diff. Second, the highlighting UI is a bit confusing. It took me a few seconds to realize I had to actually hold down the mouse button and highlight the lines. I don't think users are accustomed to do this on text in web pages unless they are copying text. It was definitely surprising to me that highlighting lines of text resulted in a special pop-up appearing. I'm no web design expert, but how about this design. As you mouseover lines, you see a graphic cursor somewhere on that line. There is likely a label next to it saying "click anywhere to follow from this line" or something like that. This is similar to the floating text label you have now. When you mouse click, that floating cursor is dropped and locked into place on that line. On subsequent mouseovers, another cursor+label appears. The lines between the initial cursor and the current mouse location are highlighted somehow (possibly adding a different background color). The label says "click anywhere to follow lines XX to YY" or something. When the user clicks to drop the 2nd cursor, either they're taken directly to the filelog view or we get an inline box with links (like the line number mouseovers on the annotate page). If the user accidentally clicks to drop a cursor, they can clear the cursor be clicking the cursor or an "x" next to it. Another minor nitpick with the highlighting UI is the "follow lines" box may not appear next to the mouse cursor. I think we want it to appear near the cursor so it is easier to find/use. Just sent a V6 of this patch implementing most of you proposal. I only left aside the floating cursor and box ideas; the patch being already quite large, I'd like to postpone this for a follow-up. The UX can obviously be improved as a follow-up. I don't want to detract from getting the core of the feature landed. This is shaping up to be pretty amazing! ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 3] minirst: remove redundant _admonitions set
On 3/30/17 4:19 AM, Gregory Szorc wrote: # HG changeset patch # User Gregory Szorc # Date 1490843966 25200 # Wed Mar 29 20:19:26 2017 -0700 # Node ID 191b0b4697cc979464ec5ad9f04ef1ffa0f2a6cb # Parent c4329b811738046b70661f5647df50d7d28b2362 minirst: remove redundant _admonitions set This series looks good to me and is a nice cleanup. Marked as pre-reviewed in patchwork. As Yuya pointed out during a review a month ago, _admonitions and _admonitiontitles are largely redundant. With the last commit, they are exactly redundant. So, remove _admonitions and use _admonitiontitles.keys() instead. diff --git a/mercurial/minirst.py b/mercurial/minirst.py --- a/mercurial/minirst.py +++ b/mercurial/minirst.py @@ -413,24 +413,12 @@ def prunecomments(blocks): return blocks -_admonitions = set([ -'attention', -'caution', -'danger', -'error', -'hint', -'important', -'note', -'tip', -'warning', -]) - def findadmonitions(blocks, admonitions=None): """ Makes the type of the block an admonition block if the first line is an admonition directive """ -admonitions = admonitions or _admonitions +admonitions = admonitions or _admonitiontitles.keys() admonitionre = re.compile(br'\.\. (%s)::' % '|'.join(sorted(admonitions)), flags=re.IGNORECASE) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 6] rebase: allow destination-free continue and abort (issue5513)
On 3/29/17 6:15 PM, Martin von Zweigbergk wrote:
On Tue, Mar 28, 2017 at 2:31 PM, Ryan McElroy wrote:
# HG changeset patch
# User Ryan McElroy
# Date 1490735056 25200
# Tue Mar 28 14:04:16 2017 -0700
# Node ID ac6634fab80b59ba48958ca03289c4434e9483c4
# Parent 409a87200f6b079c9bfe1e85fd0715822346c402
rebase: allow destination-free continue and abort (issue5513)
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -705,7 +705,7 @@ def rebase(ui, repo, **opts):
raise error.Abort(msg)
if ui.configbool('commands', 'rebase.requiredest'):
-if not destf:
+if not contf and not abortf and not destf:
Instead of saying here that we don't care about the destination being
missing here, why not move it just above the call to _definesets() so
we're checking it only when we do care about? We could even move it
inside _definesets(). I hesitated to suggest that at first, because I
thought it was a lower-level function that should not deal with
command line parsing, but it already does a fair bit of that, so it
seems fine to handle this case there too. What do you think?
Looking at the code, this suggestion seems great. There are already a
number of aborts in there for conflicting flags; this is an obvious
addition there. Thanks for the suggestion! I'll send a v2 out in a bit
after also playing around with Jun's drawdag suggestion to see if it
helps me write more understandable tests.
raise error.Abort(_('you must specify a destination'),
hint=_('use: hg rebase -d REV'))
diff --git a/tests/test-rebase-dest.t b/tests/test-rebase-dest.t
--- a/tests/test-rebase-dest.t
+++ b/tests/test-rebase-dest.t
@@ -55,6 +55,5 @@ Requiring dest should not break continue
(no more unresolved files)
continue: hg rebase --continue
$ hg rebase --continue
- abort: you must specify a destination
- (use: hg rebase -d REV)
- [255]
+ rebasing 3:0537f6b50def "dc" (tip)
+ saved backup bundle to
$TESTTMP/repo/.hg/strip-backup/0537f6b50def-be4c7386-backup.hg (glob)
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH V6] hgweb: expose a followlines UI in filerevision view
# HG changeset patch
# User Denis Laxalde
# Date 1490819176 -7200
# Wed Mar 29 22:26:16 2017 +0200
# Node ID 1c360ab5640bc326dd6ff70bdee78e4da006dcef
# Parent e540846c67e0f838bcdb1db567a57df28d92491c
# Available At http://hg.logilab.org/users/dlaxalde/hg
# hg pull http://hg.logilab.org/users/dlaxalde/hg -r 1c360ab5640b
# EXP-Topic linerange-log/hgweb-filelog
hgweb: expose a followlines UI in filerevision view
In filerevision view (/file//) we add some event listeners on
mouse clicks of elements in the block.
Those listeners will capture a range of lines selected between two mouse
clicks and a box inviting to follow the history of selected lines will then
show up. Selected lines (i.e. the block of lines) get a CSS class which make
them highlighted. Selection can be cancelled (and restarted) by either
clicking on the cancel ("x") button in the invite box or clicking on any other
source line. Also clicking twice on the same line will abort the selection and
reset event listeners to restart the process.
As a first step, this action is only advertised by the "cursor: cell" CSS rule
on source lines elements as any other mechanisms would make the code
significantly more complicated. This might be improved later.
All JavaScript code lives in a new "linerangelog.js" file, sourced in
filerevision template (only in "paper" style for now).
diff --git a/contrib/wix/templates.wxs b/contrib/wix/templates.wxs
--- a/contrib/wix/templates.wxs
+++ b/contrib/wix/templates.wxs
@@ -225,6 +225,7 @@
+
diff --git a/mercurial/templates/paper/filerevision.tmpl
b/mercurial/templates/paper/filerevision.tmpl
--- a/mercurial/templates/paper/filerevision.tmpl
+++ b/mercurial/templates/paper/filerevision.tmpl
@@ -71,8 +71,11 @@
line wrap: on
line source
-{text%fileline}
+{text%fileline}
+
+
+
diff --git a/mercurial/templates/static/linerangelog.js
b/mercurial/templates/static/linerangelog.js
new file mode 100644
--- /dev/null
+++ b/mercurial/templates/static/linerangelog.js
@@ -0,0 +1,137 @@
+// Copyright 2017 Logilab SA
+//
+// This software may be used and distributed according to the terms
+// of the GNU General Public License, incorporated herein by reference.
+
+//** Install event listeners for line block selection and followlines action */
+function installLineSelect() {
+var sourcelines = document.getElementsByClassName('sourcelines')[0];
+if (typeof sourcelines === 'undefined') {
+return;
+}
+// URL to complement with "linerange" query parameter
+var targetUri = sourcelines.dataset.logurl;
+if (typeof targetUri === 'undefined') {
+return;
+}
+
+var lineSelectedCSSClass = 'followlines-selected';
+
+//** add CSS class on element in `from`-`to` line range */
+function addSelectedCSSClass(from, to) {
+var spans = sourcelines.getElementsByTagName('span');
+for (var i = from; i <= to; i++) {
+spans.item(i).classList.add(lineSelectedCSSClass);
+}
+}
+
+//** remove CSS class from previously selected lines */
+function removeSelectedCSSClass() {
+var nodes = sourcelines.getElementsByClassName(
+lineSelectedCSSClass);
+while (nodes.length) {
+nodes[0].classList.remove(lineSelectedCSSClass);
+}
+}
+
+// add event listener to the whole block to have
+// it available in all children
+sourcelines.addEventListener('click', lineSelectStart);
+
+//** event handler for "click" on the first line of a block */
+function lineSelectStart(e) {
+var startElement = e.target;
+if (startElement.tagName !== 'SPAN') {
+// not a (maybe ): abort, keeping event listener
+// registered for other click with target
+return;
+}
+var startId = parseInt(startElement.id.slice(1));
+startElement.classList.add(lineSelectedCSSClass); // CSS
+
+// remove this event listener
+sourcelines.removeEventListener('click', lineSelectStart);
+
+//** event handler for "click" on the last line of the block */
+function lineSelectEnd(e) {
+var endElement = e.target;
+if (endElement.tagName !== 'SPAN') {
+// not a (maybe ): abort, keeping event listener
+// registered for other click with target
+return;
+}
+
+// remove this event listener
+sourcelines.removeEventListener('click', lineSelectEnd);
+
+// compute line range (startId, endId)
+var endId = parseInt(endElement.id.slice(1));
+if (endId == startId) {
+// clicked twice the same line, cancel and reset initial state
+// (CSS and event listener for selection start)
+removeSelectedCSSClass();
+sourcelines.ad
Re: [PATCH] hgweb: fix diff hunks filtering by line range in webutil.diffs()
Gregory Szorc a écrit : On Wed, Mar 29, 2017 at 6:10 AM, Denis Laxalde wrote: # HG changeset patch # User Denis Laxalde # Date 1490782027 -7200 # Wed Mar 29 12:07:07 2017 +0200 # Node ID 28297d6c3ae842f4c26f878bf5b107619366fbd5 # Parent e540846c67e0f838bcdb1db567a57df28d92491c # Available At http://hg.logilab.org/users/dlaxalde/hg # hg pull http://hg.logilab.org/users/dlaxalde/hg -r 28297d6c3ae8 # EXP-Topic linerange-log/hgweb-filelog hgweb: fix diff hunks filtering by line range in webutil.diffs() The previous clause for filter out a diff hunk was too restrictive. We need to consider the following cases (assuming linerange=(lb, ub) and the @s2,l2 hunkrange): <-(s2)(s2+l2)-> <-(lb)---(ub)-> <-(lb)---(ub)-> <-(lb)---(ub)-> previously on the first and last situations were considered. In test-hgweb-filelog.t, add a couple of lines at the beginning of file "b" so that the line range we will follow does not start at the beginning of file. This covers the change in aforementioned diff hunk filter clause. I think this is better. But there's still some oddity here. Specifically, a number of diffs render extra lines that don't appear to be relevant to the initial line range. For example, on mozilla-central, the request for /log/60d7a0496a36/layout/base/nsCSSFrameConstructor.cpp?patch=&linerange=519:534 has an initial diff covering 13,000+ lines (the entire file). It seems we went from showing too little diff (no diff) to too much diff (all diff). As far as I can tell, in all diffs on this page, there are only hunks (in general one, sometimes two) in which there are changes that concern the initial line range (GetLastIBSplitSibling function, renamed as GetLastSpecialSibling). Do you agree with that? It is true that some hunks spread far from the target line range (for instance the second row "Bug 508665 - part 3 [...]" (75c14b62556e) or "Fix crash bug 393517. [...]" (8d561edb0012)). This is because the algorithm does not split hunks but only filter them. In other words, if mdiff.allblocks() (or more likely bdiff.blocks()) produces "too big" hunks, there's nothing we can do with the current implementation. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] test-check-code: prevent files being added to the root directory
On 3/29/17 8:14 PM, Jun Wu wrote: # HG changeset patch # User Jun Wu # Date 1490814860 25200 # Wed Mar 29 12:14:20 2017 -0700 # Node ID 4b4345a62fbd9fb0f4610d013c4ee5c9c06287e0 # Parent cda83a1bfb3ac3a23cfa158c407be93755c1018e test-check-code: prevent files being added to the root directory This looks good to me. Marked as pre-reviewed in patchwork. Adding new files in the root directory is probably a mistake, and is usually discouraged [1]. The test catches it to avoid mistakes like [2]. Modify the test if files need to be added in the root. [1]: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_pipermail_mercurial-2Ddevel_2016-2DJuly_086442.html&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=7_IYmz-lrpqA1zpgDDo4EeVrvcwTfaFdAiVidGqOBWY&s=xA-QLqFwEuB9rS_dq7YumUvHY2zl3rwx0hZWq4lVgxQ&e= [2]: https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_pipermail_mercurial-2Ddevel_2017-2DMarch_095836.html&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Jw8rundaE7TbmqBYd1txIQ&m=7_IYmz-lrpqA1zpgDDo4EeVrvcwTfaFdAiVidGqOBWY&s=KUM_-mbWo_TeA41nPw9KKJrrcbuK9jFLugGhN9B7fEY&e= OMG I hate proofpoint. I want it to die in a fire. This is why I always remove the footers and the "available at" URLs haha. diff --git a/tests/test-check-code.t b/tests/test-check-code.t --- a/tests/test-check-code.t +++ b/tests/test-check-code.t @@ -55,2 +55,19 @@ New errors are not allowed. Warnings are ... 'command is %s; expected %s' % (commands[i], command)) ... break + +Prevent adding new files in the root directory accidentally. + + $ hg files 'glob:*' + .editorconfig + .hgignore + .hgsigs + .hgtags + CONTRIBUTING + CONTRIBUTORS + COPYING + Makefile + README + hg + hgeditor + hgweb.cgi + setup.py ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 5] repair: use ProgrammingError
On 3/29/17 4:56 PM, Jun Wu wrote: Excerpts from Pierre-Yves David's message of 2017-03-29 13:24:39 +0200: On 03/28/2017 08:59 PM, Ryan McElroy wrote: On 3/27/17 2:11 PM, Augie Fackler wrote: On Mon, Mar 27, 2017 at 09:18:55AM +0200, Pierre-Yves David wrote: Sure, that seems like a good idea. Can you see to it ? It's a good idea, but I feel like the name "ProgrammingError" alone provides some clue as to what's going on, and the migration is good. Queued, I wouldn't mind a followup to make the hint generic to all ProgrammingError instances, but I'm not willing to block on it either. FWIW, I agree that losing the hint is sad. How confident are we that core won't have these errors? Can we blame all ProgrammingError exceptions on extensions? The general idea about pointing at extensions here is that such bug in Mercurial should be caught by the Mercurial test suite (or local development process) so user should not be exposed to it (in theory). On the other hand, existing extensions code might use API wrong in a way that did not raised the error in previous version. So if the user get exposed to that error, it it probably because of an extension. I think the question was about *all* ProgrammingErrors, not about a particular one. I'm not sure if you have answered that. But maybe my English reading is just bad. FWIW, I think you can answer the question by either: Yes, all ProgrammingErrors are extensions' fault. or, No, some ProgrammingErrors are unrelated to extensions. which is just 60 bytes, more cleaner, 5x shorter, and save time for us all. Not sure how this is intended, Jun, but this statement come across text pretty harshly. Just be careful with your wording and your statements please. I think Pierre-Yves' statement adds important subtlety, which is that we expect all ProgrammingErrors in the wild to be caused by extensions. If a core developer sees one, we quickly fix it and add a test. I think we can use that subtlety in a hint for any programming error. I'll think about some possible hints to add here. Remember we're all on the same team here. Cheers, ~Ryan ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 4 of 7] revlog: add test to check raw processing is sane
On 3/29/17 4:34 PM, Jun Wu wrote: Although that's doable, that requires significant time. And the bugs are cancelling each other somehow: When you fix none, test 1 fails. When you fix test 1, test 2 previously passed fails. Since the bugs won't be able to be fixed incrementally (fixing the second introduce regression on the first), I'll prefer keeping it this way, that is to test all cases after the fixes. This is a great point. Carry on. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Post-pull updates and hooks
On 03/29/2017 11:57 PM, Augie Fackler wrote: Howdy folks, Alex (cc'ed) asked me a question on IRC today, and I can't figure out a fix for what is a basically-reasonable and sort-of-obvious feature. In Git, due to implementation details of how 'git pull' works[0], you can get a post-pull summary of the update from/to and a diffstat of what's changing out from under you. I swear this should be doable in hg (probably as a hook of some sort), but I can't figure out it. Anyone have any clever ideas? Relatedly, I noticed we don't appear to have a set of 'pull' hooks at all. Since 'changegroup' is now used for a lot of things internally (for example, histedit and rebase), should we add an explicit set of pre/post pull hooks for use in cases like this? The generic transaction set of hooks should do, the transaction "name" is available as HG_TXNNAME, it it set to "pull" in the pull case. You can get more details in `hg help config hooks` (pretxnopen, pretxnclose, txnclose). If you also want to get data about what --update did, the "update" hook should fit your needs (also see hg help config.hooks). If you want to push things a bit further, the '_destmerge' and '_destupdate' are experimental revset that can tell you about default update and merge destination. Cheers, -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
