[Commented On] D10917: rewriteutil: check for divergence on unfiltered repo

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D10917#166941 <https://phab.mercurial-scm.org/D10917#166941>, @martinvonz wrote:
  
  > In D10917#166940 <https://phab.mercurial-scm.org/D10917#166940>, @marmoute wrote:
  >
  >> In D10917#166939 <https://phab.mercurial-scm.org/D10917#166939>, @martinvonz wrote:
  >>
  >>> In D10917#166937 <https://phab.mercurial-scm.org/D10917#166937>, @marmoute wrote:
  >>>
  >>>> It does not looks like this is done at the right level. The code responsible for the processors walking should be the one adjusting the filter level.
  >>>
  >>> Which code is that? Do you mean that it should be done at a higher or lower level?
  >>
  >> lower level
  >
  > I still don't understand. Do you mean that `find_new_divergence_from()` should not return filtered nodeids?
  
  No, given the information returned by `find_new_divergence_from()` it seems unavoidable that filtered revision might be returned as the "common-predecessor" value. (sidenote: that the common-precessors should probably be a list, and the return of `find_new_divergence_from` and `_find_new_divergence` should probably be list too).
  
  My point is that we are looking for divergence within the current filter and that we should not pass unfiltered repository to the function that looks for that divergence. Otherwise it might compute things on the wrong set of revisions.
  
  However, function along the chain should be prepared for the "common-predecessor" nodes to be potentially filtered. In this case this means turning this line
  
    return (repo[r], repo[div[0]], repo[div[1]])
  
  to
  
    return (repo[r], repo[div[0]], unfi[div[1]])
  
  (sidenode: the previous `if div` is actually checking `if div is not None`, so we should update it too)

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D10917/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D10917

To: martinvonz, #hg-reviewers, marmoute
Cc: marmoute, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210630/03e0c8cd/attachment-0002.html>