[Commented On] D10770: docket: make compatible with py3.6, where Struct.format is bytes

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D10770#165221 <https://phab.mercurial-scm.org/D10770#165221>, @durin42 wrote:
  
  > In D10770#164629 <https://phab.mercurial-scm.org/D10770#164629>, @marmoute wrote:
  >
  >> In D10770#164628 <https://phab.mercurial-scm.org/D10770#164628>, @martinvonz wrote:
  >>
  >>> In D10770#164626 <https://phab.mercurial-scm.org/D10770#164626>, @marmoute wrote:
  >>>
  >>>> Looks like this is happening because of an inconsistency betwen the fact we passe a binary 'struct' for INDEX_HEADER and a sysstr for the extra once. I would rather fix this by making them consistent.
  >>>
  >>> I'm not sure I understand. Can you show me how?
  >>>
  >>>   $ python3.6 -c 'import struct; print(type(struct.Struct(b"I").format))'
  >>>   <class 'bytes'>
  >>>   $ python3.6 -c 'import struct; print(type(struct.Struct("I").format))'
  >>>   <class 'bytes'>
  >>>   $ python3.7 -c 'import struct; print(type(struct.Struct(b"I").format))'
  >>>   <class 'str'>
  >>>   $ python3.7 -c 'import struct; print(type(struct.Struct("I").format))'
  >>>   <class 'str'>
  >>
  >> Okay, so my assertion is correct and Python behavior is… creative. Maybe we should build a pycompat wrapper for struct.format access ?
  >
  > AFAICT your assertion was wrong: the `struct` module just isn't preserving the input type at all, and always emits `str` on 3.7+, but does `bytes` on 3.6. So we're not mixing bytes/str input types to struct, but instead struct on 3.6 is ninja-converting back to bytes "for" us.
  
  Yep, this is a typo on my side (typing is hard) (`is right` because `isn't right`)
  
  > A pycompat wrapper would make sense for this if we see more than one case of this. Is that likely?
  
  It feels wrong to me to have this kind of compatibility weirdness crawl into normal code. I though more about it and in this case I think the simplest would probably to have the format for `INDEX_HEADER` in a `INDEX_HEADER _FMT` constant and concatenate with that.

REPOSITORY
  rHG Mercurial

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

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

To: martinvonz, #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/20210601/d9d697d6/attachment-0002.html>