Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

We appear to import the submodule version of gitdb (but never do) #1717

Closed
@EliahKagan

Description

@EliahKagan

Summary

git/__init__.py imports exceptions indirectly fromgitdb before it adjustssys.path, causing the code from thegitdb git-submodule not to be used under any circumstances.

Although this could probably be fixed by movinggitdb.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 insys.path could simply be removed.

Details

The unit tests use the directgitdb git-submodule as well as the indirectsmmap git-submodule through it (and for this reason the git-submodules cannot be immediately removed). However,git/__init__.py alsoappears to use them by cauisnggitdb to be imported from thegitdb git-submodule under some circumstances:

__version__="git"
# { Initialization
def_init_externals()->None:
"""Initialize external projects by putting them into the path"""
if__version__=="git"and"PYOXIDIZER"notinos.environ:
sys.path.insert(1,osp.join(osp.dirname(__file__),"ext","gitdb"))
try:
importgitdb
exceptImportErrorase:
raiseImportError("'gitdb' could not be found in your PYTHONPATH")frome
# END verify import
# } END initialization
#################
_init_externals()
#################

The intent is that, then, importing Python modules some of which containgitdb imports should use thegitdb Python module from thegitdb git-submodule. Furthermore, it is intended that this happen even if thegitdb package is installed, as evidenced by how thegit/ext/gitdb directory is inserted near the beginning ofsys.path. The idea is that the git-submodule version ofgitdb would be used in these immediately subsequent imports:

# { Imports
try:
fromgit.configimportGitConfigParser# @NoMove @IgnorePep8
fromgit.objectsimport*# @NoMove @IgnorePep8
fromgit.refsimport*# @NoMove @IgnorePep8
fromgit.diffimport*# @NoMove @IgnorePep8
fromgit.dbimport*# @NoMove @IgnorePep8
fromgit.cmdimportGit# @NoMove @IgnorePep8
fromgit.repoimportRepo# @NoMove @IgnorePep8
fromgit.remoteimport*# @NoMove @IgnorePep8
fromgit.indeximport*# @NoMove @IgnorePep8
fromgit.utilimport (# @NoMove @IgnorePep8
LockFile,
BlockingLockFile,
Stats,
Actor,
rmtree,
)
exceptGitErroras_exc:
raiseImportError("%s: %s"% (_exc.__class__.__name__,_exc))from_exc
# } END imports

However, that does not happen. The code from thegitdb git-submodule is actuallynever used, because beforeany of that, we importgit.exc:

fromgit.excimport*# @NoMove @IgnorePep8

git.exc imports from thegitdb Python module:

fromgitdb.excimport (# noqa: @UnusedImport
AmbiguousObjectName,
BadName,
BadObject,
BadObjectType,
InvalidDBRoot,
ODBError,
ParseError,
UnsupportedOperation,
to_hex_sha,
)

Although that code was touched in#1659, the change there did not affect this behavior in any way. Furthermore, the imports ingit.exc fromgitdb.exc are not the only imports ingit.exc that causegitdb to be imported. For example,git.exc also imports fromgit.util, which imports fromgitdb.util.

Because thefrom git.exc import * importsgitdb beforesys.path is modified, the installedgitdb dependency is used, rather than the git-submodule version ofgitdb. Subsequent imports reuse the already importedgitdb module. That the installedgitdb is being used can be verified in a REPL by importinggit and thengitdb and checkinggitdb.__file__:

>>>importgit>>>importgitdb>>>gitdb.__file__'/home/ek/repos-wsl/GitPython/.venv/lib/python3.12/site-packages/gitdb/__init__.py'

It is usinggitdb from the environment'ssite-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 importsgitdb:

>>>importgit>>>importsys>>>sys.modules["gitdb"]<module'gitdb'from'/home/ek/repos-wsl/GitPython/.venv/lib/python3.12/site-packages/gitdb/__init__.py'>

sys.path was modified, just too late to have had an effect. Checking it in the same REPL after importinggit reveals:

>>>sys.path['','/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']

Possible solutions

Restoring the behavior of using the git-submodule dependency (not my recommendation)

Because the actual change tosys.path does take effect, if it were added before anything were imported fromgitdb then it would have worked. This experiment, done in a new REPL, verifies this:

>>>importsys>>>sys.path.insert(1,"/home/ek/repos-wsl/GitPython/git/ext/gitdb")>>>importgit>>>sys.modules["gitdb"]<module'gitdb'from'/home/ek/repos-wsl/GitPython/git/ext/gitdb/gitdb/__init__.py'>

It was probably working prior to6752fad, whichadded that import fromgitdb.exc. So moving thefrom git.exc import * line below the_init_externals() call would probably be sufficient to cause thegitdb git-submodule to be used, and used under the exact circumstances under which it was intended, and currently wrongly appears, to be used.

But I recommend against that.6752fad was part of#1229, which was mergedwell over two years ago. At least if this issue has not been discovered before now, then importinggitdb from the git-submodule (which is relevant, after all, only to peopledeveloping GitPython) is not something anyone is relying on as thedefault behavior.

Restoring that behavior but in a weakened form (not my recommendation)

gitdb contains similar code, for itssmmap dependency. That code ingitdb does work, because there is no direct or indirect import ofsmmap before it runs. However, its logic is different. Rather than inserting thesmmap git-submodule directory near the front of the pathas GitPython does...

