Re: [PATCH 5 of 5] rust: update to toml 0.8

2024-02-01 Thread Manuel Jacob

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'

2024-02-01 Thread Manuel Jacob

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

2024-02-01 Thread Manuel Jacob

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

2024-02-01 Thread Manuel Jacob

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

2024-02-01 Thread Manuel Jacob

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

2024-02-01 Thread Pierre-Yves David


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

2024-02-01 Thread Manuel Jacob
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