This is an archive of the discontinued Mercurial Phabricator instance.

Differential D173

tweakdefaults: output new hashes when nodes get updated
ClosedPublic

Authored by • jeroenv on Jul 21 2017, 1:21 PM.

Download Raw Diff

Details

Reviewers

ryanmce

Group Reviewers

Restricted Project

Commits

rFBHGXdec644524c33: tweakdefaults: output new hashes when nodes get updated

Summary

Users and particularly automation can benefit from having the new
revision hashes as part of the output of rebase and other operations that
update nodes. Right now, hacks such as getting the tip revision are used to get
that information.

Test Plan

unit tests

Diff Detail

Repository

rFBHGX Facebook Mercurial Extensions

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

• jeroenv created this revision.Jul 21 2017, 1:21 PM

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 21 2017, 1:21 PM

Have you considered wrapping scmutil.cleanupnodes ? That allows people to get new hashes for not only rebase, but also histedit and amend (and more things to come).

hgext3rd/tweakdefaults.py
664	Is the limit useful? I think the source revisions are usually limited.
681	Maybe `[:10]` can be removed since we have `short` (equivalent to `hex(node)[:12]`) already.
tests/test-tweakdefaults-ordering.t
30 ↗	(On Diff #377)	This `0000000000` seems weird (same as below). Could you check what's happening here?

To your queue to answer questions about scmutil.cleanupnodes

hgext3rd/tweakdefaults.py
664	If you get rid of the `maxoutput` then loop can be simplified to for key, value in rebaseruntime.state.iteritems():
tests/test-fbamend-userestack.t
59–62 ↗	(On Diff #377)	It probably shouldn't but I'm still curious if it can break current automation.

This revision now requires changes to proceed.Jul 24 2017, 5:13 AM

Will take a look at wrapping scmutil.cleanupnodes.

hgext3rd/tweakdefaults.py
664	We do the same thing in `pushrebase`. Source can be an arbitrary revset right? For example someone with a lot of heads rebasing everything on top of master. More than happy to get rid of this. cc @durham who advised.
681	This is to shave another 2 characters off to limit output, but I agree that maybe it's a little silly.

• jeroenv added inline comments.Jul 24 2017, 5:45 AM

tests/test-fbamend-userestack.t
59–62 ↗	(On Diff #377)	I wondered the same. Durham wasn't concerned, mostly due to the unstructured output of rebase. Also this is in fb-hgext. Upstream might be a different story.

stash added inline comments.Jul 24 2017, 6:29 AM

hgext3rd/tweakdefaults.py
664	That actually makes sense, thanks for explanation

wrap cleanupnodes
add tests

Update metadata

ryanmce added a subscriber: ryanmce.Jul 24 2017, 2:44 PM

ryanmce added inline comments.

hgext3rd/tweakdefaults.py
664	I would make this configurable and if it's 0 have no limit. Then nuclide could, for example, get all the output hashes. If it's nonzero, divide the number in half and take the first N/2 and last N/2 hashes produced, if more hashes than N are produced. Output a ... in between if we elide any. The first and last hash are the most important.
677	magic number 50 I don't think the description is too useful either. rebase already prints it out when it says "rebasing...". I think just the mapping is the useful thing here.

This is clearly an improvement even if its verbose in some places. Let's get it in and iterate as needed.

This revision is now accepted and ready to land.Jul 25 2017, 7:43 AM

Closed by commit rFBHGXdec644524c33: tweakdefaults: output new hashes when nodes get updated (authored by • jeroenv). · Explain WhyJul 25 2017, 7:54 AM

This revision was automatically updated to reflect the committed changes.

mitrandir mentioned this in D191: metaedit: switch to scmutil.cleanupnodes for cleanup.Jul 27 2017, 8:51 AM

mitrandir mentioned this in rFBHGX397926ee66b0: metaedit: switch to scmutil.cleanupnodes for cleanup.Jul 31 2017, 1:00 PM

quark mentioned this in D525: rebase: add a changes flag which will tell about the hash changes.Aug 26 2017, 12:14 PM

Revision Contents
Changeset List

			Path	Packages
M			hgext3rd/tweakdefaults.py (30 lines)
M			tests/test-tweakdefaults.t (84 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	377		Jul 21 2017, 1:21 PM	★	★
Diff 2	395	- wrap cleanupnodes	Jul 24 2017, 2:36 PM	★	★
Diff 3	396	Update metadata	Jul 24 2017, 2:38 PM	★	★
Diff 4	400	rFBHGXdec644524c33572de48788a8bb317e52e2190344	Jul 25 2017, 7:50 AM	★	★

Diff 400

hgext3rd/tweakdefaults.py

	bmnodestmsg = ''			bmnodestmsg = ''
	branchmessage = ''			branchmessage = ''
	branchesmessage = ''			branchesmessage = ''
	mergemessage = ''			mergemessage = ''
	nodesthint = ''			nodesthint = ''
	nodestmsg = ''			nodestmsg = ''
	rollbackhint = ''			rollbackhint = ''
	tagsmessage = ''			tagsmessage = ''

				# output new hashes when nodes get updated
				showupdated = False
	"""			"""

	from mercurial.i18n import _			from mercurial.i18n import _
				from mercurial.node import (
				short,
				)
	from mercurial import (			from mercurial import (
	bookmarks,			bookmarks,
	cmdutil,			cmdutil,
	commands,			commands,
	error,			error,
	encoding,			encoding,
	extensions,			extensions,
	hg,			hg,
	wrapblame()			wrapblame()

	entry = wrapcommand(commands.table, 'commit', commitcmd)			entry = wrapcommand(commands.table, 'commit', commitcmd)
	options = entry[1]			options = entry[1]
	options.insert(9, ('M', 'reuse-message', '',			options.insert(9, ('M', 'reuse-message', '',
	_('reuse commit message from REV'), _('REV')))			_('reuse commit message from REV'), _('REV')))
	opawarerebase = markermetadatawritingcommand(ui, _rebase, 'rebase')			opawarerebase = markermetadatawritingcommand(ui, _rebase, 'rebase')
	wrapcommand(rebase.cmdtable, 'rebase', opawarerebase)			wrapcommand(rebase.cmdtable, 'rebase', opawarerebase)
				wrapfunction(scmutil, 'cleanupnodes', cleanupnodeswrapper)
	entry = wrapcommand(commands.table, 'pull', pull)			entry = wrapcommand(commands.table, 'pull', pull)
	options = entry[1]			options = entry[1]
	options.append(			options.append(
	('d', 'dest', '', _('destination for rebase or update')))			('d', 'dest', '', _('destination for rebase or update')))

	try:			try:
	rebaseext = extensions.find('rebase')			rebaseext = extensions.find('rebase')
	# tweakdefaults is already loaded before other extensions			# tweakdefaults is already loaded before other extensions
	result = hg.update(repo, dest.node())			result = hg.update(repo, dest.node())
	if bmactive(repo):			if bmactive(repo):
	with repo.wlock():			with repo.wlock():
	bookmarks.update(repo, [prev.node()], dest.node())			bookmarks.update(repo, [prev.node()], dest.node())
	return result			return result

	return orig(ui, repo, **opts)			return orig(ui, repo, **opts)

				def cleanupnodeswrapper(orig, repo, mapping, operation):
				if repo.ui.configbool('tweakdefaults', 'showupdated', False):
				maxoutput = 10
				oldnodes = sorted(mapping.keys())
				for i in range(0, min(len(oldnodes), maxoutput)):
				oldnode = oldnodes[i]
				newnodes = mapping[oldnode]
				quarkUnsubmitted Not Done Is the limit useful? I think the source revisions are usually limited. quark: Is the limit useful? I think the source revisions are usually limited.
				stashUnsubmitted Not Done If you get rid of the `maxoutput` then loop can be simplified to for key, value in rebaseruntime.state.iteritems(): stash: If you get rid of the `maxoutput` then loop can be simplified to ``` for key, value in…
				jeroenvAuthorUnsubmitted Not Done We do the same thing in `pushrebase`. Source can be an arbitrary revset right? For example someone with a lot of heads rebasing everything on top of master. More than happy to get rid of this. cc @durham who advised. jeroenv: We do the same thing in `pushrebase`. Source can be an arbitrary revset right? For example…
				stashUnsubmitted Not Done That actually makes sense, thanks for explanation stash: That actually makes sense, thanks for explanation
				ryanmceUnsubmitted Not Done I would make this configurable and if it's 0 have no limit. Then nuclide could, for example, get all the output hashes. If it's nonzero, divide the number in half and take the first N/2 and last N/2 hashes produced, if more hashes than N are produced. Output a ... in between if we elide any. The first and last hash are the most important. ryanmce: I would make this configurable and if it's 0 have no limit. Then nuclide could, for example…
				_printupdatednode(repo, oldnode, newnodes)
				if len(oldnodes) > maxoutput + 1:
				repo.ui.status(_("...\n"))
				lastoldnode = oldnodes[-1]
				lastnewnodes = mapping[lastoldnode]
				_printupdatednode(repo, lastoldnode, lastnewnodes)
				return orig(repo, mapping, operation)

				def _printupdatednode(repo, oldnode, newnodes):
				# oldnode was not updated if newnodes is an iterable
				if len(newnodes) == 1:
				newnode = newnodes[0]
				firstline = repo[newnode].description().split("\n")[0][:50]
				ryanmceUnsubmitted Not Done magic number 50 I don't think the description is too useful either. rebase already prints it out when it says "rebasing...". I think just the mapping is the useful thing here. ryanmce: magic number 50 I don't think the description is too useful either. rebase already prints it…
				repo.ui.status(_("%s -> %s \"%s\"\n") % (
				short(oldnode), short(newnode), firstline))

	def _computeobsoletenotrebasedwrapper(orig, repo, rebaseobsrevs, dest):			def _computeobsoletenotrebasedwrapper(orig, repo, rebaseobsrevs, dest):
				quarkUnsubmitted Not Done Maybe `[:10]` can be removed since we have `short` (equivalent to `hex(node)[:12]`) already. quark: Maybe `[:10]` can be removed since we have `short` (equivalent to `hex(node)[:12]`) already.
				jeroenvAuthorUnsubmitted Not Done This is to shave another 2 characters off to limit output, but I agree that maybe it's a little silly. jeroenv: This is to shave another 2 characters off to limit output, but I agree that maybe it's a little…
	"""Wrapper for _computeobsoletenotrebased from rebase extensions			"""Wrapper for _computeobsoletenotrebased from rebase extensions

	Unlike upstream rebase, we don't want to skip purely pruned commits.			Unlike upstream rebase, we don't want to skip purely pruned commits.
	We also want to explain why some particular commit was skipped."""			We also want to explain why some particular commit was skipped."""
	res = orig(repo, rebaseobsrevs, dest)			res = orig(repo, rebaseobsrevs, dest)
	for key in res.keys():			for key in res.keys():
	if res[key] is None:			if res[key] is None:
	# key => None is a sign of a pruned commit			# key => None is a sign of a pruned commit

tests/test-tweakdefaults.t

	diff -r 1e4be0697311 -r d17770b7624d b			diff -r 1e4be0697311 -r d17770b7624d b
	--- /dev/null Thu Jan 01 00:00:00 1970 +0000			--- /dev/null Thu Jan 01 00:00:00 1970 +0000
	+++ b/b Thu Jan 01 00:00:00 1970 +0000			+++ b/b Thu Jan 01 00:00:00 1970 +0000
	@@ -0,0 +1,1 @@			@@ -0,0 +1,1 @@
	+b			+b
	$ hg diff -r ".^" -r . --per-file-stat-json			$ hg diff -r ".^" -r . --per-file-stat-json
	{"a": {"adds": 1, "isbinary": false, "removes": 0}, "b": {"adds": 1, "isbinary": false, "removes": 0}}			{"a": {"adds": 1, "isbinary": false, "removes": 0}, "b": {"adds": 1, "isbinary": false, "removes": 0}}

				Test rebase with showupdated=True
				$ cd $TESTTMP
				$ hg init showupdated
				$ cd showupdated
				$ cat >> .hg/hgrc <<EOF
				> [tweakdefaults]
				> showupdated=True
				> rebasekeepdate=True
				> EOF
				$ touch a && hg commit -Aqm a
				$ touch b && hg commit -Aqm b
				$ hg up -q 0
				$ touch c && hg commit -Aqm c
				$ hg log -G -T '{node} {rev} {bookmarks}' -r 'all()'
				@ d5e255ef74f8ec83b3a2a3f3254c699451c89e29 2
				\|
				\| o 0e067c57feba1a5694ca4844f05588bb1bf82342 1
				\|/
				o 3903775176ed42b1458a6281db4a0ccf4d9f287a 0

				$ hg rebase -r 1 -d 2
				rebasing 1:0e067c57feba "b"
				0e067c57feba -> a602e0d56f83 "b"
				saved backup bundle to $TESTTMP/showupdated/.hg/strip-backup/0e067c57feba-ca6d05e3-rebase.hg (glob)

				Test rebase with showupdate=True and a lot of source revisions
				$ hg up -q 0
				$ for i in `$TESTDIR/seq.py 11`; do touch "$i" && hg commit -Aqm "$i" && hg up -q 0; done
				$ hg log -G -T '{node} {rev} {bookmarks}' -r 'all()'
				o 6e3ddf6f49efd0a836a470a2d45e953db915c262 13
				\|
				\| o e02ec9861284762c93b8a4c9e0bac0abfbb59ac7 12
				\|/
				\| o 14218977adef919e86466c688defa3fb893a5638 11
				\|/
				\| o 6a01a2bb0a9f7a6f01c6f49ce90e60bf85de79d0 10
				\|/
				\| o e5ec40f709911f69eafada5746d3f5b969005738 9
				\|/
				\| o 73800d52e8ddab37a1f9177299d1fdfb563c061a 8
				\|/
				\| o 657f1516f142d51ba06b98413ce35a884f7f8af0 7
				\|/
				\| o 4e6ba707bdb81e60d96b84099c2d1c56530ee6f1 6
				\|/
				\| o 7ab24e484dafd1d2ebf51a4fa4523431b81b99aa 5
				\|/
				\| o ee71024c6e8c4a45ee1d3e462431bfec85ac215a 4
				\|/
				\| o 46a418a0abd225d8ad876102f495d209907b79d9 3
				\|/
				\| o a602e0d56f83e5816ebcbb78095e259ffbce94aa 2
				\| \|
				\| o d5e255ef74f8ec83b3a2a3f3254c699451c89e29 1
				\|/
				@ 3903775176ed42b1458a6281db4a0ccf4d9f287a 0

				$ hg rebase -r 'all() - 0 - 12' -d 12
				rebasing 1:d5e255ef74f8 "c"
				rebasing 2:a602e0d56f83 "b"
				rebasing 3:46a418a0abd2 "1"
				rebasing 4:ee71024c6e8c "2"
				rebasing 5:7ab24e484daf "3"
				rebasing 6:4e6ba707bdb8 "4"
				rebasing 7:657f1516f142 "5"
				rebasing 8:73800d52e8dd "6"
				rebasing 9:e5ec40f70991 "7"
				rebasing 10:6a01a2bb0a9f "8"
				rebasing 11:14218977adef "9"
				rebasing 13:6e3ddf6f49ef "11" (tip)
				14218977adef -> 2d12dd93bf8b "9"
				46a418a0abd2 -> 645dc4557ba8 "1"
				4e6ba707bdb8 -> b9598afdff23 "4"
				657f1516f142 -> 906a55b270d4 "5"
				6a01a2bb0a9f -> f5a7b375c4a9 "8"
				6e3ddf6f49ef -> ad74bbdbdb75 "11"
				73800d52e8dd -> 3145d47a5692 "6"
				7ab24e484daf -> 03a9e0e4badc "3"
				a602e0d56f83 -> e7a99f1fea2a "b"
				d5e255ef74f8 -> 72d850a207d5 "c"
				...
				ee71024c6e8c -> 0c42bb4bf23f "2"
				saved backup bundle to $TESTTMP/showupdated/.hg/strip-backup/6e3ddf6f49ef-4b18babd-rebase.hg (glob)

	Test bookmark -D			Test bookmark -D
	$ cd $TESTTMP			$ cd $TESTTMP
	$ hg init book-D			$ hg init book-D
	$ cd book-D			$ cd book-D
	$ cat >> .hg/hgrc <<EOF			$ cat >> .hg/hgrc <<EOF
	> [extensions]			> [extensions]
	> strip=			> strip=
	> [experimental]			> [experimental]