Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Dec 2022 21:13:11 -0500] rev 49811
typing: add type hints to the posix platform module matching win32.py
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Dec 2022 18:02:55 -0500] rev 49810
typing: add type hints to mercurial/win32.py
These are the low level functions that are imported by the mercurial.windows
module, which is in turn imported by mercurial.utils as the platform module.
Pretty straightforward, but pytype inferred very little of it, likely because of
the heavy ctypes usage. It also seems to trigger a pytype bug in procutil, now
that it has an idea of the underlying function type, so disable that warning to
maintain a working test.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Dec 2022 15:46:25 -0500] rev 49809
windows: drop some py2 registry module importing
The comment was actually backwards- `winreg` is importable on py3, and is
already imported by mercurial/windows.py.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Dec 2022 15:41:59 -0500] rev 49808
typing: add type hints to the platform specific scm modules
Surprisingly, pytype struggled to figure out the return types in the posix
functions.
Matt Harbison <matt_harbison@yahoo.com> [Thu, 15 Dec 2022 01:05:27 -0500] rev 49807
typing: add type hints to most mercurial/pycompat.py functions
The `rapply` methods are left out because it's not `rapply(f, xs: _T0) -> _T0`
as I first thought- it's used somewhere to walk a collection and convert between
bytes and str.
Also, the `open()` call is partially untyped because I'm not sure what its
purpose is at this point- both the name and mode can be either bytes or str as
it is currently constituted. It might make sense to assert that the file is
being opened in binary mode (like `namedtempfile()`) and cast the result to
`BinaryIO`, but that shouldn't be smuggled in with these other changes. The
return is currently typed as `Any` because something suddenly got smarter and a
few uses in util.py (like readfile()) suddenly think it returns `IO[str]`
instead of `IO[bytes]` (BinaryIO), and it flags the type mismatch there.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 14 Dec 2022 22:27:22 -0500] rev 49806
statprof: don't pass str `sys.argv` to a function expecting bytes
Found by typing the global functions in mercurial.pycompat.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 14 Dec 2022 22:24:54 -0500] rev 49805
typing: drop an unnecessary warning disabling comment in match.py
This stopped being necessary in
d2e1dcd4490d, when the exception stopped being
subscripted.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 14 Dec 2022 22:22:12 -0500] rev 49804
scmposix: don't subscript IOError
This warning disabling has been in place since late 2019 in
667f56d73ceb. We
should have had some py3 support at the time, but both pytype complains and
subscripting a real FileNotFoundError generated in `hg debugshell` crashed, so
maybe this fixes a problem. It looks like all other instances of subscripting
exceptions have been replaced (at least as far as greping for `== errno.`
revealed).
Matt Harbison <matt_harbison@yahoo.com> [Wed, 14 Dec 2022 01:51:33 -0500] rev 49803
typing: add type hints to pycompat.bytestr
The problem with leaving pytype to its own devices here was that for functions
that returned a bytestr, pytype inferred `Union[bytes, int]`. It now accepts
that it can be treated as plain bytes.
I wasn't able to figure out the arg type for `__getitem__`- `SupportsIndex`
(which PyCharm indicated is how the superclass function is typed) got flagged:
File "/mnt/c/Users/Matt/hg/mercurial/pycompat.py", line 236, in __getitem__:
unsupported operand type(s) for item retrieval: bytestr and SupportsIndex [unsupported-operands]
Function __getitem__ on bytestr expects int
But some caller got flagged when I marked it as `int`.
There's some minor spillover problems elsewhere- pytype doesn't seem to
recognize that `bytes.startswith()` can optionally take a 3rd and 4th arg, so
those few places have the warning disabled. It also flags where the tar API is
being abused, but that would be a tricky refactor (and would require typing
extensions until py3.7 is dropped), so disable those too.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 14 Dec 2022 01:38:52 -0500] rev 49802
pycompat: explicitly prefix builtin attr usage with `builtins.`
It doesn't seem like this would fix any bug, because the wrapped functions that
take bytes instead of str are defined after these calls. But PyCharm was
flagging the second and third uses, saying "Type 'str' doesn't have expected
attribute 'decode'". It wasn't flagging the first, but I changed it for
consistency.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 14 Dec 2022 01:32:03 -0500] rev 49801
typing: add type hints to global variables in mercurial/pycompat.py
The way `osaltsep` and `sysexecutable` were defined, pytype determined them to
be `Union[bytes, str]`. This was a problem because that cascaded to all of the
callers, and also because it couldn't be annotated as bytes on the initial
assignment. Therefore, we use a ternary operator.
The documentation says that `sys.executable` can either be None or an empty
string if the value couldn't be determined. We opt for an empty string here
because there are places that blindly pass it to `os.path.xxx()` functions,
which crash if given None. Other places test `if pycompat.sysexecutable`, so
empty string works for both.
Matt Harbison <matt_harbison@yahoo.com> [Tue, 13 Dec 2022 16:48:47 -0500] rev 49800
windows: drop an unused method
The only caller was removed in
563eb25e079b.
Matt Harbison <matt_harbison@yahoo.com> [Mon, 12 Dec 2022 14:10:12 -0500] rev 49799
typing: add type hints to the prompt methods in mercurial/ui.py
The @overloads allow for the callers that pass a non-None `default` to not have
to worry about handling a None return to appease pytype.
Matt Harbison <matt_harbison@yahoo.com> [Mon, 12 Dec 2022 14:17:05 -0500] rev 49798
ui: split the `default` arg out of **kwargs for the internal prompt method
This arg was required anyway, based on how it was accessed. Having it separate
allows it to be typed though, and this will simplify things for the callers- if
a non-None `default` is passed, the return can never be None. That can be
expressed with `@overload` when the arg can be typed, but that's not possible
when it is rolled up in **kwargs.
The default value is simply copied from the public `prompt()` above it.
Matt Harbison <matt_harbison@yahoo.com> [Sun, 11 Dec 2022 00:10:56 -0500] rev 49797
typing: add trivial type hints to mercurial/ui.py
There's not really a pattern here; it's mostly obvious return types and in a few
cases, obvious parameter types. Some other "obvious" functions are left out
because of quirks in how the return value for the various config() functions are
inferred cause pytype to complain.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 10 Dec 2022 14:57:42 -0500] rev 49796
doc: don't pass str to ui methods in check-seclevel.py
Matt Harbison <matt_harbison@yahoo.com> [Sat, 10 Dec 2022 14:44:46 -0500] rev 49795
typing: add type hints related to message output in mercurial/ui.py
This will shake loose some bytes vs str issues in the doc checker.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 10 Dec 2022 00:22:13 -0500] rev 49794
typing: add type hints related to progress bars in mercurial/ui.py
Pretty low hanging fruit while trying to deal with other more complicated parts
of this module.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 25 Nov 2022 18:39:47 -0500] rev 49793
pytype: stop excluding mercurial/ui.py
ui.extractchoices() is perhaps making assumptions that it shouldn't about the
pattern always matching, but presumably we have test coverage for that.
PyCharm flags the updated classes with a warning "Class xxx must implement all
abstract methods", and suggests adding `abc.ABC` to the superclasses. I'm not
sure why, unless it doesn't recognize the `__getattr__()` delegation pattern.
Additionally, we can't unconditionally subclass `typing.BinaryIO` because that
defeats the `__getattr__` delegation to the wrapped object at runtime. Instead,
it has to only subclass during the type checking phase[1].
In any event, this fixes:
File "/mnt/c/Users/Matt/hg/mercurial/ui.py", line 1518, in _runpager:
Function subprocess.Popen.__new__ was called with the wrong arguments [wrong-arg-types]
Expected: (cls, args, bufsize, executable, stdin,
stdout: Optional[Union[IO, int]] = ..., ...)
Actually passed: (cls, args, bufsize, stdin,
stdout: Union[mercurial.utils.procutil.WriteAllWrapper,
mercurial.windows.winstdout], ...)
File "/mnt/c/Users/Matt/hg/mercurial/ui.py", line 1798, in extractchoices:
No attribute 'group' on None [attribute-error]
In Optional[Match[bytes]]
File "/mnt/c/Users/Matt/hg/mercurial/ui.py", line 1799, in extractchoices:
No attribute 'group' on None [attribute-error]
In Optional[Match[bytes]]
[1] https://stackoverflow.com/q/
71365594
Anton Shestakov <av6@dwimlabs.net> [Mon, 12 Dec 2022 17:49:48 +0400] rev 49792
tests: document what the other currently enabled pylint check ensures
Anton Shestakov <av6@dwimlabs.net> [Mon, 12 Dec 2022 17:42:30 +0400] rev 49791
hghave: detect newer pylint
Older versions (e.g. 2.7.2) say: "Usage: pylint [options]"
Newer versions (e.g. 2.15.5) say: "usage: pylint [options]"
Pierre-Yves David <pierre-yves.david@octobus.net> [Sat, 12 Nov 2022 00:18:41 +0100] rev 49790
emitrevision: consider ancestors revision to emit as available base
This should make more delta base valid. This notably affects:
* case where we skipped some parent with empty delta to directly delta against
an ancestors
* case where an intermediate snapshots is stored.
This change means we could sent largish intermediate snapshots over the wire.
However this is actually a sub goal here. Sending snapshots over the wire means
the client have a high odd of simply storing the pre-computed delta instead of
doing a lengthy process that will… end up doing the same intermediate snapshot.
In addition the overall size of snapshot (or any level) is "only" some or the
overall delta size. (0.17% for my mercurial clone, 20% for my clone of Mozilla
try). So Sending them other the wire is unlikely to change large impact on the
bandwidth used.
If we decide that minimising the bandwidth is an explicit goal, we should
introduce new logic to filter-out snapshot as delta. The current code has no
notion explicite of snapshot so far, they just tended to fall into the wobbly
filtering options.
In some cases, this patch can yield large improvement to the bundling time:
### data-env-vars.name = mozilla-try-2019-02-18-zstd-sparse-revlog
# benchmark.name = perf-bundle
# benchmark.variants.revs = last-100000
before: 68.787066 seconds
after: 47.552677 seconds (-30.87%)
That translate to large improvement to the pull time :
### data-env-vars.name = mozilla-try-2019-02-18-zstd-sparse-revlog
# benchmark.name = pull
# benchmark.variants.
issue6528 = disabled
# benchmark.variants.revs = last-100000
before: 142.186625 seconds
after: 75.897745 seconds (-46.62%)
No significant negative impact have been observed.
Pierre-Yves David <pierre-yves.david@octobus.net> [Wed, 09 Nov 2022 13:54:15 -0500] rev 49789
sqlitestore: add an `ancestors` method
We will need it during bundling.
The implementation mirror the one in revlog.
Pierre-Yves David <pierre-yves.david@octobus.net> [Thu, 24 Nov 2022 04:04:19 +0100] rev 49788
emitrevision: if we need to compute a delta on the fly, try p1 or p2 first
Falling back to `prev` does not yield any real value on modern storage and
result in pathological changes to be created on the other side. Doing a delta
against a parent will likely be smaller (helping the network) and will be safer
to apply on the client (helping future pulls by Triggering intermediate
snapshop where they will be needed by later deltas).