This is necessary for them to be loaded with importlib.resources,
which we want to do for PyOxidizer and similar. importlib.resources
cannot read resources from submodules
(resources.open_binary('mercurial.templates', 'coal/map') is not
valid), so we need one __init__.py per directory.
Details
Details
- Reviewers
indygreg - Group Reviewers
hg-reviewers - Commits
- rHGf55c47f41b8e: templates: add __init__.py files to templates/ dirs
Diff Detail
Diff Detail
- Repository
- rHG Mercurial
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
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).
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.)
Comment Actions
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.
Comment Actions
Having thought about this a bit more, I think using the importlib.resources high-level APIs are better. i.e. this series as-is is fine.
Since Python is talking about yet another low-level loader API, presumably the high-level APIs will be forwards compatible with whatever new low-level APIs are introduced in the future. This future proofs Mercurial.
The downside, unfortunately, is we need to introduce extra packages/__init__.py files. But this feels like a small price to pay.