[Commented On] D8480: bundle: make obsolescence parts optional

durin42 (Augie Fackler) phabricator at mercurial-scm.org
Mon May 18 15:39:56 UTC 2020


durin42 added a comment.


  This patch is contentious, but I'm a little unclear why. Summarizing so everyone at least parses my thoughts roughly the way I mean: mandatory parts mean "fail if you can't process this part." It seems basically reasonable to me that a client can choose to ignore (perhaps through ignorance) obsolete markers and it'll be potentially confusing, but not /wrong/ in that they'll have the changesets they expected, but also didn't lose some the server thought they would. It feels like a bug if a client requests obsmarkers and then discards them? But in the case of any kind of pre-computed bundle it's a little trickier because if the server (as an optimization, in this case) emits obsmarkers that the client didn't request, it could break old clients.
  
  Given that obsmarkers are still off by default and effectively experimental, we can change whatever we want. So the question is: should we, and if so, should it be all obsmarker parts or only precomputed ones? I think it's pretty clear that we should at least allow optional obsmarker parts for precomputed bundles, and I think we also should allow mandatory obsmarker parts (so if you try to push markers to a server that doesn't grok them you get an error instead of ninja-duplicating some changesets with their evolved counterparts.)
  
  So that leaves us with the one remaining question: what should the server do when the client proactively requests markers? I think in that case it basically doesn't matter, so either way is fine. If it's less work, I'm inclined to land the patch as-is, and we can do a follow-up if there's a bug case that I'm not understanding today.
  
  In D8480#127571 <https://phab.mercurial-scm.org/D8480#127571>, @marmoute wrote:
  
  > In D8480#127498 <https://phab.mercurial-scm.org/D8480#127498>, @joerg.sonnenberger wrote:
  >
  >> While I don't really agree with the design interpretation of why the server should send mandatory, I don't care enough in this case.
  >
  > The is no "design interpretation going on here", am I the author of these APIs/protocols and I am clarifying  their semantics. The intend for this part to be mandatory in the context have been here since the begining.
  
  Given that we've got at least two readers that misinterpreted your intent, please send a clarifying patch to the protocol docs (along with rationale, preferably) so the intent is more clear? I think that might also be a good place to discuss what the semantics _should_ be, in case people have feelings that merit further discussion.

REPOSITORY
  rHG Mercurial

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

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

To: joerg.sonnenberger, #hg-reviewers, marmoute
Cc: durin42, marmoute, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200518/6b9e96ca/attachment-0002.html>


More information about the Mercurial-patches mailing list