This is an archive of the discontinued Mercurial Phabricator instance.

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.

This revision now requires changes to proceed.May 28 2021, 9:10 AM

In 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'>

In D10770#164628, @martinvonz wrote:
In 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 isn't correct and Python behavior is… creative. Maybe we should build a pycompat wrapper for struct.format access ?

In D10770#164629, @marmoute wrote:
In D10770#164628, @martinvonz wrote:
In 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 ?

In D10770#164631, @martinvonz wrote:
In D10770#164629, @marmoute wrote:
In D10770#164628, @martinvonz wrote:
In 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 ?

I see you are abandoning this revision, can you share details about your new plan ?

In D10770#164629, @marmoute wrote:
In D10770#164628, @martinvonz wrote:
In 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.

A pycompat wrapper would make sense for this if we see more than one case of this. Is that likely?

I see you are abandoning this revision, can you share details about your new plan ?

I believe he found the interaction too frustrating and is dealing with other things.

martinvonz added a commit: rHG710435a5c469: docket: make compatible with py3.6, where Struct.format is bytes.Jun 1 2021, 10:07 AM

In D10770#165221, @durin42 wrote:
In D10770#164629, @marmoute wrote:
In D10770#164628, @martinvonz wrote:
In 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.

In D10770#165223, @marmoute wrote:
In D10770#165221, @durin42 wrote:
In D10770#164629, @marmoute wrote:
In D10770#164628, @martinvonz wrote:
In 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.

Yeah, it feels a bit wrong to me too, but I don't see any great options, esp. w/ the pycompat string tool already doing what we need.

Your solution feels fine-ish, with the caveat that it's more prone to falling out of sync (though I suppose tests would catch that very quickly). I queued this, but if you like the two-constant approach send that as a follow-up?

I will follow the guide here and make docket compatible with py3.6. This is what I want, and I can focus on my australian writings work once this is done. That will make me happy!

Thanks for sharing this article.

Awesome and interesting article. Great things you've always shared with us. Thanks. Just continue composing this kind of post. automatic reply text message

Revision Contents
Changeset List

			Path	Packages
M			mercurial/revlogutils/docket.py (4 lines)

Commit	Parents	Author	Summary	Date
d62b3069b2fd	825d5a5907b4	Martin von Zweigbergk		May 25 2021, 7:46 PM

Diff 28203

mercurial/revlogutils/docket.py

	# * 4 bytes: revlog version			# * 4 bytes: revlog version
	# \| This is mandatory as docket must be compatible with the previous			# \| This is mandatory as docket must be compatible with the previous
	# \| revlog index header.			# \| revlog index header.
	# * 8 bytes: size of index-data			# * 8 bytes: size of index-data
	# * 8 bytes: pending size of index-data			# * 8 bytes: pending size of index-data
	# * 8 bytes: size of data			# * 8 bytes: size of data
	# * 8 bytes: pending size of data			# * 8 bytes: pending size of data
	# * 1 bytes: default compression header			# * 1 bytes: default compression header
	S_HEADER = struct.Struct(constants.INDEX_HEADER.format + 'LLLLc')			S_HEADER = struct.Struct(
				pycompat.sysstr(constants.INDEX_HEADER.format) + 'LLLLc'
				)


	class RevlogDocket(object):			class RevlogDocket(object):
	"""metadata associated with revlog"""			"""metadata associated with revlog"""

	def __init__(			def __init__(
	self,			self,
	revlog,			revlog,

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	28203			May 25 2021, 7:55 PM	★	★

This is an archive of the discontinued Mercurial Phabricator instance.

docket: make compatible with py3.6, where Struct.format is bytesAbandonedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 28203

mercurial/revlogutils/docket.py

docket: make compatible with py3.6, where Struct.format is bytes
AbandonedPublic

Revision Contents
Changeset List