[Commented On] D8854: templates: add __init__.py files to templates/ dirs

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Mon Aug 3 15:12:50 UTC 2020


martinvonz added a comment.


  In D8854#132358 <https://phab.mercurial-scm.org/D8854#132358>, @indygreg wrote:
  
  > OMG. When I reviewed this, I thought your assertion that `resources.open_binary('mercurial.templates', 'coal/map')` is not valid was wrong because the low-level resource reader on the meta path importer does allow relative paths! However, it appears that the `open_binary()` helper functions limit this! This is yet another example of wonky behavior in these resources APIs. FWIW I've brought many of these to the attention of the core Python people at https://bugs.python.org/issue36128 and https://gitlab.com/python-devs/importlib_resources/-/issues/58 (and a few other places that I can't find the links to).
  
  Huh, I remember reading that bug report a while ago, but I had forgotten about it when I was working on this series.
  
  > Anyway, we could potentially avoid having `__init__.py` files if we bypassed the `importlib.resources` helper functions and accessed the APIs on `mercurial.__loader__` instead. I'll hold off queuing this part of the series to give you time to contemplate that (possibly simpler) alternative. I will say that a benefit to `importlib.resources` is they are higher-level and should abstract any changes to the low-level importer/loader interface. (The Python people were considering yet another API in Python 3.9, although I'm not sure if it was finalized.)
  
  I consider you the expert on this, so I'll let you decide. So you're saying we could rewrite our `mercurial.utils.resourceutil` module so it uses `mercurial.__loader__` instead of `importlib.resources` and then we can drop this patch completely, and also remove the `helptext/__init__.py` and `defaultrc/__init__.py` files that we've added earlier? That sounds nice. I assume that will work on PyOxidizer, but is it possible that we might break similar tools? Would it be according to spec for another frozen-binary-tool to create a separate `__loader__` for non-package submodules? Even *if* that's allowed according to the specs, I trust you if you tell me that it's unlikely enough that we shouldn't worry about it.

REPOSITORY
  rHG Mercurial

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8854/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8854

To: martinvonz, #hg-reviewers, indygreg
Cc: indygreg, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mercurial-scm.org/pipermail/mercurial-patches/attachments/20200803/08ea0684/attachment.html>


More information about the Mercurial-patches mailing list