Pierre-Yves David <pierre-yves.david@octobus.net> [Tue, 08 Oct 2024 15:54:59 +0200] rev 51957
module-policy: ignore empty module policy
This make the variable easier to work with, the empty value is not ambiguous
about not wanting to get in the way.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 04 Oct 2024 13:26:29 -0400] rev 51956
tests: provide an alternate fake lock for filesystems without symlink support
Matt Harbison <matt_harbison@yahoo.com> [Fri, 04 Oct 2024 12:53:02 -0400] rev 51955
tests: disable `worker.backgroundclose` to stabilize a test on Windows
TIL that `worker.enabled=0` doesn't prevent these workers from spinning up. At
any rate, there's already a whole lot of conditionalized output following
`cat client.log`, the placement of the "starting 4 threads for background file
closing" message seems unstable, and we don't care about those worker threads
here. Preventing the message is better for test maintenance.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 04 Oct 2024 11:22:30 -0400] rev 51954
tests: fix lock file path mangling in `test-racy-mutations.t` on Windows
I guess `$TESTTMP_FORWARD_SLASH` gets translated by MSYS. This was in the
`.foo_commit_out` file:
sh: C;C:\\MinGW\\msys\\1.0\\Users\\Matt\\AppData\\Local\\Temp\\hgtests.1qc8jmdl\\child2\\test-racy-mutations.t-skip-detection\\waitlock_editor.sh: $ENOENT
Matt Harbison <matt_harbison@yahoo.com> [Fri, 04 Oct 2024 11:10:45 -0400] rev 51953
tests: stabilize `test-status-eacces.t` on Windows
As noted earlier, `chmod` doesn't complain in MSYS, but also doesn't alter the
file permissions such that they are unreadable.
I'm guessing the other lines of output in this area that are gated on `rhg` (or
not) will also need this, but I don't want to dig too deeply into something that
is apparently working well enough.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 04 Oct 2024 01:40:35 -0400] rev 51952
run-tests: bump the default timeout on Windows to 4x the normal value
There are a ridiculous number of tests that timeout on Windows with the 360 sec
default (~60). And because of the bug where timed out tests still run to
completion before the results are thrown away[1], the timeout does nothing but
waste time, so there's no reason to try to find a lower value that still works.
For reference on my system:
# Ran 909 tests, 116 skipped, 119 failed.
python hash seed:
2052473208
real 151m44.322s
user 0m0.077s
sys 0m0.046s
[1] I thought that I wrote a bug for this, but search isn't finding it.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 04 Oct 2024 01:29:45 -0400] rev 51951
run-tests: bump the minimum python to 3.8
Presumably this was an oversight when hg was updated to 3.8.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 04 Oct 2024 01:23:31 -0400] rev 51950
tests: stabilize `test-sparse.t` on Windows
One of the reserved characters for path values is '*', so it can't be used.
Fortunately, missing this seems to not get in the way of any other tests, and it
is removed shortly after with `rm -r foo*bar`, and the extant 'foo-bar' matches
the pattern.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 03 Oct 2024 21:08:10 -0400] rev 51949
tests: fix a test hang on Windows when setting a debuglock
I have no idea why, but running the `hg -R auto-upgrade debuglock --set-lock`
command near the end of `test-upgrade-repo.t` hangs the test. It does
background the process and `killdaemons.py` runs without error, but control
doesn't return to `run-tests.py` until the process is manually killed. I did
notice that `$!` in MSYS is *not* the PID of the process that got backgrounded,
even when a simple `sleep 60 &` is run in MSYS without the *.t file. When
`killdaemons.py` is run manually with the PID in ProcessExplorer, the
backgrounded process terminates immediately, and returns control to
`run-tests.py`.
This looks like it would be a race, but the test waits 10s for the lock file to
appear before attempting to kill the process, so there's time. `hg serve` has a
`--pid-file` option to write the pid to the file, but this is only a debug
command, so I'm not bothering with cluttering the command line.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 03 Oct 2024 19:49:05 -0400] rev 51948
tests: conditionalize `chmod` usage in `test-upgrade-repo.t`
While the command itself doesn't error out on Windows, it also doesn't make the
filesystem readonly. Therefore the repo gets altered to drop dirstate-v2, and
puts it out of sync with that happens on Linux.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 02 Oct 2024 18:30:12 -0400] rev 51947
tests: print the actual timeout value used in `wait-on-file`
Previously, it was printing the time passed in, prior to it being scaled up to
account for a longer timeout.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 02 Oct 2024 18:19:59 -0400] rev 51946
tests: stabilize `test-transaction-wc-rollback-race.t` on Windows
MSYS has a tendency to munge C:\Dir\SubDir\File into C:DirSubDirFile unless it
is quoted, and that's what was happening here- there were a lot of these
failures:
file not created after 5 seconds: $TESTTMP/transaction-waiting
I suspect quoting is only needed in the hook script that is generated (the
catting of the log file pointed me in the right direction here), but I missed a
spot and trial and error got me here. The quoting elsewhere doesn't harm
anything and it was taking 7+ minutes to run this test when things were timing
out, so I don't feel like reducing the quoting to the minimum required.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 02 Oct 2024 16:34:33 -0400] rev 51945
tests: stabilize `test-merge-partial-tool.t` on Windows
The test was previously failing because it was opening the shell scripts being
used as an executable in a text editor, and problems cascaded from there.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 02 Oct 2024 11:43:22 -0400] rev 51944
tests: replace `hg id --debug -i` command substitution with non-debug command
The censor and convert tests were failing on Windows because the `--debug` flag
also prints debug messages, and at least some of these were outputting:
skip updating dirstate: identity mismatch ${node}
Obviously that causes cascading problems. The other tests were OK, but it's
better to use a non debug command for stability.
Matt Harbison <matt_harbison@yahoo.com> [Tue, 01 Oct 2024 21:40:20 -0400] rev 51943
tests: correct Windows output to account for putting repos in `repo` subdir
These were missed in
55c6ebd11cb9, due to being conditionalized and not running
in CI.
Matt Harbison <matt_harbison@yahoo.com> [Tue, 01 Oct 2024 21:34:44 -0400] rev 51942
tests: use pattern matching to mask `ECONNREFUSED` messages
The second and third one of these in `test-http-proxy.t` was failing on Windows.
The others were found by grep and by failed tests when output was matched and an
attempt was made to emit the mask pattern.
The first clonebundles failure on Windows emitted:
error fetching bundle: [WinError 10061] $ECONNREFUSED$
We should probably stringify that better to get rid of the "[WinError 10061]"
part.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 05 Oct 2024 17:32:26 -0400] rev 51941
typing: add stub functions for `cext/charencoding`
I'm not sure if it's better to have a separate file, and currently pytype
doesn't really know how to handle these, so it's no help in figuring that out.
Technically, these methods are part of the `mercurial.cext.parsers` module, so
put them into the existing stub until there's a reason to split it out.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 05 Oct 2024 15:00:37 -0400] rev 51940
interfaces: introduce and use a protocol class for the `charencoding` module
See
f2832de2a46c for details when this was done for the `bdiff` module.
This lets us dump the hack where the `pure` implementation was imported during
the type checking phase to provide signatures for the module methods it
provides. Now the protocol classes are starting to shine, because these methods
are provided by `pure.charencoding` and `cext.parsers`, and references to
`cffi.charencoding` and `cext.charencoding` are forwarded to them as appropriate
by the `policy` module. But none of that matters, as long as the module
returned provides the listed methods.
The interface was copy/pasted from the `pure` module, but `jsonescapeu8fallback`
is omitted because it is accessed from the `pure` module directly when the
escaping fails in the primary module's `jsonescapeu8()`.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 04 Oct 2024 23:23:24 -0400] rev 51939
debugantivirusrunning: use bytes when opening a vfs file
I noticed this when searching for "base85" to see if anything else in the
previous commit needed to be annotated. This was added in
87047efbc6a6, after
the mass byteification in
687b865b95ad.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 04 Oct 2024 23:21:41 -0400] rev 51938
interfaces: introduce and use a protocol class for the `base85` module
See
f2832de2a46c for details when this was done for the `bdiff` module.
It looks like PEP-688 removed the special casing of `bytes` being a standin
for any type of `ByteString`, and defines a `typing.Buffer` class (with a
backport in `typing_extensions` for Python prior to 3.12). There's been a lot
of churn in this area with pytype, but recent versions of pytype and PyCharm
recognize this, and e.g. have `mercurial.node.hex()` defined as:
from typing_extensions import Buffer
def hex(data: Buffer, sep: str | bytes = ..., bytes_per_sep: int = ...) -> bytes
This covers `bytes`, `bytearray`, and `memoryview` by default. Both of the C
functions here use `y#` to parse the arguments, which means the arg is a
byte-like object[2], so the args would appear to be better typed as `Buffer`.
However, pytype has a bug that prevents using this from `typing_extensions`[3],
and mypy complained `Unsupported left operand type for + ("memoryview")` in the
pure module on line 37 (meaning it's only a subset of `Buffer`). So hold off on
changing any of that for now.
[1] https://peps.python.org/pep-0688/#no-special-meaning-for-bytes
[2] https://docs.python.org/3/glossary.html#term-bytes-like-object
[3] https://github.com/google/pytype/issues/1772
Matt Harbison <matt_harbison@yahoo.com> [Fri, 04 Oct 2024 23:09:56 -0400] rev 51937
base85: avoid a spurious use-before-initialized warning in `pure` module
The error wasn't possible because the only way for `acc` to not be initialized
was if `len(text) == 0`. But then `0 % 5 == 0`, so no attempt at padding was
done. It's a simple enough fix to not have PyCharm flag this though. The value
needs to be reset on each loop iteration, so it's a line copy, not a line move.
Matt Harbison <matt_harbison@yahoo.com> [Mon, 30 Sep 2024 19:40:14 -0400] rev 51936
typing: add type annotations to `mercurial/mdiff.py`
We'll leave converting `diffopts` to `attrs` as another project.
Matt Harbison <matt_harbison@yahoo.com> [Mon, 30 Sep 2024 23:50:40 -0400] rev 51935
mdiff: convert a few block definitions from lists to tuples
These were flagged by adding type hints. Some places were using a tuple of 4
ints to define a block, and others were using a list of 4. A tuple is better
for typing, because we can define the length and the type of each entry. One of
the places had to redefine the tuple, since writing to a tuple at an index isn't
supported.
This change spills out into the tests, and archeology says it was added to the
repo in this state. There was no reason given for the divergence, and I suspect
it wasn't intentional.
It looks like `splitblock()` is completely unused in the codebase.
Matt Harbison <matt_harbison@yahoo.com> [Sun, 29 Sep 2024 02:03:20 -0400] rev 51934
interfaces: add the optional `bdiff.xdiffblocks()` method
PyCharm flagged where this was called on the protocol class in `mdiff.py` in the
previous commit, but pytype completely missed it. PyCharm is correct here, but
I'm committing this separately to highlight this potential problem- some of the
implementations don't implement _all_ of the methods the others do, and there's
not a great way to indicate on a protocol class that a method or attribute is
optional- that's kinda the opposite of what static typing is about.
Making the method an `Optional[Callable]` attribute works here, and keeps both
PyCharm and pytype happy, and the generated `mdiff.pyi` and `modules.pyi` look
reasonable. We might be getting a little lucky, because the method isn't
invoked directly- it is returned from another method that selects which block
function to use. Except since it is declared on the protocol class, every
module needs this attribute (in theory, but in practice this doesn't seem to be
checked), so the check for it on the module has to change from `hasattr()` to
`getattr(..., None)`. We defer defining the optional attrs to the type checking
phase as an extra precaution- that way it isn't an attr with a `None` value at
runtime if someone is still using `hasattr()`.
As to why pytype missed this, I have no clue. The generated `mdiff.pyi` even
has the global variable typed as `bdiff: intmod.BDiff`, so uses of it really
should comply with what is on the class, protocol class or not.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 28 Sep 2024 19:12:18 -0400] rev 51933
interfaces: introduce and use a protocol class for the `bdiff` module
This is allowed by PEP 544[1], and we basically follow the example there. The
class here is copied from `mercurial.pure.bdiff`, and the implementation
removed.
There are several modules that have a few different implementations, and the
implementation chosen is controlled by `HGMODULEPOLICY`. The module is loaded
via `mercurial/policy.py`, and has been inferred by pytype as `Any` up to this
point. Therefore it and PyCharm were blind to all functions on the module, and
their signatures. Also, having multiple instances of the same module allows
their signatures to get out of sync.
Introducing a protocol class allows the loaded module that is stored in a
variable to be given type info, which cascades through the various places it is
used. This change alters 11 *.pyi files, for example. In theory, this would
also allow us to ensure the various implementations of the same module are kept
in alignment- simply import the module in a test module, attempt to pass it to a
function that uses the corresponding protocol as an argument, and run pytype on
it.
In practice, this doesn't work (yet). PyCharm (erroneously) flags imported
modules being passed where a protocol class is used[2]. Pytype has problems the
other way- it fails to detect when a module that doesn't adhere to the protocol
is passed to a protocol argument. The good news is that mypy properly detects
this case. The bad news is that mypy spews a bunch of other errors when
importing even simple modules, like the various `bdiff` modules. Therefore I'm
punting on the tests for now because the type info around a loaded module in
PyCharm is a clear win by itself.
[1] https://peps.python.org/pep-0544/#modules-as-implementations-of-protocols
[2] https://youtrack.jetbrains.com/issue/PY-58679/Support-modules-implementing-protocols
Matt Harbison <matt_harbison@yahoo.com> [Sat, 28 Sep 2024 19:11:39 -0400] rev 51932
mdiff: tweak calls into `bdiff.fixws` to match its type hints
It turns out that protocol classes can be used for modules too, which is great
because all of the dynamically loaded modules (and their attributes) are
currently inferred as `Any`. See the next commit for details.
A protocol class for the `bdiff` module detected this (trivial) mismatch, so
correct it first. The various implementations of this method are typed as
taking a `bool`. The `cext` implementation parses its arguments with
`PyArg_ParseTuple(args, "Sb:fixws", &s, &allws)`, which wants an `int`. But
experimenting in `hg debugshell` under py38, passing `True` or `False` to
`cext.fixws()` also works. We can change the implementation to use "p" (which
was introduced in py33) instead of "b", but that's beyond the scope of this.
Matt Harbison <matt_harbison@yahoo.com> [Tue, 01 Oct 2024 15:04:06 -0400] rev 51931
util: minor copy editing of the documentation for `mmapread()`
Matt Harbison <matt_harbison@yahoo.com> [Tue, 01 Oct 2024 15:00:39 -0400] rev 51930
util: make `mmapread()` work on Windows again
522b4d729e89 started referencing `mmap.MAP_PRIVATE`, but that's not available on
Windows, so `hg version` worked, but `make local` did not. That commit also
started calling the constructor with the fine-grained `flags` and `prot` args,
but those aren't available on Windows either[1] (though the backing C code
doesn't seem conditionalized to disallow usage of them).
I assume the change away from from the `access` arg was to provide the same
options, plus `MAP_POPULATE`. Looking at the source code[2], they're not quite
the same- `ACCESS_READ` is equivalent to `flags = MAP_SHARED` and `prot = PROT_READ`.
`MAP_PRIVATE` is only used with `ACCESS_COPY`, which allows read and write.
Therefore, we can't quite get the same baseline flags on Windows, but this was
the status quo ante and `MAP_POPULATE` is a Linux thing, so presumably it works.
I realize that typically the OS differences are abstracted into the platform
modules, but I'm leaving it here so that it is obvious what the differences are
between the platforms.
[1] https://docs.python.org/3/library/mmap.html#mmap.mmap
[2] https://github.com/python/cpython/blob/
5e0abb47886bc665eefdcc19fde985f803e49d4c/Modules/mmapmodule.c#L1539
Matt Harbison <matt_harbison@yahoo.com> [Fri, 27 Sep 2024 12:30:37 -0400] rev 51929
typing: add type annotations to the dirstate classes
The basic procedure here was to use `merge-pyi` to merge the `git/dirstate.pyi`
file in (after renaming the interface class to match), cleaning up the import
statement mess, and then repeating the procedure for `mercurial/dirstate.pyi`.
Surprisingly, git's dirstate had more hints inferred in its *.pyi file.
After that, it was a manual examination of each method in the interface, and how
they were implemented in the core and git classes to verify what was inferred by
pytype, and fill in the missing gaps. Since this involved jumping around
between three different files, I applied the same type info to all three at the
same time. Complex types I rolled up into type aliases in the interface module,
and used that as needed. That way if it changes, there's one place to edit.
There are some hints still missing, and some documentation that doesn't match
the signatures. They should all be marked with TODOs. There are also a bunch
of methods on the core class that aren't on the Protocol class that seem like
maybe they should be (like `set_tracked()`). There are even more methods
missing from the git class. But that's a project for another time.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 27 Sep 2024 12:10:25 -0400] rev 51928
interfaces: change a couple of dirstate fields to `@property`
As I was adding type hints here and to the concrete classes, PyCharm flagged the
property in the core class as not being compatible with the base class's
version.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 27 Sep 2024 12:05:48 -0400] rev 51927
git: make `dirstate.parents()` return a list like the core class
The core class returned a list, so that's how I type annotated it, and this got
flagged. I suppose we could annotate it as a `Sequence[bytes]`, but it's a
trivial difference.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 27 Sep 2024 11:57:42 -0400] rev 51926
typing: add type hints for the overloads of `matchmod.readpatternfile()`
The return type is conditional on an argument passed, and it very much confused
both pytype and PyCharm inside `dirstate._ignorefileandline()` after adding
type hints for the return value there.