Fix Pkg require check (JuliaLang#20433)

* Fix Pkg require check

Pkg previously used `iszero` as a check on the return value of `revparseid`. `revparseid` no longer returns a zero hash if the result does not exist.

Fixes JuliaLang#20419.

* fix another revparseid issue

* remove extra paranthesis

* Use the git index to check if REQUIRE exists.
Also fix a few miscellaneous bugs.

* add tests for GitObject specifiers

* correct variable name

Author: simonbyrne

base/libgit2/index.jl

@@ -113,12 +113,12 @@ function Base.find(path::String, idx::GitIndex)
     pos_ref = Ref{Csize_t}(0)
     ret = ccall((:git_index_find, :libgit2), Cint,
                   (Ref{Csize_t}, Ptr{Void}, Cstring), pos_ref, idx.ptr, path)
-    ret == Error.ENOTFOUND && return Nullable{Csize_t}()
+    ret == Cint(Error.ENOTFOUND) && return Nullable{Csize_t}()
     return Nullable(pos_ref[]+1)
 end
 
 stage(ie::IndexEntry) = ccall((:git_index_entry_stage, :libgit2), Cint, (Ptr{IndexEntry},), Ref(ie))
 
 function Base.show(io::IO, idx::GitIndex)
-    println(io, "GitIndex:\nRepository: ", repository(idk), "\nNumber of elements: ", count(idx))
+    println(io, "GitIndex:\nRepository: ", repository(idx), "\nNumber of elements: ", count(idx))
 end

base/libgit2/libgit2.jl

@@ -110,9 +110,7 @@ tracked files in the working tree (if `cached=false`) or the index (if `cached=t
 Equivalent to `git diff-index <treeish> [-- <pathspecs>]`.
 """
 function isdiff(repo::GitRepo, treeish::AbstractString, paths::AbstractString=""; cached::Bool=false)
-    tree_oid = revparseid(repo, "$treeish^{tree}")
-    result = false
-    tree = GitTree(repo, tree_oid)
+    tree = GitTree(repo, "$treeish^{tree}")
     try
         diff = diff_tree(repo, tree, paths, cached=cached)
         result = count(diff) > 0

base/libgit2/repository.jl

@@ -100,6 +100,11 @@ function (::Type{T}){T<:GitObject}(repo::GitRepo, spec::AbstractString)
     obj_ptr_ptr = Ref{Ptr{Void}}(C_NULL)
     @check ccall((:git_revparse_single, :libgit2), Cint,
                  (Ptr{Ptr{Void}}, Ptr{Void}, Cstring), obj_ptr_ptr, repo.ptr, spec)
+    # check object is of correct type
+    if T != GitObject && T != GitUnknownObject
+        t = Consts.OBJECT(obj_ptr_ptr[])
+        t == Consts.OBJECT(T) || throw(GitError(Error.Object, Error.ERROR, "Expected object of type $T, received object of type $(objtype(t))"))
+    end
     return T(repo, obj_ptr_ptr[])
 end
 

base/libgit2/types.jl

@@ -535,7 +535,7 @@ end
 
 ## Calling `GitObject(repo, ...)` will automatically resolve to the appropriate type.
 function GitObject(repo::GitRepo, ptr::Ptr{Void})
-    T = objtype(ccall((:git_object_type, :libgit2), Consts.OBJECT, (Ptr{Void},), ptr))
+    T = objtype(Consts.OBJECT(ptr))
     T(repo, ptr)
 end
 
@@ -602,6 +602,9 @@ Consts.OBJECT(::Type{GitTag})           = Consts.OBJ_TAG
 Consts.OBJECT(::Type{GitUnknownObject}) = Consts.OBJ_ANY
 Consts.OBJECT(::Type{GitObject})        = Consts.OBJ_ANY
 
+Consts.OBJECT(ptr::Ptr{Void}) =
+    ccall((:git_object_type, :libgit2), Consts.OBJECT, (Ptr{Void},), ptr)
+
 """
     objtype(obj_type::Consts.OBJECT)
 

base/pkg/read.jl

@@ -64,8 +64,9 @@ function isfixed(pkg::AbstractString, prepo::LibGit2.GitRepo, avail::Dict=availa
     LibGit2.isdirty(prepo) && return true
     LibGit2.isattached(prepo) && return true
     LibGit2.need_update(prepo)
-    LibGit2.iszero(LibGit2.revparseid(prepo, "HEAD:REQUIRE")) && isfile(pkg,"REQUIRE") && return true
-
+    if isnull(find("REQUIRE", LibGit2.GitIndex(prepo)))
+        isfile(pkg,"REQUIRE") && return true
+    end
     head = string(LibGit2.head_oid(prepo))
     for (ver,info) in avail
         head == info.sha1 && return false
@@ -182,7 +183,9 @@ function requires_path(pkg::AbstractString, avail::Dict=available(pkg))
     head = LibGit2.with(LibGit2.GitRepo, pkg) do repo
         LibGit2.isdirty(repo, "REQUIRE") && return pkgreq
         LibGit2.need_update(repo)
-        LibGit2.iszero(LibGit2.revparseid(repo, "HEAD:REQUIRE")) && isfile(pkgreq) && return pkgreq
+        if isnull(find("REQUIRE", LibGit2.GitIndex(repo)))
+            isfile(pkgreq) && return pkgreq
+        end
         string(LibGit2.head_oid(repo))
     end
     for (ver,info) in avail

test/libgit2.jl

@@ -454,6 +454,16 @@ mktempdir() do dir
                 close(repo)
             end
         end
+        @testset "trees" begin
+            repo = LibGit2.GitRepo(cache_repo)
+            try
+                @test_throws LibGit2.Error.GitError LibGit2.GitTree(repo, "HEAD")
+                @test isa(LibGit2.GitTree(repo, "HEAD^{tree}"), LibGit2.GitTree)
+                @test isa(LibGit2.GitObject(repo, "HEAD^{tree}"), LibGit2.GitTree)
+            finally
+                close(repo)
+            end
+        end
 
         @testset "diff" begin
             repo = LibGit2.GitRepo(cache_repo)
@@ -596,6 +606,9 @@ mktempdir() do dir
                 i = find(test_file, idx)
                 @test !isnull(i)
                 @test idx[get(i)] !== nothing
+
+                i = find("zzz", idx)
+                @test isnull(i)
             end
 
             # check non-existent file status