Re: [PATCH 5 of 5] rust: update to toml 0.8
On 12/01/2024 00.59, Mads Kiilerich wrote: # HG changeset patch # User Mads Kiilerich # Date 1704999033 -3600 # Thu Jan 11 19:50:33 2024 +0100 # Branch stable # Node ID 646c00200241229c23dad41b1d086a610aff08b2 # Parent f2e61759ac0e0125e275acc72bda8a00258762b9 rust: update to toml 0.8 This is needed for Fedora packaging. diff --git a/rust/hg-core/Cargo.toml b/rust/hg-core/Cargo.toml --- a/rust/hg-core/Cargo.toml +++ b/rust/hg-core/Cargo.toml @@ -31,7 +31,7 @@ sha-1 = "0.10.0" twox-hash = "1.6.3" same-file = "1.0.6" tempfile = "3.3.0" -toml = "0.6" +toml = "0.8" thread_local = "1.1.4" crossbeam-channel = "0.5.6" log = "0.4.17" ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel Doesn’t rust/Cargo.lock need to be updated? ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 3 of 5] python3.13: address deprecation of re.sub positional argument 'count'
On 12/01/2024 00.59, Mads Kiilerich wrote: # HG changeset patch # User Mads Kiilerich # Date 1705006735 -3600 # Thu Jan 11 21:58:55 2024 +0100 # Branch stable # Node ID 8e16bc622b04e2eabb3a47138aa3bdffba03e142 # Parent a06a7677696d8fa4fc3e33923425ef3fadd6f441 python3.13: address deprecation of re.sub positional argument 'count' Python 3.13 introduced: DeprecationWarning: 'count' is passed as positional argument Making it mandatory to pass 'count' as named argument reduces the risk of passing 'flags' to it. That is exactly what happened in test-doctest.py . diff --git a/mercurial/subrepoutil.py b/mercurial/subrepoutil.py --- a/mercurial/subrepoutil.py +++ b/mercurial/subrepoutil.py @@ -112,7 +112,7 @@ def state(ctx, ui): # extra escapes are needed because re.sub string decodes. repl = re.sub(br'([0-9]+)', br'\\\1', repl) try: -src = re.sub(pattern, repl, src, 1) +src = re.sub(pattern, repl, src, count=1) except re.error as e: raise error.Abort( _(b"bad subrepository pattern in %s: %s") diff --git a/tests/test-doctest.py b/tests/test-doctest.py --- a/tests/test-doctest.py +++ b/tests/test-doctest.py @@ -21,9 +21,9 @@ class py3docchecker(doctest.OutputChecke r'''^mercurial\.\w+\.(\w+): (['"])(.*?)\2''', r'\1: \3', got2, -re.MULTILINE, +flags=re.MULTILINE, ) -got2 = re.sub(r'^mercurial\.\w+\.(\w+): ', r'\1: ', got2, re.MULTILINE) +got2 = re.sub(r'^mercurial\.\w+\.(\w+): ', r'\1: ', got2, flags=re.MULTILINE) return any( doctest.OutputChecker.check_output(self, w, g, optionflags) for w, g in [(want, got), (want2, got2)] ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel LGTM! ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 2 of 5] python3.13: fix resourceutil for removed deprecated importlib.resources
On 12/01/2024 00.59, Mads Kiilerich wrote: # HG changeset patch # User Mads Kiilerich # Date 1705001574 -3600 # Thu Jan 11 20:32:54 2024 +0100 # Branch stable # Node ID a06a7677696d8fa4fc3e33923425ef3fadd6f441 # Parent ab3021e9b0012db64e5bdc70e3f5a36324925d8c python3.13: fix resourceutil for removed deprecated importlib.resources The old functionality was deprecated in 3.11 and is on track to be removed in 3.13 . The documentation on https://docs.python.org/3.12/library/importlib.resources.html recommends using the new .files() that was introduced in 3.9. The pytype annotation is probably not preserved correctly. Which annotation do you mean exactly? If you mean “# pytype: disable=module-attr”, let’s see whether CI fails without it. Otherwise it would probably need to be on the same line as “resources.files”. diff --git a/mercurial/utils/resourceutil.py b/mercurial/utils/resourceutil.py --- a/mercurial/utils/resourceutil.py +++ b/mercurial/utils/resourceutil.py @@ -60,8 +60,10 @@ try: # Force loading of the resources module if hasattr(resources, 'files'): +# New in Python 3.9 resources.files # pytype: disable=module-attr else: +# Deprecated in Python 3.11 resources.open_binary # pytype: disable=module-attr # py2exe raises an AssertionError if uses importlib.resources @@ -109,12 +111,20 @@ else: ) def is_resource(package, name): -return resources.is_resource( # pytype: disable=module-attr -pycompat.sysstr(package), encoding.strfromlocal(name) -) +if hasattr(resources, 'files'): +return resources.files(pycompat.sysstr(package)).joinpath(encoding.strfromlocal(name)).is_file() +else: +return resources.is_resource( # pytype: disable=module-attr +pycompat.sysstr(package), encoding.strfromlocal(name) +) def contents(package): # pytype: disable=module-attr -for r in resources.contents(pycompat.sysstr(package)): -# pytype: enable=module-attr -yield encoding.strtolocal(r) +if hasattr(resources, 'files'): +for resource in resources.files(pycompat.sysstr(package)).iterdir(): +if resource.is_file(): +yield encoding.strtolocal(path.name) I think you meant “resource” instead of “path”. +else: +for r in resources.contents(pycompat.sysstr(package)): +# pytype: enable=module-attr +yield encoding.strtolocal(r) ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel Otherwise it LGTM. ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 5] python3.13: use sys.executable instead of removed Py_GetProgramFullPath
On 12/01/2024 00.59, Mads Kiilerich wrote: # HG changeset patch # User Mads Kiilerich # Date 1705001527 -3600 # Thu Jan 11 20:32:07 2024 +0100 # Branch stable # Node ID ab3021e9b0012db64e5bdc70e3f5a36324925d8c # Parent 3f87e0d305cda6e66139a1969cd2cedd45477139 python3.13: use sys.executable instead of removed Py_GetProgramFullPath According to https://docs.python.org/3.13/c-api/init.html#c.Py_GetProgramFullPath, it’s only deprecated, but not removed. Do you have other information? I could not make it work with the PyConfig API from the extension. But fetching sys.executable seems to work fine and isn't that verbose. I think the PyConfig struct memory is released after interpreter initialization, so I think PyConfig is out of question anyway. https://docs.python.org/3.13/c-api/init.html#c.Py_GetProgramFullPath suggests sys.executable as the alternative, so feel to change the patch description to be more bold. :) diff --git a/mercurial/cext/parsers.c b/mercurial/cext/parsers.c --- a/mercurial/cext/parsers.c +++ b/mercurial/cext/parsers.c @@ -1232,6 +1232,15 @@ static int check_python_version(void) * should only occur in unusual circumstances (e.g. if sys.hexversion * is manually set to an invalid value). */ if ((hexversion == -1) || (hexversion >> 16 != PY_VERSION_HEX >> 16)) { + PyObject *sys = PyImport_ImportModule("sys"), *executable; + if (!sys) { + return -1; + } + executable = PyObject_GetAttrString(sys, "executable"); You could use PySys_GetObject(). + Py_DECREF(sys); + if (!executable) { + return -1; + } PyErr_Format(PyExc_ImportError, "%s: The Mercurial extension " "modules were compiled with Python " PY_VERSION @@ -1240,7 +1249,8 @@ static int check_python_version(void) "sys.hexversion=%ld: " "Python %s\n at: %s", versionerrortext, hexversion, Py_GetVersion(), -Py_GetProgramFullPath()); +PyUnicode_AsUTF8(executable)); + Py_DECREF(executable); return -1; } return 0; ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Accidental modification of mercurial.changelog._defaultextra
On 01/02/2024 13.56, Pierre-Yves David wrote: On 2/1/24 10:08, Manuel Jacob wrote: 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? I think this is a pretty good catch, thanks you. I don't believe this is reasonable to assume or enforce that caller will not modify the returned dictionnary, and we should return a new dictionnary in all cases. I don't expect any significant performance impact from this. Would you send a patch in that direction ? https://foss.heptapod.net/mercurial/mercurial-devel/-/merge_requests/758 I added the patch against the default branch to ensure compatibility with extensions that depend on the buggy behavior. For example, evolve requires fixes like that: https://foss.heptapod.net/mercurial/evolve/-/merge_requests/554 ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: Accidental modification of mercurial.changelog._defaultextra
On 2/1/24 10:08, Manuel Jacob wrote: 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? I think this is a pretty good catch, thanks you. I don't believe this is reasonable to assume or enforce that caller will not modify the returned dictionnary, and we should return a new dictionnary in all cases. I don't expect any significant performance impact from this. Would you send a patch in that direction ? -- Pierre-Yves David ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel
Accidental modification of mercurial.changelog._defaultextra
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? ___ Mercurial-devel mailing list Mercurial-devel@lists.mercurial-scm.org https://lists.mercurial-scm.org/mailman/listinfo/mercurial-devel