gca refactoring
This MR decouples the content-divergence resolution logic from gca which was unnecessarily populated here, as the same goal could be achieved by a simple 3-way comparison.
Merge request reports
Activity
assigned to @marmoute
By Sushil Khanchi on 2021-01-08T11:08:18 (imported from GitLab)
@marmoute This new MR is the one that goes with what you planned in the email. i.e. a new MR which removes
gca
from the main logic and keep it only in the special case when both csets moved.By Sushil Khanchi on 2021-01-08T11:14:10 (imported from GitLab)
- Resolved by Raphaël Gomès
added 1 commit
- 0f301a96e459 - evolve: separate the gca-logic from main resolution part
By Sushil Khanchi on 2021-01-10T06:25:06 (imported from GitLab)
added 13 commits
-
0f301a96...5cbf9c21 - 9 commits from branch
branch/default
- b6fc7dfa9386 - evolve: no need to keep it as a smartset as it's a single value
- d4a7d6807675 - evolve: remove unnecessary condition
- cb5272c7cbde - evolve: return rev from _pickresolutionparent()
- 3331219064e3 - evolve: separate the gca-logic from main resolution part
By Pierre-Yves David on 2021-01-16T02:43:31 (imported from GitLab)
Toggle commit list-
0f301a96...5cbf9c21 - 9 commits from branch
- Resolved by Raphaël Gomès
evolve: no need to keep it as a smartset as it's a single value
Note that gca can be multiple revisions, this happens during criss cross merge. In practice after we reduces GCA usage to a single changesets, we mostly want to know if
X in parents(ancestors(Y) - Y)
and we use that :-Devolve: separate the gca-logic from main resolution part
oof, the test change are quite brutal. I'll need a bit more time to review them. Could we have the refactoring without the logic change? It would make me more more comfortable.
By Pierre-Yves David on 2021-01-18T08:59:16 (imported from GitLab)
added 2 commits
- fd7dfac95d94 - evolve: return rev from _pickresolutionparent()
- 1f911f4d32dd - evolve: separate the gca-logic from main resolution part
By Sushil Khanchi on 2021-01-18T04:49:10 (imported from GitLab)
added 15 commits
-
1f911f4d...9f8ff149 - 4 commits from branch
branch/default
- 6dc54f1537f7 - divergence-resolution: return rev from _pickresolutionparent()
- 25dc8faeb799 - divergence-resolution: pre-indent some code
- 2131d71c92ea - divergence-resolution: pass the base changeset to _pickresolutionparent
- f7cf0182c37a - divergence-resolution: clarify some test case
- f6780c6f16bb - divergence-resolution: remove explicite hash in some command
- 0d87cfb8a228 - I HAVE NO IDEA WHAT THIS CHANGE IS ABOUT
- 264c427587b9 - divergence-resolution: deal with simple "just one moved case" earlier
- 63a2906610b8 - test change ?
- 17324f9d3c5f - actual test change
- 35faf0c6cdcf - FOLLOW SUCCESSORS, FIX ME ?
- e7ee467b2ed9 - FIX ME: evolve: separate the gca-logic from main resolution part
By Pierre-Yves David on 2021-01-22T04:28:02 (imported from GitLab)
Toggle commit list-
1f911f4d...9f8ff149 - 4 commits from branch
added 5 commits
- bb883c7e278e - divergence-resolution: deal with simple "just one moved case" earlier
- b57dfae894af - I HAVE NO IDEA WHAT THIS CHANGE IS ABOUT
- 57351613fb5e - actual test change
- 1ec396232d64 - FOLLOW SUCCESSORS, FIX ME ?
- 53a74e246ae5 - FIX ME: evolve: separate the gca-logic from main resolution part
By Pierre-Yves David on 2021-01-22T04:41:11 (imported from GitLab)
Toggle commit list- Resolved by Raphaël Gomès
I started doing work for splitting this is more diguestable bits.
I suspect everything up to:
divergence-resolution: deal with simple "just one moved case" earlier
Is in a good enough shape to get in. @khanchi97 can you double check them ?
The rest of the series is a bit muckier. I initially had no idea what the extra change to
test-evolve-content-divergent-interrupted.t
were about. However I later got a better grasp on them. It seems that the order change mean we no longer have a conflict and that the test is about testing conflict. We probably need more explanation about that part.In addition the change around
elif succsbasep1 in (divp1, otherp1):
is a bit weird. So we should probably clarify it before going further in the test modificiation.I understand the logic but:
- why do we only do it the base ?
- shouldn't we just consider "single-successors" for everything anyway ? for the first logic ?
If in this code:
elif basep1 in (divp1, otherp1): # only one side moved, set parent of moved one as resolution parent if basep1 == divp1: resolutionparent = succsotherp1 else: pass
We would replace all of
basep1
,divp1
andotherp1
by their "single-successors", we would have the upgraded logic that we are trying to do with that change, don't we ?By Pierre-Yves David on 2021-01-23T04:37:01 (imported from GitLab)
assigned to @khanchi97 and unassigned @marmoute
By Pierre-Yves David on 2021-01-22T09:50:00 (imported from GitLab)
added 5 commits
- 92770d98e18d - divergence-resolution: deal with simple "just one moved case" earlier
- 5f590fe0a73e - I HAVE NO IDEA WHAT THIS CHANGE IS ABOUT
- c8cfe2f53f49 - actual test change
- a34ea78f5c21 - FOLLOW SUCCESSORS, FIX ME ?
- 21b470b10e6a - FIX ME: evolve: separate the gca-logic from main resolution part
By Sushil Khanchi on 2021-01-23T04:36:30 (imported from GitLab)
Toggle commit listadded 2 commits
- 6d93922432ff - FOLLOW SUCCESSORS, FIX ME ?
- 5f65b3dd3cb4 - FIX ME: evolve: separate the gca-logic from main resolution part
By Sushil Khanchi on 2021-01-23T04:38:23 (imported from GitLab)
@marmoute you can publish up to the revision
divergence-resolution: deal with simple "just one moved case" earlier
I am looking into the remaining commits to fix them accordingly.
By Sushil Khanchi on 2021-01-23T04:40:37 (imported from GitLab)
added 9 commits
- deff2e70dd19 - divergence-resolution: return rev from _pickresolutionparent()
- 551de82cbc53 - divergence-resolution: pre-indent some code
- 8705ccb23fc8 - divergence-resolution: pass the base changeset to _pickresolutionparent
- da73aa37f739 - divergence-resolution: clarify some test case
- d32f8912061d - divergence-resolution: remove explicite hash in some command
- 7a91d36ba310 - divergence-resolution: deal with simple "just one moved case" earlier
- 70f443559c50 - divergence-resolution: [patch 1] fix a messed up test case
- 522247b4d68f - divergence-resolution: [patch 2] fix a messed up test case
- 846b3c8f1782 - divergence-resolution: [patch 3] fix a messed up test case
By Sushil Khanchi on 2021-01-24T18:04:03 (imported from GitLab)
Toggle commit list- Resolved by Raphaël Gomès
assigned to @marmoute and unassigned @khanchi97
By Sushil Khanchi on 2021-01-25T03:39:27 (imported from GitLab)
added 30 commits
-
846b3c8f...61d2f1cf - 20 commits from branch
branch/default
- e6b0ed34bc18 - divergence-resolution: return rev from _pickresolutionparent()
- 1319abf8d8c4 - divergence-resolution: pre-indent some code
- bc323205241b - divergence-resolution: pass the base changeset to _pickresolutionparent
- 6dbfd71cdcba - divergence-resolution: clarify some test case
- 3a1e07c27e4a - divergence-resolution: remove explicite hash in some command
- a119432bafc1 - divergence-resolution: deal with simple "just one moved case" earlier
- 04095b70bee5 - divergence-resolution: take parent successors in account
- caee3020a5c5 - divergence-resolution: [patch 1] fix a messed up test case
- 0d3c452e8a9d - divergence-resolution: [patch 2] fix a messed up test case
- 9decfbec933e - divergence-resolution: [patch 3] fix a messed up test case
By Pierre-Yves David on 2021-01-25T13:30:10 (imported from GitLab)
Toggle commit list-
846b3c8f...61d2f1cf - 20 commits from branch