On 12/06/2016 05:41 PM, Mateusz Kwapich wrote:
# HG changeset patch
# User Mateusz Kwapich <mitran...@fb.com>
# Date 1481028829 28800
# Tue Dec 06 04:53:49 2016 -0800
# Branch stable
# Node ID 78b75ed14103cee05ed13948025310919adde559
# Parent 727c7211c810d304ebf92b32db7ecf697ce46ac6
metaedit: add a helper function for just metadata rewrites
It will be used by metaedit.
I've eventually came to review this. Sorry for the delay.
The overall approach seems right, and I'm happy to see the "multiple
revs" case implemented. However, I'm not thrilled with the amount of
code duplicated. I think we can share more codepath here. I was not too
sure of where to explain how, so you get a wall of text in patch 1.
Happy reading.
About rewrite
-------------
The very venerable 'rewrite' function have been in this repository since
september 19th 2011. And was actually written by Peter Arrenbrecht in
spring 2011 (I just imported it in). It was originally written to handle
'amend'. But is no longer used for this purpose.
Its signature is a bit strange.
def rewrite(repo, old, updates, head, newbases, commitopts):
If will effectively fold "old + updates" and "rebase" the result on
"newbases" without actually merging anything. so "updates" MUST be
linear direct descendant of "old", and "newbases" must have the same
manifest content as the old's parents.
"head" seems to just be 'heads(old + updates)'
The meta-data of "old" are reused for the resulting commit and mixed
with "commitopts"
It looks like it could use some cleanup if you feel brave enough.
About metarewrite vs rewrite
--------------------------
metarewrite seems to be doing the same things as rewrite but:
1) For only a single changesets
2) In the case we know we can reuse the manifest revision (and so we do).
We can probably detect that directly within 'rewrite' and take a fast
path in that case.
* We can detect (1) if 'updates' is empty (and therefore head is old),
* we check if the manifest is reusable by checking if the manifest
nodeid for 'old.p1()' and 'old.p2()' match the one in 'newbases'. If we
detect these two conditions, we can skip the expensive 'files'
computation in rewrite ctx, and just call memlightctx to create the
changeset.
These two checkes + small amount of conditional branching seems small
enough to me that I don't think we should duplicate the code and just
have rewrite be a bit smarter. This would avoid the two code bases to
slowly diverge for no good reason.
In addition that would benefit other command that use 'rewrite' in
compatible situation (eg: fold).
What do you think ?
About the core fold code
------------------------
The same apply to the code duplication in the body of the metaedit
function. We get two entirely new "codebase" for each code path (fold
and not fold) while they still have a lot in common. This lead to slow
but dangerous code drift. We already see it in your series where fold is
properly handling phase preservation while not-fold lost it.
So in my opinion we could have a single code base using the fact that:
* Having only 1 folding action can still be expressed in a list of actions
* Not folding changesets together can be expressed with folding a set of
only one changeset.
At that point, I'll move the rest into the reply to patch 3. for clarity.
Cheers
diff --git a/hgext/evolve.py b/hgext/evolve.py
--- a/hgext/evolve.py
+++ b/hgext/evolve.py
@@ -907,6 +907,13 @@ def rewrite(repo, old, updates, head, ne
finally:
lockmod.release(tr, lock, wlock)
+def metarewrite(repo, old, newbases, commitopts):
+ '''Like rewrite but affects only the changeset metadata.'''
+ # TODO: reuse the manifest for speed
+ newid, created = rewrite(repo, old, [old], old, newbases,
+ commitopts=commitopts)
+ return newid, created
+
class MergeFailure(error.Abort):
pass
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
--
Pierre-Yves David
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
