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: https://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