Julia: LibGit2 refactor

[simonbyrne]

The LibGit2 module could do with some work. Some possible changes

• [ ] functions should mirror CLI (command line interface) git functionality when possible
  
  
  
  • e.g. LibGit2.checkout instead of LibGit2.branch!
  
  
  • possible exceptions when CLI does multiple things in one go
  
  
• [ ] consistent use of ! suffix
• [ ] use Enum and FlagEnum (pending #19470) for constants whenever possible
• [ ] don't swallow errors:
  
  
  
  • e.g. rebase! will silently abort if it fails
  
  
  • revparse as well
  
  
• [ ] Normalize and check path inputs, ref #18724.
• [ ] Clean up a few objects which allocate memory in a confusing way:
  
  
  
  • StrArrayStruct: manually calls Libc.malloc, but then frees via LibGit2 git_strarray_free
  
  
  • Buffer: is an immutable object, but needs a finalizer
  
  
  • SignatureStruct, GitSignature, Signature objects should be simplified
  
  
• [x] split out LibGit2.get into more usefully-named functions
  
  
  
  • Maybe via constructors? e.g. GitBlob(repo::GitRepo, hash::GitHash)
  
  
  • when returning a GitObject, automatically resolve its type via git_object_type function
  
  
  • LibGit2.get(T, repo, oid) should throw an error (TypeError?) if incorrect type T is used.
  
  
• [ ] Make GitConfig objects act like an Associative{String,String} (i.e. overload getindex/setindex!)
• [x] rename GitAnyObject to GitUnknownObject #19935
• [x] rename Oid to GitHash #19878
• [x] Different types for full GitHash and short hashes: see https://github.com/JuliaLang/julia/pull/19878#issuecomment-270714556
• [ ] More sensible errors and return values
  
  
  
  • [ ] LibGit2.fetch should return nothing (since it will throw an error for any other value)
  
  
  • [x] LibGit2.reset! and rebase should return current HEAD commit
  
  
  • [x] LibGit2.upstream and LibGit2.lookup_branch should throw errors if not found, instead of returning nothing
  
  
  • [ ] Don't use "zero" GitHash objects as null values.
  
  
• [x] Support newbase option for rebase! (cf https://github.com/JuliaLang/julia/pull/19651#discussion_r94463640)
• [ ] Use correct iteration protocol for Base.next(::LibGit2.Rebase) (or until new iteration protocol is in place)
• [x] Combine LibGit2.owner and LibGit2.repository
• [ ] Get rid of a lot of the with and explicit finalize/close calls (pending #19660)
• [ ] Sensible show methods for various objects
  
  
  
  • [x] GitBlob
  
  
  • [x] GitCommit
  
  
  • [x] GitRemote
  
  
  • [x] GitSignature
  
  
  • [x] GitTag
  
  
  • [x] GitTreeEntry
  
  
  • [x] GitReference
  
  
  • [x] GitIndex
  
  
  • [x] DiffDelta
  
  
  • [x] DiffFile
  
  
  • [x] FetchHead
  
  
  • [x] GitTree
  
  
  • [x] GitDiff
  
  
  • [ ] GitAnnotated
  
  
  • [ ] GitRebase
  
  
• [ ] Better documentation (#18810)
  
  
  
  • what the function/type does
  
  
  • arguments/output value
  
  
  • equivalent git CLI command where appropriate
  
  
• [ ] Better tests:
  
  
  
  • More coverage
  
  
  • Can test behaviour against command line Git
  
  
• [ ] Consistent handling of git_*_options structs
  
  
  
  • Correct handling of memory allocation of pointer fields
  
  
  • Callback interface
  
  
• [ ] Rename isdiff to a more descriptive name (https://github.com/JuliaLang/julia/pull/20155#discussion_r97181726)
• [ ] get rid of cat?
  
  
  
  • It's more or less a wrapper around content.
  
  
• [ ] document status/GitStatus return types and what results mean https://github.com/JuliaLang/julia/pull/20503#issuecomment-278394964

Some of these are breaking, so we may want to get the deprecations in place before 0.6.

[simonbyrne]

I should say that I would love some help with this if anyone is keen.

[pfitzseb]

Maybe add a

• Normalize and check path inputs, ref #18724.

item to that list.

[kshyatt]

Currently we have show for:

• Oid (soon to be Hash)
• GitError
• IndexEntry
• RebaseOperation

The IndexEntry and RebaseOperation ones are really barebones.

[simonbyrne]

One issue that has arisen a couple of times now (e.g. https://github.com/JuliaLang/PkgDev.jl/issues/101) is how to check if an object is in a repo, then do or not do something, e.g. in calls to GitBlob(repo, ...).

The old code would return nothing, which is probably not a good idea as (1) it's type unstable, and (2) it is returning a type of a different object than called by the constructor.

Two possible options, neither of which I'm particularly keen on:

1. Define a Nullable{GitBlob}(repo, ...) method:

nobj = Nullable{GitBlob}(repo, hash)
if !isnull(nobj)
   obj = get(nobj)
   ...
end

• could make this a function instead of a constructor (e.g. tryobject or something)?

1. Define an in method:

if hash in repo
   obj = GitBlob(repo, hash)
   ...
end

• how do we check if it's actually a GitBlob, and not say, a GitCommit?
• should we also overload getindex(repo, ...)?

Any better ideas? Or preferences on which is the least worst?

[tkelman]

always returning Nullable{result} for most of these (probably using a function name instead of the type constructor name as the exported API) is what I'd prefer

[simonbyrne]

any suggestions for such a name?

[kshyatt]

query_blob?

[kshyatt]

are_you_there_blob_its_me_margaret?

[kshyatt]

i_cant_believe_its_not_blobcedonia

[simonbyrne]

tryobject?

[omus]

functions should mirror CLI (command line interface) git functionality when possible

• e.g. LibGit2.checkout instead of LibGit2.branch!
• possible exceptions when CLI does multiple things in one go

Does this mean that LibGit2.checkout won't be able to create a branch? I would be ok with that.

[simonbyrne]

The other problem is that you often want to know the reason why the field is null, e.g. https://github.com/JuliaLang/julia/pull/20752/files#diff-82889f262e9bc3ab1b675224c253a830R163

What if we were to define a special GitNullable type?:

immutable GitNullable{T}
    code::Error.Code
    value::T
end

Base.isnull(x::GitNullable) = x.code != Error.GIT_OK

and we were to create lower-level "try" interfaces based on this?

[tkelman]

Not really a libgit2-specific concept, but would certainly be useful here. Ref https://github.com/iamed2/ResultTypes.jl

[simonbyrne]

Here's a confusing one that I came across in #20916: git checkout can do several different things:

1. change the working tree and index
2. switch branches, i.e. change HEAD reference to a commit or branch (if called without a file path or without the --patch option)
3. create a new branch (if called with the -b/-B option)

On the other hand, the libgit2 git_checkout_* functions only do 1 (and available in Julia via, e.g. checkout_tree). At the moment, we provide functionality for 1 & 2 via checkout! and 1,2 & 3 via branch!.

Too late for 0.6, but it would be nice to have a better and more consistent way to handle this.

[StefanKarpinski]

I think the trying to emulate the git CLI one-on-one is not the best idea – the ways that it's overloaded are confusing and hard to use, IMO. Probably better to have a cleaner API that's more like an easy-to-use version of the libgit2 API.

[simonbyrne]

Another one: in a couple of places it is useful to immediately peel an object obtained from a GitHash or refspec (i.e. to get the GitTree underlying a GitCommit).

Two options:

1. A keyword arg to the constructors, e.g.:

GitTree(repo, hash, peel=true)

1. Allow a hash or refspec arg to peel, e.g.:

peel(GitTree, repo, hash)