worktree: add ability to create worktree with pre-existing branch

[pks-t]

Currently, we always create a new branch after the new worktree's name
when creating a worktree. In some workflows, though, the caller may want
to check out an already existing reference instead of creating a new
one, which is impossible to do right now.

Add a new option ref to the options structure for adding worktrees. In
case it is set, a branch and not already checked out by another
worktree, we will re-use this reference instead of creating a new one.

[mminns]

My first comments on libgit2 code, apologies if I've misunderstood anything. :)

git worktree add allows using an existing branch name or commit, this code only supports branches? Should it add commit support now rather than adding that later?

[pks-t]

Even if you misunderstand anything that is a worthwhile comment, as it hints at things that are hard to understand and could be improved. So don't worry about that ;)

That being said, I didn't really want to allow using committish things here. The main problem is actually the interface. In case we allow either a hash or a reference, I'd have to make sure what it is the user passed to me by parsing it. Same for branches vs tags -- in case the user passed a tag to me, I need to make sure to not let HEAD point at the tag, but instead create a detached HEAD.

So, if you ask me, it creates quite some complexity for rather minor usecases. At least I think these usecases are minor as long as I'm not convinced otherwise by a convincing scenario. Alternatively, I'd also be happy to implement that if somebody has an idea how the interface could look like such that it is trivial to implement and easy to use.

[mminns]

To be honest I have never actually used a commit or tag. My personal usecase for worktrees is always checking out branches locally for PRs without disturbing my working copy. So this change helps me.

But I wonder how quickly requests would follow for tags/commits, perhaps it is enough to get there in incremental steps.

[pks-t]

Yeah, I definitly see where you're coming from. I mean I would already include the ability to use arbitrary commits, but I'm simply undecided on how that interface should look like.

So a long as nobody has any proposals I think I'll just stay with the current one for now and wait until people start complaining.

[mminns]

When/how does the version get bumped?

[pks-t]

The option versions shall get bumped whenever the structure changes, but this will only happen as soon as we have our first stable (1.0) release.

[mminns]

I have a general question about changing this struct/the version and breaking changes, really for my own education.

In libgit2sharp the shape of the _options structs is effectively fixed, e.g. for git_blame_options
see https://github.com/libgit2/libgit2sharp/blob/3f9b415fa1edfc31ce1ec2b4b3d18441c34adfff/LibGit2Sharp/Core/GitBlame.cs#L44

[StructLayout(LayoutKind.Sequential)]
internal class git_blame_options
{
    public uint version = 1;
    public GitBlameOptionFlags flags;

    public UInt16 min_match_characters;
    public git_oid newest_commit;
    public git_oid oldest_commit;
    public UIntPtr min_line;
    public UIntPtr max_line;
}

vs in libgit2

libgit2/include/git2/blame.h

Line 70 in 89c332e

typedef struct git_blame_options {

[StructLayout(LayoutKind.Sequential)]
internal class git_blame_options
{
    public uint version = 1;
    public GitBlameOptionFlags flags;

    public UInt16 min_match_characters;
    public git_oid newest_commit;
    public git_oid oldest_commit;
    public UIntPtr min_line;
    public UIntPtr max_line;
}

So the libgit2sharp code would need reworking if a new field was added to the struct in libgit2.

So my question is why is it ok to refactor the worktree options struct, without bumping the version, in libgit2 but not to add params to a method definition?

Why does adding a param to the method break the API but adding a field to the struct not break it?

Hope that makes sense. :)

[ethomson]

You're very much right in that both are breaking API changes. The way that we extend things now is (generally) through the options interface instead of params. In the future it will always be through the options interface plus incrementing the version number.

We haven't been very responsible here about stabilizing. We should do that soon.

[mminns]

Hi

Do we know if this is likely to be approved?

[pks-t]

@mminns: As I'm the only one really caring about worktrees in libgit2 this will most certainly get approved. Only question is how the API will look like exactly. And this definitly won't make it in for v0.27.0.

[ethomson]

However, since (I think) you're interested in LibGit2Sharp, we can pull this in for a prerelease there as soon as it's merged here.

[mminns]

Yes, I'm primarily interested in libgit2sharp.

[ethomson]

Thanks, as always, @pks-t