Skip to content
Snippets Groups Projects

Report the extension and keyring module versions in `hg version -v`

Merged Matt Harbison requested to merge topic/default/report-version into branch/default
3 unresolved threads

Given the history of compatibility issues between the extension and the keyring module, as well as compatibility issues between the keyring module and the OS or some behavior differences depending on the version of the keyring module, it's a good idea to have the information available for bug reports. This is similar to how the built in git extension reports its pygit2 dependency.

The mechanism used to figure out the version value was originally how the keyring module filled in a __version__ attribute[1], but was subsequently removed due to overhead.[2] Since this method is only called on demand instead of on load, I think the overhead is worth having the info. Even if a manually maintained __version__ attribute is added, this mechanism would be needed to get the keyring module version.

In testing this, I hit a case a few months ago where using pkg_resources would raise a cryptic error about modules having been replaced since they were loaded. It worked in most cases, though when I hit it, it was completely reproducible. It was related to demandimport, but comparing the modules loaded with and without demandimport enabled and adding the differences to the disable list didn't help. Wrapping the try/catch that imports pkg_resources with demandimport.disable() fixed the problem, but when I commented that out now to reproduce the issue, I couldn't. It's been tested in WSL with py2.7.15, and py3.8.0, the TortoiseHg package on Windows (py2.7.18), py3.8.1 in a venv on Windows, and py3.9.0 on Windows from a Mercurial source based installation.

[1] https://github.com/jaraco/keyring/blob/9066fbf0abee465bbe230e0e76d2598a9f08569c/keyring/__init__.py#L9 [2] https://github.com/jaraco/keyring/blob/main/CHANGES.rst#1100

Merge request reports

Merged by Marcin KasperskiMarcin Kasperski 2 years ago (Mar 26, 2022 1:03pm UTC)

Merge details

  • Changes merged into with 3ca40404.
  • Deleted the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Author Maintainer

    Hold off on this- I hit the issue again. Maybe it's a 3.6.9 vs 3.8.0 issue, or maybe it running from hg source vs a pip installed hg:

    Copyright (C) 2005-2021 Olivia Mackall and others
    This is free software; see the source for copying conditions. There is NO
    warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
    Traceback (most recent call last):
      File "/mnt/c/Users/Matt/projects/foss/mercurial/mercurial_keyring/mercurial_keyring.py", line 899, in _getversion
        dist = pkg_resources.get_distribution(mod)
      File "/usr/lib/python3.6/importlib/util.py", line 233, in __getattribute__
        self.__spec__.loader.exec_module(self)
      File "<frozen importlib._bootstrap_external>", line 678, in exec_module
      File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
      File "/usr/lib/python3/dist-packages/pkg_resources/__init__.py", line 74, in <module>
        from pkg_resources.extern import packaging
      File "<frozen importlib._bootstrap>", line 971, in _find_and_load
      File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
      File "<frozen importlib._bootstrap>", line 656, in _load_unlocked
      File "<frozen importlib._bootstrap>", line 629, in _load_backward_compatible
      File "/usr/lib/python3.6/importlib/util.py", line 238, in __getattribute__
        raise ValueError(f"module object for {original_name!r} "
    ValueError: module object for 'pkg_resources._vendor.packaging' substituted in sys.modules during a lazy load
  • Matt Harbison added 1 commit

    added 1 commit

    • 8784ee615cac - Report the extension and keyring module versions in `hg version -v`

    Compare with previous version

    • Not quite sure about that. Mayhaps it would be safer to simply implement separate command like hg keyring_version? This way whatever wrong happens, doesn't happen without explicit request. At least this could be safer first step.

      I wiill think more about it next weekend.

    • Author Maintainer

      Mayhaps it would be safer to simply implement separate command like hg keyring_version

      That's possible too, but this has the nice benefit of providing data with the generated bug report output in hg and thg. Some people just do a driveby report. If there's really a safety issue, it should probably be handled in core hg because this wouldn't be the only extension that might have an issue. But I'm not really in a rush for this.

      You mentioned previously about not getting emails about PRs. Did it make a difference if I set you as a reviewer (this one) or "reviewer" and "assigned to" (the other one you merged today)?

    • Author Maintainer

      Gentle ping on this. Mercurial wraps the call to this in except Exception so even if this goes completely off the rails, there's no harm. It would be nice to have this info in the thg bug reporting, and with the rest of the extension info in hg version -v.

    • Please register or sign in to reply
  • mentioned in commit 3ca404042599

    • Released in 1.4.3.

      I slightly fixed the patch so it works for older mercurials too, and added info about selected backend:

      $ hg version -v
      
        mercurial_keyring           external  1.4.3 (keyring 23.5.0, backend keyring.backends.SecretService)
      

      Let's see whether it works OK.

    • Author Maintainer

      Nice, that will be helpful too.

      Do you have a policy for what old versions to support? I didn't think really old versions of hg were still used, as there are security fixes as recent as 4.9.

    • Author Maintainer

      I'm not sure how to handle this, so I'll just leave a comment for now and come back to it at some point.

      When I built the thg app on Windows, I noticed it was still reporting the version info as 1.2.1 (which is what was previously bundled on Windows). It turns out that pkg_resources.get_distribution isn't doing anything with the code bundled in the app, and instead is looking at what's installed in user-site. It should still be good enough on Linux where the app isn't bundled into a zip file (and where most of the version headaches happen anyway). Maybe the situation will improve with py3, but we're kinda stuck since the keyring module doesn't provide any version info about itself.

    • Please register or sign in to reply
    • I try to support as old versions, as possible (in mercurial_keyring, and in other extensions) – still have users of hg 4.0.2, for example.

      Regarding version … I accepted your patch which uses pkg_resources, but I can simply revert to constant if that causes problems (I use https://pypi.org/project/mercurial_update_version/ while tagging so that's not major maintanance hassle). Does it make sense?

    • Author Maintainer

      A hardcoded version field makes sense for the extension itself, but I don't think a simple revert does- the backend module is also useful info, so we'll want to keep the function to report both.

      The keyring module doesn't export version info, and a request to do so[1] was closed because it sounds like there was a sufficient workaround, though it doesn't work here. I'll try asking again with this use case[2].

      [1] https://github.com/jaraco/keyring/issues/537 [2] https://github.com/jaraco/keyring/issues/567

    • Author Maintainer

      I got a reply on 567 that seems to work for keyring in py2exe (and I think py2app, but it's been several days now). Unfortunately, the *.egg-info that gets generated when pip installing mercurial_keyring doesn't get bundled into the application (I assume because they are just a couple of *.py modules, and not a package).

      Assuming that's why it's not bundled, having a constant is the easiest thing to do. I'll implement the code to handle the 567 suggestion and constant for the extension if you handle generating the file.

    • Please register or sign in to reply
Please register or sign in to reply
Loading