Add support for paths in checkout arguments

[rbistolfi]

Hi there, would be possible to add support for paths to repository.checkout and friends?
https://libgit2.org/docs/guides/101-samples/#checkout_paths

Thanks!

[imbuedhope]

Fairly certain passing directory= to Repository.checkout does just that.

See: https://www.pygit2.org/working-copy.html?highlight=checkout#pygit2.Repository.checkout

[rbistolfi]

Hi @imbuedhope

I think that's different, I am looking for the equivalent to git checkout ... -- path/to/a path/to/b. The directory option seems to create a directory and checking out there instead. In other words, it is using https://github.com/libgit2/libgit2/blob/master/include/git2/checkout.h#L285 when I need https://github.com/libgit2/libgit2/blob/master/include/git2/checkout.h#L272

[imbuedhope]

Ah, yeah that does seem to be the case (in repository.py).

[rbistolfi]

I tried passing a StrArray.array to copts.paths but FFI complains about it, it requires a list or dict, if I provide a dict I get a segmentation fault. Sorry I am new to FFI, any ideas about how to correctly initialize copts.paths?

[imbuedhope]

It needs to be of the type git_strarray * from the C libgit2 bindings see the api docs for libgit2 or the source.

You'll have to make the struct with FFI first and then pass it as the value for copts.paths I think (not 100% certain, I haven't messed with FFI much either).

[rbistolfi]

Yep, it seems to be the right type:

index e609687..cc1fded 100644
--- a/pygit2/repository.py
+++ b/pygit2/repository.py
@@ -54,7 +54,7 @@ from .ffi import ffi, C
 from .index import Index
 from .remote import RemoteCollection
 from .blame import Blame
-from .utils import to_bytes, is_string
+from .utils import to_bytes, is_string, StrArray
 from .submodule import Submodule
 
 
@@ -204,7 +204,7 @@ class BaseRepository(_Repository):
     # Checkout
     #
     @staticmethod
-    def _checkout_args_to_options(strategy=None, directory=None):
+    def _checkout_args_to_options(strategy=None, directory=None, paths=None):
         # Create the options struct to pass
         copts = ffi.new('git_checkout_options *')
         check_error(C.git_checkout_init_options(copts, 1))
@@ -223,6 +223,10 @@ class BaseRepository(_Repository):
             refs.append(target_dir)
             copts.target_directory = target_dir
 
+        if paths:
+            strarray = StrArray(paths)
+            copts.paths = strarray.array
+
         return copts, refs
 
     def checkout_head(self, **kwargs):
diff --git a/test/test_repository.py b/test/test_repository.py
index 523a907..f87c6dd 100644
--- a/test/test_repository.py
+++ b/test/test_repository.py
@@ -332,6 +332,10 @@ class RepositoryTest_II(utils.RepoTestCase):
         self.repo.checkout(ref_i18n, directory=extra_dir)
         assert not len(os.listdir(extra_dir)) == 0
 
+    def test_checkout_paths(self):
+        ref_i18n = self.repo.lookup_reference('refs/heads/i18n')
+        self.repo.checkout(ref_i18n, paths=['README.rst'])
+
     def test_merge_base(self):
         commit = self.repo.merge_base(
             '5ebeeebb320790caf276b9fc8b24546d63316533',

But the test raises:

        if paths:
            strarray = StrArray(paths)
>           copts.paths = strarray.array
E           TypeError: initializer for ctype 'git_strarray' must be a list or tuple or dict or struct-cdata, not cdata 'git_strarray *'

[imbuedhope]

It looks like git_checkout_options.paths is of type git_strarray, while StrArray.array is a git_strarray *.

Try the following instead?

 if paths:
    strarray = StrArray(paths)
    copts.paths = strarray.array[0]

[rbistolfi]

That seems to work sometimes. I get segmentation faults in some runs. I will try to make time for properly investigating the requirements for getting this to work consistently. I will start looking for some examples for safe use of StrArray in the codebase. Thanks a lot for your help.

[imbuedhope]

Looks like the ffi variable will be cleaned up by python when the last reference to it vanishes based on the ffi documentation here.

Since copts is used after _checkout_args_to_options returns the grabage collector and libgit2 are in a race to use the variable, which is probably what causes the segfault.

Probably just need to add it to the refs list that is present in the same function so it doesn't go out of scope before the C bindings are called. So something like this should resolve it, I think.

if paths:
    strarray = StrArray(paths)
    copts.paths = strarray.array[0]
    refs.append(strarray)

[rbistolfi]

Good catch, thanks! I will give it a try. I will focus in correctness now, creating a more comprehensive test case.

[rbistolfi]

I made some progress. Appending strarray to refs seems to have worked. I also improved the behavior a bit: If paths is provided, HEAD is not updated to the given reference, just like the cli tool. Test now asserts that the path checked out has status GIT_STATUS_INDEX_NEW.

[rbistolfi]

Created #859

[rbistolfi]

PR has been merged, this is now fixed. Thanks @imbuedhope for your help!