Accidental modification of mercurial.changelog._defaultextra

Manuel Jacob me at manueljacob.de
Thu Feb 1 09:08:06 UTC 2024


When the raw extra field on a changeset is missing, the 
mercurial.changelog.changelogrevision.extra property returns the 
_defaultextra dictionary as-is. Method 
mercurial.context.changectx.extra() returns that property as-is. The 
problem is that if someone changes the return value in this case, all 
later calls to mercurial.context.changectx.extra() on changesets without 
the raw extra field will return a dictionary containing these changes.

To check whether the problem results in bugs in practice, I changed the 
mercurial.changelog.changelogrevision.extra property to return a 
read-only proxy to _defaultextra. Three tests fail because of this: 
https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/2253663

We could change the mercurial.changelog.changelogrevision.extra property 
to always return a new dictionary. This is already the case if a 
changeset has the raw extra field.

If we don’t want to return a copied dictionary when a changeset has no 
raw extra field, we would need to fix all callers that modify the 
dictionary to copy it first instead of modifying the returned 
dictionary. In the cases when the returned dictionary is already a 
copied dictionary (i.e. if the changeset has the raw extra field), it 
would be redundant.

If we make it part of the contract that the returned extra dictionary 
should not be modified, should that contract be enforced somehow? We 
could return types.MappingProxyType (1) never, (2) always or (3) only 
when _defaultextra is returned.

What do you think?


More information about the Mercurial-devel mailing list