Skip to content
Snippets Groups Projects
  1. Oct 03, 2024
  2. Oct 02, 2024
  3. Oct 05, 2024
    • Matt Harbison's avatar
      typing: add stub functions for `cext/charencoding` · e58f02e2
      Matt Harbison authored
      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.
      e58f02e2
    • Matt Harbison's avatar
      interfaces: introduce and use a protocol class for the `charencoding` module · 54d9f496
      Matt Harbison authored
      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()`.
      54d9f496
    • Matt Harbison's avatar
      debugantivirusrunning: use bytes when opening a vfs file · 8d9767bf
      Matt Harbison authored
      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.
      8d9767bf
    • Matt Harbison's avatar
      interfaces: introduce and use a protocol class for the `base85` module · fa7059f0
      Matt Harbison authored
      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
      fa7059f0
    • Matt Harbison's avatar
      base85: avoid a spurious use-before-initialized warning in `pure` module · 936f85b2
      Matt Harbison authored
      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.
      936f85b2
  4. Sep 30, 2024
  5. Oct 01, 2024
    • Matt Harbison's avatar
      mdiff: convert a few block definitions from lists to tuples · 77e2994b
      Matt Harbison authored
      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.
      77e2994b
  6. Sep 29, 2024
    • Matt Harbison's avatar
      interfaces: add the optional `bdiff.xdiffblocks()` method · 09f3a679
      Matt Harbison authored
      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.
      09f3a679
  7. Sep 28, 2024
    • Matt Harbison's avatar
      interfaces: introduce and use a protocol class for the `bdiff` module · f2832de2
      Matt Harbison authored
      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
      f2832de2
    • Matt Harbison's avatar
      mdiff: tweak calls into `bdiff.fixws` to match its type hints · d94e21b5
      Matt Harbison authored
      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.
      d94e21b5
  8. Oct 01, 2024
  9. Sep 27, 2024
    • Matt Harbison's avatar
      typing: add type annotations to the dirstate classes · 93d872a0
      Matt Harbison authored
      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.
      93d872a0
    • Matt Harbison's avatar
      interfaces: change a couple of dirstate fields to `@property` · 3688a984
      Matt Harbison authored
      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.
      3688a984
    • Matt Harbison's avatar
      git: make `dirstate.parents()` return a list like the core class · e99c0070
      Matt Harbison authored
      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.
      e99c0070
    • Matt Harbison's avatar
      typing: add type hints for the overloads of `matchmod.readpatternfile()` · 70fe33bd
      Matt Harbison authored
      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.
      70fe33bd
  10. Sep 26, 2024
    • Matt Harbison's avatar
      dirstate: subclass the new dirstate Protocol class · 3a90a6fd
      Matt Harbison authored
      Behold the chaos that ensues.  We'll use the generated *.pyi files to apply type
      annotations to the interface, and see how much agrees with the documentation.
      
      Since the CamelCase name was used to try to work around pytype issues with zope
      interfaces and is a new innovation this cycle (see c1d7ac70980b), drop the
      CamelCase name.  I think the Protocol classes *should* be CamelCase, but that
      can be done later in one pass.  For now, the CamelCase alias is extra noise in
      the *.pyi files.
      3a90a6fd
    • Matt Harbison's avatar
      git: correct some signature mismatches between dirstate and the Protocol class · 51be8bf8
      Matt Harbison authored
      These were flagged by PyCharm when subclassing the Protocol class.  Note that
      both `is_changing_xxx` were only flagged when the Protocol class used a plain
      field, as mentioned in the previous commit.  After converting those attrs in the
      Protocol class to @property to match the regular dirstate class, it stopped
      flagging these.  But I don't think that makes sense- `@property` should look
      like an attribute to the outside world, not a callable.
      51be8bf8
    • Matt Harbison's avatar
      interfaces: convert the zope `Attribute` attrs to regular fields · b455dfdd
      Matt Harbison authored
      At this point, we should have a useful protocol class.
      
      The file syntax requires the type to be supplied for any fields that are
      declared, but we'll leave the complex ones partially unspecified for now, for
      simplicity.  (Also, the things documented as `Callable` are really as future
      type annotating worked showed- roll with it for now, but they're marked as TODO
      for fixing later.)  All of the fields and all of the attrs will need type
      annotations, or the type rules say they are considered to be `Any`.  That can be
      done in a separate pass, possibly applying the `dirstate.pyi` file generated
      from the concrete class.
      
      The first cut of this turned the `interfaceutil.Attribute` fields into plain
      fields, and thus the types on them.  PyCharm flagged a few things as having
      incompatible signatures when the concrete dirstate class subclassed this, when
      the concrete class has them declared as `@property`.  So they've been changed to
      `@property` here in those cases.  The remaining fields that are decorated in the
      concrete class have comments noting the differences.  We'll see if they need to
      be changed going forward, but leave them for now.  We'll be in trouble if the
      `@util.propertycache` is needed, because we can't import that module here at
      runtime, due to circular imports.
      b455dfdd
    • Matt Harbison's avatar
      interfaces: add the missing `self` arg to the dirstate Protocol class · 13aa1751
      Matt Harbison authored
      This clears all of the errors that PyCharm has been flagging in this file, since
      the zope interface was declared here.
      13aa1751
    • Matt Harbison's avatar
      interfaces: convert the dirstate zope interface to a Protocol class · 382d9629
      Matt Harbison authored
      This is a small trial run for converting the repository interfaces enmasse, in
      the same series of steps.  I'm not sure that this current code is valid (it has
      zope attribute fields, and it's missing all of the `self` args on its functions,
      but that was the previous state of things, and made PyCharm really unhappy).
      But it will be easier to review the repository interface changes if this change
      is separate from adding `self` and dropping the zope attributes all over.
      
      Having an empty constructor in a protocol is weird.  I'm not sure if these args
      should be converted to fields that all subclasses would have, and comments
      around existing attributes say some should be going away.  Comment it out for
      now so that it's not in the way, but also not forgotten.
      382d9629
    • Matt Harbison's avatar
      tests: disable `test-check-interfaces.py` while converting to protocols · ef7d8508
      Matt Harbison authored
      The goal is to convert everything, so get it all out of the way.  The interfaces
      don't get that much maintenance that this needs to be tested right now.
      ef7d8508
  11. Oct 02, 2024
  12. Sep 27, 2024
  13. Jul 20, 2024
  14. Jul 19, 2024
  15. Sep 02, 2024
    • Pierre-Yves David's avatar
      rev-branch-cache: reenable memory mapping of the revision data · 5d352e36
      Pierre-Yves David authored
      Now that we are no longer truncating it, we can mmap it again.
      
      This provide a sizeable speedup on repository with a very large amount of
      revision for example for a mozilla-try clone with 5 793 383 revisions, this
      provide a speedup of 5ms - 10ms. Since they happens within the "critical" locked
      path during push. These miliseconds are important.
      
      In addition, the v3 branchmap format is use the rev-branch-cache more than the
      v2 branchmap cache so this will be important.
      
      On smaller repository we consistently see an improvement of one or two percents,
      but the gain in absolute time is usually < 10 ms.
      
      #### benchmark.name                                 = hg.command.unbundle
         # benchmark.variants.issue6528                   = disabled
         # benchmark.variants.reuse-external-delta-parent = yes
         # benchmark.variants.revs                        = any-1-extra-rev
         # benchmark.variants.source                      = unbundle
         # benchmark.variants.verbosity                   = quiet
      
       ### data-env-vars.name = mozilla-try-2024-03-26-zstd-sparse-revlog
        ## bin-env-vars.hg.flavor = default
      e51161b12c7e: 3.527923
      ebdcfe85b070: 3.468178   (-1.69%, -0.06)
        ## bin-env-vars.hg.flavor = rust
      e51161b12c7e: 3.580158
      ebdcfe85b070: 3.480564   (-2.78%, -0.10)
       ### data-env-vars.name = mozilla-try-2024-03-26-ds2-pnm
        ## bin-env-vars.hg.flavor = rust
      e51161b12c7e: 3.527923
      ebdcfe85b070: 3.468178 (-1.69%, -0.06)
      5d352e36
  16. Sep 25, 2024
Loading