Stop using the internal git_worktree struct

[mystor]

Currently the worktree implementation in pygit2 directly re-declares and uses the internal git_worktree struct.

pygit2/src/worktree.h

Lines 36 to 43 in 8d6940f

	struct git_worktree {
	char *name;
	char *gitlink_path;
	char *gitdir_path;
	char *commondir_path;
	char *parent_path;
	int locked:1;
	};

It would be nice if we could avoid using this unexposed libgit2 data structure for stability reasons.

For example, building pygit2 against the libgit2 master branch succeeds, however causes silent & dangerous incorrect behaviour due to the definition of git_worktree in pygit2 no longer matching git_worktree in libgit2 (a new field, worktree_path, was added to git_worktree).

https://github.com/libgit2/libgit2/blob/e212011b9872c52f6205d3a30b10f753c3108918/src/worktree.h#L15-L35

Unfortunately, the fields which are currently exposed as the name, path, and git_path properties on Worktree objects are not public at all in the libgit2 API. My current thoughts about how to handle this are the following:

1. Change the name property to return git_worktree_name(self->worktree)

   no change in behaviour

2. Change the path property to return git_worktree_path(self->worktree)

   breaking change - git_worktree_path returns the 'worktree_path' rather than the current gitlink_path. worktree_path is currently ~$(basename $gitlink_path) - this usually results in a .git segment being removed from the end of the path.

3. Remove the git_path property, as it is not exposed in any way from libgit2.

   breaking change - property removal

This would make it fall more in line with the libgit2-exposed methods, which seems desirable. Alternatively, the required APIs could be added to libgit2 to maintain behaviour between releases.

[jdavid]

Thanks @mystor, you're right.
Breaking changes are ok, removing features not so much.

@rmedaer @nhynes @terminalmage
What do you think?

[nhynes]

👍 agreed. A stable and maintainable API is worth the breaking changes. Since git worktree is a less used feature, the impact should be minimal.

[terminalmage]

Makes sense, relying on non-public attributes is a recipe for disaster. As long as this is in the release notes, it should be fine.

[rmedaer]

Agreed too ! No problem with breaking changes but as @jdavid said: we still have to support the feature.

[jdavid]

Done. If this change bothers you, in particular the removal of workdir.git_path, you still have a chance to speak before the next release.