if__version__=="git"and"PYOXIDIZER"notinos.environ:sys.path.insert(1,osp.join(osp.dirname(__file__),"ext","gitdb"))

...it instead inserts itat the end, where a package installed any other way would take precedence:

if'PYOXIDIZER'notinos.environ:where=os.path.join(os.path.dirname(__file__),'ext','smmap')ifos.path.exists(where):sys.path.append(where)

So 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 ingitdb.

But 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 ingitdb, but that the same thing should be done ingitdb, and for the same reason.

Removing the_init_externals logic and documenting editable installation of dependencies

I recommend_init_externals be removed, and in the infrequent (but plausible) case that one wants local changes to thegitdb git-submodule to be used when the local GitPython code is run, one can install thegitdb dependency by making an editable install with the git-submodule path, rather than allowingpip to automatically installgitdb from PyPI. (See related discussion ingitdb#90 andsmmap#51.)

That is, pass-e git/ext/gitdb in apip install command. For example:

ek@Glub:~/repos-wsl/GitPython (main $=)$ python3.12 -m venv .venvek@Glub:~/repos-wsl/GitPython (main $=)$ . .venv/bin/activate(.venv) ek@Glub:~/repos-wsl/GitPython (main $=)$ python -m pip install -U pip wheel  ... (output omitted for brevity) ...(.venv) ek@Glub:~/repos-wsl/GitPython (main $=)$ pip install -e . -e git/ext/gitdbObtaining file:///home/ek/repos-wsl/GitPython  Installing build dependencies ... done  Checking if build backend supports build_editable ... done  Getting requirements to build editable ... done  Installing backend dependencies ... done  Preparing editable metadata (pyproject.toml) ... doneObtaining file:///home/ek/repos-wsl/GitPython/git/ext/gitdb  Installing build dependencies ... done  Checking if build backend supports build_editable ... done  Getting requirements to build editable ... done  Preparing editable metadata (pyproject.toml) ... doneCollecting smmap<6,>=3.0.1 (from gitdb==4.0.10)  Using cached smmap-5.0.1-py3-none-any.whl.metadata (4.3 kB)Using cached smmap-5.0.1-py3-none-any.whl (24 kB)Building wheels for collected packages: GitPython, gitdb  Building editable for GitPython (pyproject.toml) ... done  Created wheel for GitPython: filename=GitPython-3.1.40-0.editable-py3-none-any.whl size=9794 sha256=3d31f9b0a44f58d64b73fa78ec5d681d1265def380a44b7725ebc8e887e78782  Stored in directory: /tmp/pip-ephem-wheel-cache-sy2luyhg/wheels/27/35/44/18948e9bc28191247cebeb25bb595b99e7c611bdd848ca7b24  Building editable for gitdb (pyproject.toml) ... done  Created wheel for gitdb: filename=gitdb-4.0.10-0.editable-py3-none-any.whl size=4306 sha256=6a0fba25581d6f08ea1c3396a87cc87f31e168f27a40a39c55938d3026cac811  Stored in directory: /tmp/pip-ephem-wheel-cache-sy2luyhg/wheels/33/53/45/0fe45259258a26e9d0b60a71ea239d1a2e7d01a8eaadee912eSuccessfully built GitPython gitdbInstalling collected packages: smmap, gitdb, GitPythonSuccessfully installed GitPython-3.1.40 gitdb-4.0.10 smmap-5.0.1
(.venv) ek@Glub:~/repos-wsl/GitPython (main $=)$ pythonPython 3.12.0 (main, Oct  2 2023, 15:04:50) [GCC 11.4.0] on linuxType "help", "copyright", "credits" or "license" for more information.>>> import gitdb>>> gitdb.__file__'/home/ek/repos-wsl/GitPython/git/ext/gitdb/gitdb/__init__.py'

More likely thanpip install -e . -e git/ext/gitdb is that one would want to usepip 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 thetest extra make the output quite long.

This can be done in a separatepip install command, either before or after. is installed:

  • Ifpip install -e git/ext/gitdb is run first, thenpip install -e . orpip install -e .[test] will use the editably installed git-submodule version ofgitdb, 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).
  • Ifpip 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 (thatpip install -e . installed to satisfy the dependency) and perform the editable install.

So it works in one or two commands and in any order.

Of course, if onealso wants an editable install ofsmmap from the recursive submodule to be used, then one must tellpip to do an editable install from its path as well.

Having 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.)

In conclusion...

I 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 ofgitdb and/orsmmap dependencies from git-submodules be documented somewhere in GitPython's documentation as something that mayoccasionally be useful when working on GitPython and its dependencies together.

Since this would still not commonly be done, I'm not sure it really belongs inREADME.md, and perhaps it should only go in documentation indoc/. 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 indoc/ is revised and updated, then it may make sense to add the information about this toREADME.md, even if it will be moved out ofREADME.md later. Alternatively, documentation could be omitted until then, or it could be noted in comments ingit/__init__.py and not otherwise documented, or fixing this could be put off altogether until thedoc/ documentation is revised and updated (though I would least prefer that option).

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions


      [8]ページ先頭

      ©2009-2025 Movatter.jp