Matt Harbison <matt_harbison@yahoo.com> [Wed, 21 Aug 2024 16:13:14 -0400] rev 51820
typing: add type hints to the `opener` attributes and arguments of revlog
When making revlog and filelog classes visible to pytype, it got confused quite
a bit in `mercurial/revlogutils/rewrite.py`, thinking it had a plain `Callable`,
and flagging additional methods on it like `join()` and `rename()`. I couldn't
figure out how it reduced to that (and PyCharm flagged `opener` references as
`Any`), but this makes it happy. So make this change before making the classes
visible.
The vfs class hierarchy is a bit wonky (e.g. `filteredvfs` is not a `vfs`), so
this may need to be revisited with a Protocol class that covers all of the `vfs`
classes. But for now, everything works.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 21 Aug 2024 16:09:22 -0400] rev 51819
remotefilelog: honor the `--format` arg of the `debugindex` command
Flagged by PyCharm while investigating pytype spew. The other `**opts` above
are already accessed as str. I've never used remotefilelog, and don't have a
repo to test this on, so I'm trusting the nearby code.
Manuel Jacob <me@manueljacob.de> [Wed, 07 Aug 2024 22:05:36 +0200] rev 51818
merge: sort filemap only if requested by the caller
The name `sorted` refers to a built-in function, which is always true, so the else branch of this if statement was dead code.
Because, with this fix, the function can iterate over the dict items while yielding values, the dict should not change size while the generator is running. Because of that, it is required to re-introduce code that makes a caller copy the filemap before modification, which was removed in
3c783ff08d40cbaf36eb27ffe1d296718c0f1d77 (that changeset also introduced the filemap() method including the bug that’s being fixed by this changeset).
Matt Harbison <matt_harbison@yahoo.com> [Tue, 20 Aug 2024 22:47:11 -0400] rev 51817
shelve: consistently convert exception to bytes via `stringutil.forcebytestr`
The other two places in this module use this, and past experience shows that
this method does a nicer job. I'm not sure why we're converting to bytes here-
`KeyError` is built-in and will have str attrs, and `RepoLookupError` is a
subclass of the built-in `Exception` class (not `errors.Error`, which is
allegedly the baseclass for all Mercurial exceptions).
Matt Harbison <matt_harbison@yahoo.com> [Tue, 20 Aug 2024 22:34:51 -0400] rev 51816
typing: add type hints to `mercurial.shelve`
Pytype wasn't flagging anything here yet, but PyCharm was really unhappy about
the usage of `state` objects being passed to various methods that accessed attrs
on it, without any obvious attrs on the class because there's no contructor.
Filling that out made PyCharm happy, and a few other things needed to be filled
in to make that easier, so I made a pass over the whole file and filled in the
trivial hints. The other repo, ui, context, matcher, and pats items can be
filled in after the context and match modules are typed.
Matt Harbison <matt_harbison@yahoo.com> [Tue, 20 Aug 2024 18:30:47 -0400] rev 51815
typing: lock in correct changes from pytype 2023.04.11 -> 2023.06.16
There were a handful of other changes to the pyi files generated when updating
pytype locally (and jumping from python 3.8.0 to python 3.10.11), but they were
not as clear (e.g. the embedded type in a list changing from `nothing` to `Any`
or similar). These looked obviously correct, and agreed with PyCharm's thoughts
on the signatures.
Oddly, even though pytype starting inferring `obsutil._getfilteredreason()` as
returning bytes, it (correctly) complained about the None path when it was typed
that way. Instead, raise a ProgrammingError if an unhandled fate is calculated.
(Currently, all possibilities are handled, so this isn't reachable unless
another fate is added in the future.)
Matt Harbison <matt_harbison@yahoo.com> [Tue, 20 Aug 2024 17:46:17 -0400] rev 51814
monotone: replace %s interpolation with appropriate numeric specifiers
The length is an int, and the version is a float. Neither work with bytes on
py3. This was noticed when looking at nearby code after updating pytype changed
some signatures.
Matt Harbison <matt_harbison@yahoo.com> [Tue, 20 Aug 2024 16:32:13 -0400] rev 51813
shelve: raise an error when loading a corrupt state file in an impossible case
The old return statement was flagged by pytype 2023.06.16 running under python
3.10.11. No idea why it isn't caught in CI running the same pytype with py3.7.
This function is only called by `unshelvecmd()` (which first checks that either
`--abort` or `--continue` is specified), and `hgabortunshelve()` and
`hgcontinueunshelve()`, which locally apply `--abort` or `--continue`
respectively. Therefore, there is no other way to call this, and this error
should never be seen, but pytype can't figure that out on its own. Given that
the abort case clears the state, it seems reasonable to defensively code this
and not make that a blanket `else` case, on the off chance a 3rd way of calling
this appears in the future.
Matt Harbison <matt_harbison@yahoo.com> [Tue, 20 Aug 2024 11:18:10 -0400] rev 51812
contrib: print the version of pytype used to do the type checking
This will help with CI. I don't see a way to print the version of python that's
running it. When I tried `head -n 1 $(which pytype)`, the CI run printed:
#!/usr/bin/env bash
Locally, that gives the path to the python interpreter in the venv, so IDK
what's different.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 17 Aug 2024 18:43:23 -0400] rev 51811
typing: create an @overload of `phasecache` ctor to handle the copy case
In `phasecache.copy()`, it calls `self.__class__(None, None, _load=False)`, but
the constuctor is typed to take a non-None repository. For the `_load=False`
case, all args are ignored (and the copy function itself populates the attrs on
the new object), so this isn't an error. For the default `_load=True` case, it
needs a non-None repository. This is the simplest way to handle that duality.
The reason this wasn't being detected is because pytype is confused by the
interface decorators on the `localrepository` class, and is inferring the whole
class as `Any`. (See
3e9a660b074a or
c1d7ac70980b) Therefore, the type hint of
`localrepo.localrepository` here was also effectively `Any`, which disabled the
type checking entirely.
This is the first foray into using `typing_extensions` to unlock future typing
features. I think this is safe and reasonable because 1) it is only imported in
the type checking phase (so no need to vendor our own copy), and 2) pytype has
its own copy of `typing_extensions` bundled with it, so no need to alter the
test environment. When run with a version of python that supports the symbol(s)
natively, `typing_extensions` simply re-exports from `typing`, so there
shouldn't be any future headaches with this.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 17 Aug 2024 17:38:35 -0400] rev 51810
typing: declare the `_phasesets` member of `phasecache` to be `Optional`
Something in this area got flagged while making the repository class visible to
pytype (instead of being typed as `Any`). A None assignment to something not
optional is wrong, and when I tried setting it to `{}` to keep it non-Optional,
some tests failed. There are checks for the attr being None elsewhere, so this
seems to have just been an oversight.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 16 Aug 2024 18:11:52 -0400] rev 51809
typing: hide the interface version of `dirstate` during type checking
As noted in the previous commit, the `dirstate` type is still inferred as `Any`
by pytype, including where it is used as a base class for the largefiles
dirstate. That effectively disables most type checking. The problems fixed two
commits ago were flagged by this change.
I'm not at all clear what the benefit of the original type is, but that was what
was used at runtime, so I don't want to change the largefiles base class to the
raw class. Having both a lowercase and camelcase name for the same thing isn't
great, but given that this trivially finds problems without worrying about which
symbol clients may be using, and the non-raw type is useless to pytype anyway,
I'm not going to worry about it.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 16 Aug 2024 18:02:32 -0400] rev 51808
dirstate: remove the interface decorator to help pytype
This is the same change that was made for some of the manifest classes in
3e9a660b074a. Note that `dirstate` is still inferred as `Any`, but at least we
have `DirState` with all of the expected attributes.
Matt Harbison <matt_harbison@yahoo.com> [Fri, 16 Aug 2024 17:58:17 -0400] rev 51807
largefiles: sync up `largefilesdirstate` methods with `dirstate` base class
As it currently stands, pytype infers the `dirstate` class (and anything else
decorated with `@interfaceutil.implementer`) as `Any`. When that is worked
around, it suddenly noticed that most of these methods don't exist in the
`dirstate` class anymore. Since they only called into the missing methods and
there's no test failures, we can assume these are never called, and they can be
dropped.
In addition, PyCharm flagged `set_tracked()` and `_ignore()` as not overriding
a superclass method with the same arguments. The missing default parameter for
the former was the obvious issue. I'm guessing that the latter was named wrong
because while there is `_ignore()` in the base class, it takes no arguments and
returns a matcher. The `_ignorefiles()` superclass method also takes no args,
and returns a list of bytes. The `_ignorefileandline()` superclass method DOES
take a file, but returns a tuple. Therefore, the closest match is `_dirignore()`,
which takes a file AND returns a bool. No idea why this needs to be overridden
though.
Arseniy Alekseyev <aalekseyev@janestreet.com> [Fri, 16 Aug 2024 11:12:19 +0100] rev 51806
sparse: reliably avoid writing to store without a lock
With the code as written before this patch we can still end up writing to
store in `debugsparse`. Obviously we'll write to it if by accident a store
requirement is modified, but more importantly we write to it if another
concurrent transaction modifies the requirements file on disk.
We can't rule this out since we're not holding the store lock,
so it's better to explicitly pass a permission to write instead
of inferring it based on file contents.
Arseniy Alekseyev <aalekseyev@janestreet.com> [Thu, 15 Aug 2024 13:52:14 +0100] rev 51805
debugsparse: stop taking the store lock
debugsparse is a workspace-only opperation, or it better be workspace-only.
Let's make it to stop taking the store lock.
Arseniy Alekseyev <aalekseyev@janestreet.com> [Thu, 15 Aug 2024 14:54:22 +0100] rev 51804
scmutils: read the requires file before writing to avoid unnecessary rewrite
This lets us get away without the repo lock in situations where we need
to write requirements, but we know we're not changing the store requirements.
Arseniy Alekseyev <aalekseyev@janestreet.com> [Thu, 15 Aug 2024 14:56:50 +0100] rev 51803
localrepo: remove _readrequires function in favor of scmutil.readrequires
Arseniy Alekseyev <aalekseyev@janestreet.com> [Thu, 15 Aug 2024 14:53:17 +0100] rev 51802
scmutil: add `readrequires` next to `writerequires`
The code is copied from localrepo.py.
Matt Harbison <matt_harbison@yahoo.com> [Wed, 14 Aug 2024 03:25:16 -0400] rev 51801
typing: correct a type hint in `mercurial.manifest`
Obvious typo that was flagged by PyCharm.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 10 Aug 2024 14:22:26 -0400] rev 51800
typing: add hints to `mercurial.util.mktempcopy()`
Might as well, now that the previous commit indicated what types are required.
Matt Harbison <matt_harbison@yahoo.com> [Sat, 10 Aug 2024 14:18:44 -0400] rev 51799
typing: fix the hint for the `mode` argument of `platform.copymode()`
The posix module is doing a bitwise AND with this and an integer, so it can't be
bytes. The only caller that provides the argument is `util.mktempcopy()`, and
pytype infers the type as Any, which explains why this wasn't caught.
Manuel Jacob <me@manueljacob.de> [Fri, 09 Aug 2024 22:45:32 +0200] rev 51798
largefiles: fix check that ensures that --all-largefiles is only used locally
Previously, the command added in the test failed with “abort: --all-largefiles is incompatible with non-local destination existing_destination”.
The reason for the buggy behavior was the use of hg.islocal(), which does “return true if repo (or path pointing to repo) is local” and, for local paths, assumes that the path is actually pointing to an existing repository and returns whether the path is not a regular file (in which case it assumes that it is a bundlerepo, which are considered non-local).
Felipe Contreras <felipe.contreras@gmail.com> [Fri, 05 May 2023 06:08:36 -0600] rev 51797
exchange: trivial simplification
Both sides of the condition do essentially the same thing, except one
with fastpath=True.
No functional changes.