Skip to content

Commit

Permalink
Replace GitPython with pygit2
Browse files Browse the repository at this point in the history
Replace the use of GitPython package with pygit2.  The latter seems
to have better git support, in particular it supports the newer index
versions 3 and 4.  Since it is backed by the libgit2 library that is
also used by Cargo, it seems to have the best chances of being updated
for compatibility with new git versions.

Admittedly, the API feels very low-level.  In particular, it is
necessary to explicitly request writing changes to index back,
and explicitly reread it when it's modified externally (e.g. via another
`pygit2.Repository` instance, as in tests).  On the plus side, it does
not invoke `git` at all -- everything is done by the library.

Fixes conda-forge#2116
  • Loading branch information
mgorny committed Nov 6, 2024
1 parent 973c922 commit a450f1a
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 50 deletions.
37 changes: 24 additions & 13 deletions conda_smithy/feedstock_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,21 @@
def get_repo(path, search_parent_directories=True):
repo = None
try:
import git
import pygit2

repo = git.Repo(
path, search_parent_directories=search_parent_directories
)
if search_parent_directories:
path = pygit2.discover_repository(path)
if path is not None:
repo = pygit2.Repository(path, pygit2.enums.RepositoryOpenFlag.NO_SEARCH)
except ImportError:
pass
except git.InvalidGitRepositoryError:
pass

return repo


def get_repo_root(path):
try:
return get_repo(path).working_tree_dir
return get_repo(path).workdir.rstrip(os.path.sep)
except AttributeError:
return None

Expand All @@ -32,8 +31,13 @@ def set_exe_file(filename, set_exe=True):

repo = get_repo(filename)
if repo:
mode = "+x" if set_exe else "-x"
repo.git.execute(["git", "update-index", f"--chmod={mode}", filename])
index_entry = repo.index[os.path.relpath(filename, repo.workdir)]
if set_exe:
index_entry.mode |= all_execute_permissions
else:
index_entry.mode &= ~all_execute_permissions
repo.index.add(index_entry)
repo.index.write()

mode = os.stat(filename).st_mode
if set_exe:
Expand All @@ -54,7 +58,8 @@ def write_file(filename):

repo = get_repo(filename)
if repo:
repo.index.add([filename])
repo.index.add(os.path.relpath(filename, repo.workdir))
repo.index.write()


def touch_file(filename):
Expand All @@ -68,7 +73,8 @@ def remove_file_or_dir(filename):

repo = get_repo(filename)
if repo:
repo.index.remove([filename], r=True)
repo.index.remove_all([f"filename/**"])
repo.index.write()
shutil.rmtree(filename)


Expand All @@ -77,7 +83,11 @@ def remove_file(filename):

repo = get_repo(filename)
if repo:
repo.index.remove([filename])
try:
repo.index.remove(os.path.relpath(filename, repo.workdir))
repo.index.write()
except IOError: # this is specifically "file not in index"
pass

os.remove(filename)

Expand Down Expand Up @@ -106,4 +116,5 @@ def copy_file(src, dst):

repo = get_repo(dst)
if repo:
repo.index.add([dst])
repo.index.add(os.path.relpath(dst, repo.workdir))
repo.index.write()
1 change: 1 addition & 0 deletions environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ dependencies:
- requests
- pycryptodome
- gitpython
- pygit2
- pygithub >=2,<3
- ruamel.yaml
- conda-forge-pinning
Expand Down
78 changes: 41 additions & 37 deletions tests/test_feedstock_io.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
import tempfile
import unittest

import git
from git.index.typ import BlobFilter
import pygit2

import conda_smithy.feedstock_io as fio

