add GitShortHash type, remove some get methods (#20104)

* add GitShortHash type, remove some get methods
* improve docs
* add test for valid blob data

base/deprecated.jl

@@ -18,13 +18,13 @@
 # the name of the function, which is used to ensure that the deprecation warning
 # is only printed the first time for each call place.
 
-macro deprecate(old,new)
+macro deprecate(old,new,ex=true)
     meta = Expr(:meta, :noinline)
     if isa(old,Symbol)
         oldname = Expr(:quote,old)
         newname = Expr(:quote,new)
         Expr(:toplevel,
-            Expr(:export,esc(old)),
+            ex ? Expr(:export,esc(old)) : nothing,
             :(function $(esc(old))(args...)
                   $meta
                   depwarn(string($oldname," is deprecated, use ",$newname," instead."),
@@ -1555,8 +1555,6 @@ _promote_eltype_op(op, A, B, C, D...) = (@_inline_meta; _promote_eltype_op(op, e
     _promote_eltype_op(args...)
 end
 
-# Rename LibGit2.Oid to LibGit2.GitHash (part of #19839)
-eval(Base.LibGit2, :(Base.@deprecate_binding Oid GitHash))
 
 function unsafe_wrap(::Type{String}, p::Union{Ptr{UInt8},Ptr{Int8}}, len::Integer, own::Bool=false)
     Base.depwarn("unsafe_wrap(String, ...) is deprecated; use `unsafe_string` instead.", :unsafe_wrap)
@@ -1575,9 +1573,6 @@ unsafe_wrap(::Type{String}, p::Cstring, len::Integer, own::Bool=false) =
 @deprecate finalize(sa::LibGit2.StrArrayStruct) close(sa)
 @deprecate finalize(sa::LibGit2.Buffer) close(sa)
 
-# Rename LibGit2.GitAnyObject to LibGit2.GitUnknownObject (part of #19839)
-eval(LibGit2, :(Base.@deprecate_binding GitAnyObject GitUnknownObject))
-
 ## produce, consume, and task iteration
 # NOTE: When removing produce/consume, also remove field Task.consumers and related code in
 # task.jl and event.jl
@@ -1829,6 +1824,19 @@ function colon{T<:Dates.Period}(start::T, stop::T)
     colon(start, T(1), stop)
 end
 
+# LibGit2 refactor (#19839)
+eval(Base.LibGit2, quote
+     Base.@deprecate_binding Oid GitHash
+     Base.@deprecate_binding GitAnyObject GitUnknownObject
+
+     @deprecate owner(x) repository(x) false
+     @deprecate get{T<:GitObject}(::Type{T}, repo::GitRepo, x) T(repo, x) false
+     @deprecate get{T<:GitObject}(::Type{T}, repo::GitRepo, oid::GitHash, oid_size::Int) T(repo, GitShortHash(oid, oid_size)) false
+     @deprecate revparse(repo::GitRepo, objname::AbstractString) GitObject(repo, objname) false
+     @deprecate object(repo::GitRepo, te::GitTreeEntry) GitObject(repo, te) false
+     @deprecate commit(ann::GitAnnotated) GitHash(ann) false
+end)
+
 # when this deprecation is deleted, remove all calls to it, and all
 # negate=nothing keyword arguments, from base/dates/adjusters.jl
 eval(Dates, quote

base/libgit2/blob.jl

@@ -1,13 +1,20 @@
 # This file is a part of Julia. License is MIT: http://julialang.org/license
 
-function content(blob::GitBlob)
-    return ccall((:git_blob_rawcontent, :libgit2), Ptr{Void}, (Ptr{Void},), blob.ptr)
-end
-
 function Base.length(blob::GitBlob)
     return ccall((:git_blob_rawsize, :libgit2), Int64, (Ptr{Void},), blob.ptr)
 end
 
+function rawcontent(blob::GitBlob)
+    ptr = ccall((:git_blob_rawcontent, :libgit2), Ptr{UInt8}, (Ptr{Void},), blob.ptr)
+    copy(unsafe_wrap(Array, ptr, (length(blob),), false))
+end
+
+function content(blob::GitBlob)
+    s = String(rawcontent(blob))
+    isvalid(s) || error("Blob does not contain valid UTF-8 data")
+    return s
+end
+
 """
 Use a heuristic to guess if a file is binary: searching for NULL bytes and
 looking for a reasonable ratio of printable to non-printable characters among
@@ -26,24 +33,29 @@ function lookup(repo::GitRepo, oid::GitHash)
     return GitBlob(blob_ptr_ptr[])
 end
 
-function GitBlob(repo::GitRepo, path::AbstractString)
-    blob_id_ptr = Ref(GitHash())
+"""
+    LibGit2.addblob!(repo::GitRepo, path::AbstractString)
+
+Reads the file at `path` and adds it to the object database of `repo` as a loose blob.
+Returns the `GitHash` of the resulting blob.
+"""
+function addblob!(repo::GitRepo, path::AbstractString)
+    id_ref = Ref{GitHash}()
     @check ccall((:git_blob_create_fromdisk, :libgit2), Cint,
-                 (Ptr{GitHash}, Ptr{Void}, Ptr{UInt8}), blob_id_ptr,
-                 repo.ptr, path)
-    blob = get(GitBlob, repo, blob_id_ptr[])
-    return blob
+                 (Ptr{GitHash}, Ptr{Void}, Cstring),
+                 id_ref, repo.ptr, path)
+    return id_ref[]
 end
 
 function Base.show(io::IO, blob::GitBlob)
     if !isbinary(blob)
         conts   = split(content(blob), "\n")
-        showlen = max(len(conts), 3)
-        print(io, "GitBlob:\nBlob id: ", GitHash(blob.ptr), "\nContents:\n")
+        showlen = max(length(conts), 3)
+        print(io, "GitBlob:\nBlob id: ", GitHash(blob), "\nContents:\n")
         for i in 1:showlen
             print(io, conts[i],"\n")
         end
     else
-        print(io, "GitBlob:\nBlob id: ", GitHash(blob.ptr), "\nContents are binary.")
+        print(io, "GitBlob:\nBlob id: ", GitHash(blob), "\nContents are binary.\n")
     end
 end

base/libgit2/commit.jl

@@ -74,13 +74,13 @@ function commit(repo::GitRepo, msg::AbstractString;
     commit_id  = GitHash()
 
     # get necessary objects
-    tree = get(GitTree, repo, tree_id)
+    tree = GitTree(repo, tree_id)
     auth_sig = convert(GitSignature, author)
     comm_sig = convert(GitSignature, committer)
     parents = GitCommit[]
     try
-        for parent in parent_ids
-            push!(parents, get(GitCommit, repo, parent))
+        for id in parent_ids
+            push!(parents, GitCommit(repo, id))
         end
         commit_id = commit(repo, refname, msg, auth_sig, comm_sig, tree, parents...)
     finally

base/libgit2/consts.jl

@@ -7,12 +7,13 @@ module Consts
     const REMOTE_ORIGIN = "origin"
 
     # objs
-    const OBJ_ANY    = Cint(-2)
-    const OBJ_BAD    = Cint(-1)
-    const OBJ_COMMIT = Cint(1)
-    const OBJ_TREE   = Cint(2)
-    const OBJ_BLOB   = Cint(3)
-    const OBJ_TAG    = Cint(4)
+    @enum(OBJECT,
+          OBJ_ANY    = -2,
+          OBJ_BAD    = -1,
+          OBJ_COMMIT = 1,
+          OBJ_TREE   = 2,
+          OBJ_BLOB   = 3,
+          OBJ_TAG    = 4)
 
     #revwalk
     const SORT_NONE        = Cint(0)

base/libgit2/index.jl

@@ -32,16 +32,19 @@ function repository(idx::GitIndex)
     end
 end
 
-function read_tree!(idx::GitIndex, tree_id::GitHash)
-    repo = repository(idx)
-    tree = get(GitTree, repo, tree_id)
-    try
-        @check ccall((:git_index_read_tree, :libgit2), Cint,
-                     (Ptr{Void}, Ptr{Void}), idx.ptr, tree.ptr)
-    finally
-        close(tree)
-    end
-end
+"""
+    LibGit2.read_tree!(idx::GitIndex, tree::GitTree)
+    LibGit2.read_tree!(idx::GitIndex, treehash::AbstractGitHash)
+
+Read the tree `tree` (or the tree pointed to by `treehash` in the repository owned by
+`idx`) into the index `idx`. The current index contents will be replaced.
+"""
+function read_tree!(idx::GitIndex, tree::GitTree)
+    @check ccall((:git_index_read_tree, :libgit2), Cint,
+                 (Ptr{Void}, Ptr{Void}), idx.ptr, tree.ptr)
+end
+read_tree!(idx::GitIndex, hash::AbstractGitHash) =
+    read_tree!(idx, GitTree(repository(idx), hash))
 
 function add!{T<:AbstractString}(idx::GitIndex, files::T...;
              flags::Cuint = Consts.INDEX_ADD_DEFAULT)

base/libgit2/libgit2.jl

@@ -76,7 +76,7 @@ is in the repository.
 function iscommit(id::AbstractString, repo::GitRepo)
     res = true
     try
-        c = get(GitCommit, repo, id)
+        c = GitCommit(repo, id)
         if c === nothing
             res = false
         else
@@ -111,9 +111,8 @@ Equivalent to `git diff-index <treeish> [-- <pathspecs>]`.
 """
 function isdiff(repo::GitRepo, treeish::AbstractString, paths::AbstractString=""; cached::Bool=false)
     tree_oid = revparseid(repo, "$treeish^{tree}")
-    iszero(tree_oid) && error("invalid treeish $treeish") # this can be removed by #20104
     result = false
-    tree = get(GitTree, repo, tree_oid)
+    tree = GitTree(repo, tree_oid)
     try
         diff = diff_tree(repo, tree, paths, cached=cached)
         result = count(diff) > 0
@@ -129,8 +128,8 @@ function diff_files(repo::GitRepo, branch1::AbstractString, branch2::AbstractStr
                     filter::Set{Cint}=Set([Consts.DELTA_ADDED, Consts.DELTA_MODIFIED, Consts.DELTA_DELETED]))
     b1_id = revparseid(repo, branch1*"^{tree}")
     b2_id = revparseid(repo, branch2*"^{tree}")
-    tree1 = get(GitTree, repo, b1_id)
-    tree2 = get(GitTree, repo, b2_id)
+    tree1 = GitTree(repo, b1_id)
+    tree2 = GitTree(repo, b2_id)
     files = AbstractString[]
     try
         diff = diff_tree(repo, tree1, tree2)
@@ -307,7 +306,7 @@ function branch!(repo::GitRepo, branch_name::AbstractString,
             GitHash(commit)
         end
         iszero(commit_id) && return
-        cmt =  get(GitCommit, repo, commit_id)
+        cmt =  GitCommit(repo, commit_id)
         new_branch_ref = nothing
         try
             new_branch_ref = Nullable(create_branch(repo, branch_name, cmt, force=force))
@@ -371,28 +370,17 @@ function checkout!(repo::GitRepo, commit::AbstractString = "";
     end
 
     # search for commit to get a commit object
-    obj = get(GitUnknownObject, repo, GitHash(commit))
-    obj === nothing && return
-    try
-        peeled = peel(obj, Consts.OBJ_COMMIT)
-        peeled === nothing && return
-        opts = force ? CheckoutOptions(checkout_strategy = Consts.CHECKOUT_FORCE) :
-                       CheckoutOptions()
-        try
-            # detach commit
-            obj_oid = GitHash(peeled)
-            ref = GitReference(repo, obj_oid, force=force,
-                msg="libgit2.checkout: moving from $head_name to $(string(obj_oid))")
-            close(ref)
-
-            # checkout commit
-            checkout_tree(repo, peeled, options = opts)
-        finally
-            close(peeled)
-        end
-    finally
-        close(obj)
-    end
+    obj = GitObject(repo, GitHash(commit))
+    peeled = peel(GitCommit, obj)
+
+    opts = force ? CheckoutOptions(checkout_strategy = Consts.CHECKOUT_FORCE) : CheckoutOptions()
+    # detach commit
+    obj_oid = GitHash(peeled)
+    ref = GitReference(repo, obj_oid, force=force,
+                       msg="libgit2.checkout: moving from $head_name to $(string(obj_oid))")
+
+    # checkout commit
+    checkout_tree(repo, peeled, options = opts)
 end
 
 """ git clone [-b <branch>] [--bare] <url> <dir> """
@@ -416,42 +404,22 @@ end
 
 """ git reset [<committish>] [--] <pathspecs>... """
 function reset!(repo::GitRepo, committish::AbstractString, pathspecs::AbstractString...)
-    obj = revparse(repo, !isempty(committish) ? committish : Consts.HEAD_FILE)
+    obj = GitObject(repo, isempty(committish) ? Consts.HEAD_FILE : committish)
     # do not remove entries in the index matching the provided pathspecs with empty target commit tree
-    obj === nothing && throw(GitError(Error.Object, Error.ERROR, "`$committish` not found"))
-    try
-        head = reset!(repo, Nullable(obj), pathspecs...)
-        return head
-    finally
-        close(obj)
-    end
-    return head_oid(repo)
+    reset!(repo, Nullable(obj), pathspecs...)
 end
 
-""" git reset [--soft | --mixed | --hard] <commit> """
-function reset!(repo::GitRepo, commit::GitHash, mode::Cint = Consts.RESET_MIXED)
-    obj = get(GitUnknownObject, repo, commit)
-    # object must exist for reset
-    obj === nothing && throw(GitError(Error.Object, Error.ERROR, "Commit `$(string(commit))` object not found"))
-    try
-        head = reset!(repo, obj, mode)
-        return head
-    finally
-        close(obj)
-    end
-    return head_oid(repo)
-end
+""" git reset [--soft | --mixed | --hard] <id> """
+reset!(repo::GitRepo, id::GitHash, mode::Cint = Consts.RESET_MIXED) =
+    reset!(repo, GitObject(repo, id), mode)
 
 """ git cat-file <commit> """
-function cat{T<:GitObject}(repo::GitRepo, ::Type{T}, object::AbstractString)
-    obj_id = revparseid(repo, object)
-    iszero(obj_id) && return nothing
-
-    obj = get(T, repo, obj_id)
+function cat(repo::GitRepo, spec)
+    obj = GitObject(repo, spec)
     if isa(obj, GitBlob)
-        return unsafe_string(convert(Ptr{UInt8}, content(obj)))
+        content(obj)
     else
-        return nothing
+        nothing
     end
 end
 
@@ -515,10 +483,10 @@ function merge!(repo::GitRepo;
                 remotename = with(GitConfig, repo) do cfg
                     LibGit2.get(String, cfg, "branch.$branchname.remote")
                 end
-                obj = with(GitReference(repo, "refs/remotes/$remotename/$branchname")) do ref
+                oid = with(GitReference(repo, "refs/remotes/$remotename/$branchname")) do ref
                     LibGit2.GitHash(ref)
                 end
-                with(get(GitCommit, repo, obj)) do cmt
+                with(GitCommit(repo, oid)) do cmt
                     LibGit2.create_branch(repo, branchname, cmt)
                 end
                 return true
@@ -616,7 +584,7 @@ end
 """ Returns all commit authors """
 function authors(repo::GitRepo)
     return with(GitRevWalker(repo)) do walker
-        map((oid,repo)->with(get(GitCommit, repo, oid)) do cmt
+        map((oid,repo)->with(GitCommit(repo, oid)) do cmt
                             author(cmt)::Signature
                         end,
             walker) #, by = Consts.SORT_TIME)

base/libgit2/merge.jl

@@ -25,18 +25,13 @@ function GitAnnotated(repo::GitRepo, fh::FetchHead)
 end
 
 function GitAnnotated(repo::GitRepo, comittish::AbstractString)
-    obj = revparse(repo, comittish)
-    try
-        cmt = peel(obj, Consts.OBJ_COMMIT)
-        cmt === nothing && return nothing
-        return GitAnnotated(repo, GitHash(cmt))
-    finally
-        close(obj)
-    end
+    obj = GitObject(repo, comittish)
+    cmt = peel(GitCommit, obj)
+    return GitAnnotated(repo, GitHash(cmt))
 end
 
-function commit(ann::GitAnnotated)
-    return GitHash(ccall((:git_annotated_commit_id, :libgit2), Ptr{GitHash}, (Ptr{Void},), ann.ptr))
+function GitHash(ann::GitAnnotated)
+    unsafe_load(ccall((:git_annotated_commit_id, :libgit2), Ptr{GitHash}, (Ptr{Void},), ann.ptr))
 end
 
 function merge_analysis(repo::GitRepo, anns::Vector{GitAnnotated})
@@ -52,23 +47,18 @@ end
 
 """Fastforward merge changes into current head """
 function ffmerge!(repo::GitRepo, ann::GitAnnotated)
-    ann_cmt_oid = commit(ann)
-    cmt = get(GitCommit, repo, ann_cmt_oid)
-    cmt === nothing && return false # could not find commit tree
-    try
-        checkout_tree(repo, cmt)
-        with(head(repo)) do head_ref
-            cmt_oid = GitHash(cmt)
-            msg = "libgit2.merge: fastforward $(string(cmt_oid)) into $(name(head_ref))"
-            new_head_ref = if reftype(head_ref) == Consts.REF_OID
-                target!(head_ref, cmt_oid, msg=msg)
-            else
-                GitReference(repo, cmt_oid, fullname(head_ref), msg=msg)
-            end
-            close(new_head_ref)
+    cmt = GitCommit(repo, GitHash(ann))
+
+    checkout_tree(repo, cmt)
+    with(head(repo)) do head_ref
+        cmt_oid = GitHash(cmt)
+        msg = "libgit2.merge: fastforward $(string(cmt_oid)) into $(name(head_ref))"
+        new_head_ref = if reftype(head_ref) == Consts.REF_OID
+            target!(head_ref, cmt_oid, msg=msg)
+        else
+            GitReference(repo, cmt_oid, fullname(head_ref), msg=msg)
         end
-    finally
-        close(cmt)
+        close(new_head_ref)
     end
     return true
 end