René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"We appear to import the submodule version of gitdb (but never do)","articleBody":"### Summary\r\n\r\n`git/__init__.py` imports exceptions indirectly from `gitdb` before it adjusts `sys.path`, causing the code from the `gitdb` git-submodule not to be used under any circumstances.\r\n\r\nAlthough this could probably be fixed by moving `gitdb.exc` imports below the `_init_externals` call, I recommend against it, because this is an opportunity to further decrease the project's use and dependence on its git-submodules. This is to say that I suggest viewing this as a bug due to the code appearing to do something it doesn't, rather than as a bug due to the code not doing something it should. Even without removing the git-submodule--which GitPython's tests need--the logic to insert it in `sys.path` could simply be removed.\r\n\r\n### Details\r\n\r\nThe unit tests use the direct `gitdb` git-submodule as well as the indirect `smmap` git-submodule through it (and for this reason the git-submodules cannot be immediately removed). However, `git/__init__.py` also *appears* to use them by cauisng `gitdb` to be imported from the `gitdb` git-submodule under some circumstances:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/44102f30eaadcd122899f5f801f28b83bd9a5111/git/__init__.py#L17-L38\r\n\r\nThe intent is that, then, importing Python modules some of which contain `gitdb` imports should use the `gitdb` Python module from the `gitdb` git-submodule. Furthermore, it is intended that this happen even if the `gitdb` package is installed, as evidenced by how the `git/ext/gitdb` directory is inserted near the beginning of `sys.path`. The idea is that the git-submodule version of `gitdb` would be used in these immediately subsequent imports:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/44102f30eaadcd122899f5f801f28b83bd9a5111/git/__init__.py#L40-L62\r\n\r\nHowever, that does not happen. The code from the `gitdb` git-submodule is actually *never* used, because before *any* of that, we import `git.exc`:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/44102f30eaadcd122899f5f801f28b83bd9a5111/git/__init__.py#L8\r\n\r\n`git.exc` imports from the `gitdb` Python module:\r\n\r\nhttps://github.com/gitpython-developers/GitPython/blob/44102f30eaadcd122899f5f801f28b83bd9a5111/git/exc.py#L8-L18\r\n\r\nAlthough that code was touched in #1659, the change there did not affect this behavior in any way. Furthermore, the imports in `git.exc` from `gitdb.exc` are not the only imports in `git.exc` that cause `gitdb` to be imported. For example, `git.exc` also imports from `git.util`, which imports from `gitdb.util`.\r\n\r\nBecause the `from git.exc import *` imports `gitdb` before `sys.path` is modified, the installed `gitdb` dependency is used, rather than the git-submodule version of `gitdb`. Subsequent imports reuse the already imported `gitdb` module. That the installed `gitdb` is being used can be verified in a REPL by importing `git` and then `gitdb` and checking `gitdb.__file__`:\r\n\r\n```python\r\n\u003e\u003e\u003e import git\r\n\u003e\u003e\u003e import gitdb\r\n\u003e\u003e\u003e gitdb.__file__\r\n'/home/ek/repos-wsl/GitPython/.venv/lib/python3.12/site-packages/gitdb/__init__.py'\r\n```\r\n\r\nIt is using `gitdb` from the environment's `site-packages` directory, which is the installed version. This alternative demonstration (in a new REPL) is no more robust, but it is perhaps more compelling since no code outside of GitPython ever imports `gitdb`:\r\n\r\n```python\r\n\u003e\u003e\u003e import git\r\n\u003e\u003e\u003e import sys\r\n\u003e\u003e\u003e sys.modules[\"gitdb\"]\r\n\u003cmodule 'gitdb' from '/home/ek/repos-wsl/GitPython/.venv/lib/python3.12/site-packages/gitdb/__init__.py'\u003e\r\n```\r\n\r\n`sys.path` was modified, just too late to have had an effect. Checking it in the same REPL after importing `git` reveals:\r\n\r\n```python\r\n\u003e\u003e\u003e sys.path\r\n['', '/home/ek/repos-wsl/GitPython/git/ext/gitdb', '/usr/lib/python312.zip', '/usr/lib/python3.12', '/usr/lib/python3.12/lib-dynload', '/home/ek/repos-wsl/GitPython/.venv/lib/python3.12/site-packages']\r\n```\r\n\r\n### Possible solutions\r\n\r\n#### Restoring the behavior of using the git-submodule dependency (not my recommendation)\r\n\r\nBecause the actual change to `sys.path` does take effect, if it were added before anything were imported from `gitdb` then it would have worked. This experiment, done in a new REPL, verifies this:\r\n\r\n```python\r\n\u003e\u003e\u003e import sys\r\n\u003e\u003e\u003e sys.path.insert(1, \"/home/ek/repos-wsl/GitPython/git/ext/gitdb\")\r\n\u003e\u003e\u003e import git\r\n\u003e\u003e\u003e sys.modules[\"gitdb\"]\r\n\u003cmodule 'gitdb' from '/home/ek/repos-wsl/GitPython/git/ext/gitdb/gitdb/__init__.py'\u003e\r\n```\r\n\r\nIt was probably working prior to 6752fad, which [added](https://github.com/gitpython-developers/GitPython/commit/6752fad0e93d1d2747f56be30a52fea212bd15d6#diff-6789dd4ae9fe98745671baf3f0e86d8ead5505232e6ecb26a3760ca10c636c63R8) that import from `gitdb.exc`. So moving the `from git.exc import *` line below the `_init_externals()` call would probably be sufficient to cause the `gitdb` git-submodule to be used, and used under the exact circumstances under which it was intended, and currently wrongly appears, to be used.\r\n\r\nBut I recommend against that. 6752fad was part of #1229, which was merged [well over two years ago](https://github.com/gitpython-developers/GitPython/pull/1229#event-4699299216). At least if this issue has not been discovered before now, then importing `gitdb` from the git-submodule (which is relevant, after all, only to people *developing* GitPython) is not something anyone is relying on as the *default* behavior.\r\n\r\n#### Restoring that behavior but in a weakened form (not my recommendation)\r\n\r\n`gitdb` contains similar code, for its `smmap` dependency. That code in `gitdb` does work, because there is no direct or indirect import of `smmap` before it runs. However, its logic is different. Rather than inserting the `smmap` git-submodule directory near the front of the path [as GitPython does](https://github.com/gitpython-developers/GitPython/blob/44102f30eaadcd122899f5f801f28b83bd9a5111/git/__init__.py#L23-L24)...\r\n\r\n```python\r\n    if __version__ == \"git\" and \"PYOXIDIZER\" not in os.environ:\r\n        sys.path.insert(1, osp.join(osp.dirname(__file__), \"ext\", \"gitdb\"))\r\n```\r\n\r\n...it instead inserts it [at the end, where a package installed any other way would take precedence](https://github.com/gitpython-developers/gitdb/blob/6a2270635842c4287ae14d94fdc3f9e1ddd0527a/gitdb/__init__.py#L15-L18):\r\n\r\n```python\r\n    if 'PYOXIDIZER' not in os.environ:\r\n        where = os.path.join(os.path.dirname(__file__), 'ext', 'smmap')\r\n        if os.path.exists(where):\r\n            sys.path.append(where)\r\n```\r\n\r\nSo we could move the import of exception types below the `_init_externals` call, but also weaken the logic in `_init_externals` to match the behavior of the corresponding logic in `gitdb`.\r\n\r\nBut I recommend against this, too. I'd much rather do the simpler and, in my opinion, more useful thing of eliminating this logic altogether. I think this is the best approach even if no corresponding change is made in `gitdb`, but that the same thing should be done in `gitdb`, and for the same reason.\r\n\r\n#### Removing the `_init_externals` logic and documenting editable installation of dependencies\r\n\r\nI recommend `_init_externals` be removed, and in the infrequent (but plausible) case that one wants local changes to the `gitdb` git-submodule to be used when the local GitPython code is run, one can install the `gitdb` dependency by making an editable install with the git-submodule path, rather than allowing `pip` to automatically install `gitdb` from PyPI. (See related discussion in [gitdb#90](https://github.com/gitpython-developers/gitdb/pull/90) and [smmap#51](https://github.com/gitpython-developers/smmap/issues/51).)\r\n\r\nThat is, pass `-e git/ext/gitdb` in a `pip install` command. For example:\r\n\r\n```text\r\nek@Glub:~/repos-wsl/GitPython (main $=)$ python3.12 -m venv .venv\r\nek@Glub:~/repos-wsl/GitPython (main $=)$ . .venv/bin/activate\r\n(.venv) ek@Glub:~/repos-wsl/GitPython (main $=)$ python -m pip install -U pip wheel\r\n  ... (output omitted for brevity) ...\r\n(.venv) ek@Glub:~/repos-wsl/GitPython (main $=)$ pip install -e . -e git/ext/gitdb\r\nObtaining file:///home/ek/repos-wsl/GitPython\r\n  Installing build dependencies ... done\r\n  Checking if build backend supports build_editable ... done\r\n  Getting requirements to build editable ... done\r\n  Installing backend dependencies ... done\r\n  Preparing editable metadata (pyproject.toml) ... done\r\nObtaining file:///home/ek/repos-wsl/GitPython/git/ext/gitdb\r\n  Installing build dependencies ... done\r\n  Checking if build backend supports build_editable ... done\r\n  Getting requirements to build editable ... done\r\n  Preparing editable metadata (pyproject.toml) ... done\r\nCollecting smmap\u003c6,\u003e=3.0.1 (from gitdb==4.0.10)\r\n  Using cached smmap-5.0.1-py3-none-any.whl.metadata (4.3 kB)\r\nUsing cached smmap-5.0.1-py3-none-any.whl (24 kB)\r\nBuilding wheels for collected packages: GitPython, gitdb\r\n  Building editable for GitPython (pyproject.toml) ... done\r\n  Created wheel for GitPython: filename=GitPython-3.1.40-0.editable-py3-none-any.whl size=9794 sha256=3d31f9b0a44f58d64b73fa78ec5d681d1265def380a44b7725ebc8e887e78782\r\n  Stored in directory: /tmp/pip-ephem-wheel-cache-sy2luyhg/wheels/27/35/44/18948e9bc28191247cebeb25bb595b99e7c611bdd848ca7b24\r\n  Building editable for gitdb (pyproject.toml) ... done\r\n  Created wheel for gitdb: filename=gitdb-4.0.10-0.editable-py3-none-any.whl size=4306 sha256=6a0fba25581d6f08ea1c3396a87cc87f31e168f27a40a39c55938d3026cac811\r\n  Stored in directory: /tmp/pip-ephem-wheel-cache-sy2luyhg/wheels/33/53/45/0fe45259258a26e9d0b60a71ea239d1a2e7d01a8eaadee912e\r\nSuccessfully built GitPython gitdb\r\nInstalling collected packages: smmap, gitdb, GitPython\r\nSuccessfully installed GitPython-3.1.40 gitdb-4.0.10 smmap-5.0.1\r\n```\r\n\r\n```text\r\n(.venv) ek@Glub:~/repos-wsl/GitPython (main $=)$ python\r\nPython 3.12.0 (main, Oct  2 2023, 15:04:50) [GCC 11.4.0] on linux\r\nType \"help\", \"copyright\", \"credits\" or \"license\" for more information.\r\n\u003e\u003e\u003e import gitdb\r\n\u003e\u003e\u003e gitdb.__file__\r\n'/home/ek/repos-wsl/GitPython/git/ext/gitdb/gitdb/__init__.py'\r\n```\r\n\r\nMore likely than `pip install -e . -e git/ext/gitdb` is that one would want to use `pip install -e \".[test]\" -e git/ext/gitdb`, which I have also verified works. But I've shown the simpler command because the optional dependencies from the `test` extra make the output quite long.\r\n\r\nThis can be done in a separate `pip install` command, either before or after `.` is installed:\r\n\r\n- If `pip install -e git/ext/gitdb` is run first, then `pip install -e .` or `pip install -e .[test]` will use the editably installed git-submodule version of `gitdb`, because the dependency is satisfied already (at least unless GitPython declares a version range for the gitdb dependency that does not include the version in the git-submodule, which I have not tested).\r\n- If `pip install -e git/ext/gitdb` is run second, then because the editable install from that path was given explicitly, `pip` will uninstall the version from PyPI (that `pip install -e .` installed to satisfy the dependency) and perform the editable install.\r\n\r\nSo it works in one or two commands and in any order.\r\n\r\nOf course, if one *also* wants an editable install of `smmap` from the recursive submodule to be used, then one must tell `pip` to do an editable install from its path as well.\r\n\r\nHaving developers who want the git-submodule versions of gitdb and/or smmap to be used just install them editably has the additional advantage that static type checkers, at least if installed in the same environment or otherwise aware of what is installed in it (which they should be), could know that the git-submodule version is being used when it is. (This relates to the topic of #1716, though there is no order dependency for fixing these issues.)\r\n\r\n### In conclusion...\r\n\r\nI recommend that `_init_externals` be removed, imports be regrouped and reordered in a way that makes it easier to understand and edit them, and the technique of making editable installs of `gitdb` and/or `smmap` dependencies from git-submodules be documented somewhere in GitPython's documentation as something that may *occasionally* be useful when working on GitPython and its dependencies together.\r\n\r\nSince this would still not commonly be done, I'm not sure it really belongs in `README.md`, and perhaps it should only go in documentation in `doc/`. However, that documentation is out of date with respect to installation and development practices, so if this bug is to be fixed before the documentation in `doc/` is revised and updated, then it may make sense to add the information about this to `README.md`, even if it will be moved out of `README.md` later. Alternatively, documentation could be omitted until then, or it could be noted in comments in `git/__init__.py` and not otherwise documented, or fixing this could be put off altogether until the `doc/` documentation is revised and updated (though I would least prefer that option).","author":{"url":"https://github.com/EliahKagan","@type":"Person","name":"EliahKagan"},"datePublished":"2023-10-20T05:13:36.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":2},"url":"https://github.com/1717/GitPython/issues/1717"}