Expand All @@ -27,7 +26,7 @@ def parameterize():
]:
for get_repo in [
lambda tmp_dir: None,
lambda tmp_dir: git.Repo.init(tmp_dir),
lambda tmp_dir: pygit2.init_repository(tmp_dir),
]:
try:
tmp_dir = tempfile.mkdtemp()
Expand Down Expand Up @@ -64,7 +63,7 @@ def test_repo(self):
self.assertTrue(fio.get_repo(pathfunc(tmp_dir)) is None)
else:
self.assertIsInstance(
fio.get_repo(pathfunc(tmp_dir)), git.Repo
fio.get_repo(pathfunc(tmp_dir)), pygit2.Repository
)
possible_repo_subdir = os.path.join(
tmp_dir,
Expand All @@ -85,34 +84,34 @@ def test_set_exe_file(self):

for set_exe in [True, False]:
for tmp_dir, repo, pathfunc in parameterize():
filename = "test.txt"
filename = os.path.join(tmp_dir, filename)
basename = "test.txt"
filename = os.path.join(tmp_dir, basename)
with open(filename, "w", encoding="utf-8", newline="\n") as fh:
fh.write("")
if repo is not None:
repo.index.add([filename])
repo.index.add(basename)
repo.index.write()

fio.set_exe_file(pathfunc(filename), set_exe)

file_mode = os.stat(filename).st_mode
self.assertEqual(file_mode & set_mode, int(set_exe) * set_mode)
if repo is not None:
blob = next(repo.index.iter_blobs(BlobFilter(filename)))[1]
repo.index.read()
blob = repo.index[basename]
self.assertEqual(
blob.mode & set_mode, int(set_exe) * set_mode
)

def test_write_file(self):
for tmp_dir, repo, pathfunc in parameterize():
for filename in ["test.txt", "dir1/dir2/test.txt"]:
filename = os.path.join(tmp_dir, filename)
for basename in ["test.txt", "dir1/dir2/test.txt"]:
filename = os.path.join(tmp_dir, basename)

write_text = "text"

with fio.write_file(pathfunc(filename)) as fh:
fh.write(write_text)
if repo is not None:
repo.index.add([filename])

read_text = ""
with open(filename, encoding="utf-8") as fh:
Expand All @@ -121,15 +120,16 @@ def test_write_file(self):
self.assertEqual(write_text, read_text)

if repo is not None:
blob = next(repo.index.iter_blobs(BlobFilter(filename)))[1]
read_text = blob.data_stream[3].read().decode("utf-8")
repo.index.read()
blob = repo.index[basename]
read_text = repo[blob.id].data.decode("utf-8")

self.assertEqual(write_text, read_text)

def test_touch_file(self):
for tmp_dir, repo, pathfunc in parameterize():
for filename in ["test.txt", "dir1/dir2/test.txt"]:
filename = os.path.join(tmp_dir, filename)
for basename in ["test.txt", "dir1/dir2/test.txt"]:
filename = os.path.join(tmp_dir, basename)

fio.touch_file(pathfunc(filename))

Expand All @@ -140,32 +140,34 @@ def test_touch_file(self):
self.assertEqual("", read_text)

if repo is not None:
blob = next(repo.index.iter_blobs(BlobFilter(filename)))[1]
read_text = blob.data_stream[3].read().decode("utf-8")
repo.index.read()
blob = repo.index[basename]
read_bytes = repo[blob.id].data

self.assertEqual("", read_text)
self.assertEqual(b"", read_bytes)

def test_remove_file(self):
for tmp_dir, repo, pathfunc in parameterize():
for filename in ["test.txt", "dir1/dir2/test.txt"]:
dirname = os.path.dirname(filename)
for basename in ["test.txt", "dir1/dir2/test.txt"]:
dirname = os.path.dirname(basename)
if dirname and not os.path.exists(dirname):
os.makedirs(dirname)

filename = os.path.join(tmp_dir, filename)
filename = os.path.join(tmp_dir, basename)

with open(filename, "w", encoding="utf-8", newline="\n") as fh:
fh.write("")
if repo is not None:
repo.index.add([filename])
repo.index.add(basename)
repo.index.write()

self.assertTrue(os.path.exists(filename))
if dirname:
self.assertTrue(os.path.exists(dirname))
self.assertTrue(os.path.exists(os.path.dirname(dirname)))
if repo is not None:
self.assertTrue(
list(repo.index.iter_blobs(BlobFilter(filename)))
self.assertIsNotNone(
repo.index[basename]
)

fio.remove_file(pathfunc(filename))
Expand All @@ -175,17 +177,18 @@ def test_remove_file(self):
self.assertFalse(os.path.exists(dirname))
self.assertFalse(os.path.exists(os.path.dirname(dirname)))
if repo is not None:
self.assertFalse(
list(repo.index.iter_blobs(BlobFilter(filename)))
repo.index.read()
self.assertRaises(
KeyError, lambda: repo.index[basename]
)

def test_copy_file(self):
for tmp_dir, repo, pathfunc in parameterize():
filename1 = "test1.txt"
filename2 = "test2.txt"
basename1 = "test1.txt"
basename2 = "test2.txt"

filename1 = os.path.join(tmp_dir, filename1)
filename2 = os.path.join(tmp_dir, filename2)
filename1 = os.path.join(tmp_dir, basename1)
filename2 = os.path.join(tmp_dir, basename2)

write_text = "text"
with open(filename1, "w", encoding="utf-8", newline="\n") as fh:
Expand All @@ -194,17 +197,18 @@ def test_copy_file(self):
self.assertTrue(os.path.exists(filename1))
self.assertFalse(os.path.exists(filename2))
if repo is not None:
self.assertFalse(
list(repo.index.iter_blobs(BlobFilter(filename2)))
self.assertRaises(
KeyError, lambda: repo.index[basename2]
)

fio.copy_file(pathfunc(filename1), pathfunc(filename2))

self.assertTrue(os.path.exists(filename1))
self.assertTrue(os.path.exists(filename2))
if repo is not None:
self.assertTrue(
list(repo.index.iter_blobs(BlobFilter(filename2)))
repo.index.read()
self.assertIsNotNone(
repo.index[basename2]
)

read_text = ""
Expand All @@ -214,8 +218,8 @@ def test_copy_file(self):
self.assertEqual(write_text, read_text)

if repo is not None:
blob = next(repo.index.iter_blobs(BlobFilter(filename2)))[1]
read_text = blob.data_stream[3].read().decode("utf-8")
blob = repo.index[basename2]
read_text = repo[blob.id].data.decode("utf-8")

self.assertEqual(write_text, read_text)

Expand Down

0 comments on commit a450f1a

Please sign in to comment.