Skip to content

Commit f2b03d4

Browse files
simonbyrneararslan
simonbyrne
authored and
ararslan
committed
Revamp libgit2 owner interface. (JuliaLang#20786)
It seems that some objects can be owned by non-repo objects. This makes the code a bit more general to allow for this. * Make GitTreeEntry always have a reference to the GitTree, get rid of unnecessary repo arguments
1 parent 05cea50 commit f2b03d4

File tree

6 files changed

+52
-63
lines changed

6 files changed

+52
-63
lines changed

‎base/libgit2/index.jl

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ function write_tree!(idx::GitIndex)
2525
end
2626

2727
function repository(idx::GitIndex)
28-
if isnull(idx.nrepo)
28+
if isnull(idx.owner)
2929
throw(GitError(Error.Index, Error.ENOTFOUND, "Index does not have an owning repository."))
3030
else
31-
return Base.get(idx.nrepo)
31+
return Base.get(idx.owner)
3232
end
3333
end
3434

‎base/libgit2/reference.jl

+7-7
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ function peel{T<:GitObject}(::Type{T}, ref::GitReference)
171171
obj_ptr_ptr = Ref{Ptr{Void}}(C_NULL)
172172
@check ccall((:git_reference_peel, :libgit2), Cint,
173173
(Ptr{Ptr{Void}}, Ptr{Void}, Cint), obj_ptr_ptr, ref.ptr, Consts.OBJECT(T))
174-
return T(ref.repo, obj_ptr_ptr[])
174+
return T(ref.owner, obj_ptr_ptr[])
175175
end
176176
peel(ref::GitReference) = peel(GitObject, ref)
177177

@@ -280,21 +280,21 @@ function upstream(ref::GitReference)
280280
return Nullable{GitReference}()
281281
end
282282
if ref_ptr_ptr[] != C_NULL
283-
close(GitReference(ref.repo, ref_ptr_ptr[]))
283+
close(GitReference(ref.owner, ref_ptr_ptr[]))
284284
end
285285
throw(Error.GitError(err))
286286
end
287-
return Nullable{GitReference}(GitReference(ref.repo, ref_ptr_ptr[]))
287+
return Nullable{GitReference}(GitReference(ref.owner, ref_ptr_ptr[]))
288288
end
289289

290-
repository(ref::GitReference) = ref.repo
290+
repository(ref::GitReference) = ref.owner
291291

292292
function target!(ref::GitReference, new_oid::GitHash; msg::AbstractString="")
293293
ref_ptr_ptr = Ref{Ptr{Void}}(C_NULL)
294294
@check ccall((:git_reference_set_target, :libgit2), Cint,
295295
(Ptr{Ptr{Void}}, Ptr{Void}, Ptr{GitHash}, Cstring),
296296
ref_ptr_ptr, ref.ptr, Ref(new_oid), isempty(msg) ? C_NULL : msg)
297-
return GitReference(ref.repo, ref_ptr_ptr[])
297+
return GitReference(ref.owner, ref_ptr_ptr[])
298298
end
299299

300300
function GitBranchIter(repo::GitRepo, flags::Cint=Cint(Consts.BRANCH_LOCAL))
@@ -311,7 +311,7 @@ function Base.start(bi::GitBranchIter)
311311
(Ptr{Ptr{Void}}, Ptr{Cint}, Ptr{Void}),
312312
ref_ptr_ptr, btype, bi.ptr)
313313
err != Int(Error.GIT_OK) && return (nothing, -1, true)
314-
return (GitReference(bi.repo, ref_ptr_ptr[]), btype[], false)
314+
return (GitReference(bi.owner, ref_ptr_ptr[]), btype[], false)
315315
end
316316

317317
Base.done(bi::GitBranchIter, state) = Bool(state[3])
@@ -323,7 +323,7 @@ function Base.next(bi::GitBranchIter, state)
323323
(Ptr{Ptr{Void}}, Ptr{Cint}, Ptr{Void}),
324324
ref_ptr_ptr, btype, bi.ptr)
325325
err != Int(Error.GIT_OK) && return (state[1:2], (nothing, -1, true))
326-
return (state[1:2], (GitReference(bi.repo, ref_ptr_ptr[]), btype[], false))
326+
return (state[1:2], (GitReference(bi.owner, ref_ptr_ptr[]), btype[], false))
327327
end
328328

329329
Base.iteratorsize(::Type{GitBranchIter}) = Base.SizeUnknown()

‎base/libgit2/repository.jl

+1-1
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ function peel{T<:GitObject}(::Type{T}, obj::GitObject)
235235
@check ccall((:git_object_peel, :libgit2), Cint,
236236
(Ptr{Ptr{Void}}, Ptr{Void}, Cint), new_ptr_ptr, obj.ptr, Consts.OBJECT(T))
237237

238-
return T(obj.repo, new_ptr_ptr[])
238+
return T(obj.owner, new_ptr_ptr[])
239239
end
240240
peel(obj::GitObject) = peel(GitObject, obj)
241241

‎base/libgit2/tree.jl

+5-7
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,8 @@ function treewalk(f::Function, tree::GitTree, payload=Any[], post::Bool = false)
1616
return cbf_payload
1717
end
1818

19-
function repository(tree::GitTree)
20-
repo_ptr = ccall((:git_tree_owner, :libgit2), Ptr{Void},
21-
(Ptr{Void},), tree.ptr)
22-
return GitRepo(repo_ptr)
23-
end
19+
repository(tree::GitTree) = tree.owner
20+
repository(te::GitTreeEntry) = repository(te.owner)
2421

2522
function filename(te::GitTreeEntry)
2623
str = ccall((:git_tree_entry_name, :libgit2), Cstring, (Ptr{Void},), te.ptr)
@@ -53,10 +50,11 @@ function Base.getindex(tree::GitTree, i::Integer)
5350
te_ptr = ccall((:git_tree_entry_byindex, :libgit2),
5451
Ptr{Void},
5552
(Ptr{Void}, Csize_t), tree.ptr, i-1)
56-
return GitTreeEntry(te_ptr, false)
53+
return GitTreeEntry(tree, te_ptr, false)
5754
end
5855

59-
function (::Type{T}){T<:GitObject}(repo::GitRepo, te::GitTreeEntry)
56+
function (::Type{T}){T<:GitObject}(te::GitTreeEntry)
57+
repo = repository(te)
6058
obj_ptr_ptr = Ref{Ptr{Void}}(C_NULL)
6159
@check ccall((:git_tree_entry_to_object, :libgit2), Cint,
6260
(Ptr{Ptr{Void}}, Ptr{Void}, Ref{Void}),

‎base/libgit2/types.jl

+36-45
Original file line numberDiff line numberDiff line change
@@ -480,30 +480,31 @@ Base.isempty(obj::AbstractGitObject) = (obj.ptr == C_NULL)
480480

481481
abstract type GitObject <: AbstractGitObject end
482482

483-
for (typ, reporef, sup, cname) in [
484-
(:GitRepo, nothing, :AbstractGitObject, :git_repository),
485-
(:GitTreeEntry, nothing, :AbstractGitObject, :git_tree_entry),
486-
(:GitDiffStats, nothing, :AbstractGitObject, :git_diff_stats),
487-
(:GitConfig, :Nullable, :AbstractGitObject, :git_config),
488-
(:GitIndex, :Nullable, :AbstractGitObject, :git_index),
489-
(:GitRemote, :GitRepo, :AbstractGitObject, :git_remote),
490-
(:GitRevWalker, :GitRepo, :AbstractGitObject, :git_revwalk),
491-
(:GitReference, :GitRepo, :AbstractGitObject, :git_reference),
492-
(:GitDiff, :GitRepo, :AbstractGitObject, :git_diff),
493-
(:GitAnnotated, :GitRepo, :AbstractGitObject, :git_annotated_commit),
494-
(:GitRebase, :GitRepo, :AbstractGitObject, :git_rebase),
495-
(:GitStatus, :GitRepo, :AbstractGitObject, :git_status_list),
496-
(:GitBranchIter, :GitRepo, :AbstractGitObject, :git_branch_iterator),
497-
(:GitUnknownObject, :GitRepo, :GitObject, :git_object),
498-
(:GitCommit, :GitRepo, :GitObject, :git_commit),
499-
(:GitBlob, :GitRepo, :GitObject, :git_blob),
500-
(:GitTree, :GitRepo, :GitObject, :git_tree),
501-
(:GitTag, :GitRepo, :GitObject, :git_tag)]
502-
503-
if reporef === nothing
483+
for (typ, owntyp, sup, cname) in [
484+
(:GitRepo, nothing, :AbstractGitObject, :git_repository),
485+
(:GitDiffStats, nothing, :AbstractGitObject, :git_diff_stats),
486+
(:GitConfig, :(Nullable{GitRepo}), :AbstractGitObject, :git_config),
487+
(:GitIndex, :(Nullable{GitRepo}), :AbstractGitObject, :git_index),
488+
(:GitRemote, :GitRepo, :AbstractGitObject, :git_remote),
489+
(:GitRevWalker, :GitRepo, :AbstractGitObject, :git_revwalk),
490+
(:GitReference, :GitRepo, :AbstractGitObject, :git_reference),
491+
(:GitDiff, :GitRepo, :AbstractGitObject, :git_diff),
492+
(:GitAnnotated, :GitRepo, :AbstractGitObject, :git_annotated_commit),
493+
(:GitRebase, :GitRepo, :AbstractGitObject, :git_rebase),
494+
(:GitStatus, :GitRepo, :AbstractGitObject, :git_status_list),
495+
(:GitBranchIter, :GitRepo, :AbstractGitObject, :git_branch_iterator),
496+
(:GitUnknownObject, :GitRepo, :GitObject, :git_object),
497+
(:GitCommit, :GitRepo, :GitObject, :git_commit),
498+
(:GitBlob, :GitRepo, :GitObject, :git_blob),
499+
(:GitTree, :GitRepo, :GitObject, :git_tree),
500+
(:GitTag, :GitRepo, :GitObject, :git_tag),
501+
(:GitTreeEntry, :GitTree, :AbstractGitObject, :git_tree_entry),
502+
]
503+
504+
if owntyp === nothing
504505
@eval mutable struct $typ <: $sup
505506
ptr::Ptr{Void}
506-
function $typ(ptr::Ptr{Void},fin=true)
507+
function $typ(ptr::Ptr{Void}, fin::Bool=true)
507508
# fin=false should only be used when the pointer should not be free'd
508509
# e.g. from within callback functions which are passed a pointer
509510
@assert ptr != C_NULL
@@ -515,35 +516,25 @@ for (typ, reporef, sup, cname) in [
515516
return obj
516517
end
517518
end
518-
elseif reporef == :Nullable
519+
else
519520
@eval mutable struct $typ <: $sup
520-
nrepo::Nullable{GitRepo}
521+
owner::$owntyp
521522
ptr::Ptr{Void}
522-
function $typ(repo::GitRepo, ptr::Ptr{Void})
523-
@assert ptr != C_NULL
524-
obj = new(Nullable(repo), ptr)
525-
Threads.atomic_add!(REFCOUNT, UInt(1))
526-
finalizer(obj, Base.close)
527-
return obj
528-
end
529-
function $typ(ptr::Ptr{Void})
523+
function $typ(owner::$owntyp, ptr::Ptr{Void}, fin::Bool=true)
530524
@assert ptr != C_NULL
531-
obj = new(Nullable{GitRepo}(), ptr)
532-
Threads.atomic_add!(REFCOUNT, UInt(1))
533-
finalizer(obj, Base.close)
525+
obj = new(owner, ptr)
526+
if fin
527+
Threads.atomic_add!(REFCOUNT, UInt(1))
528+
finalizer(obj, Base.close)
529+
end
534530
return obj
535531
end
536532
end
537-
elseif reporef == :GitRepo
538-
@eval mutable struct $typ <: $sup
539-
repo::GitRepo
540-
ptr::Ptr{Void}
541-
function $typ(repo::GitRepo, ptr::Ptr{Void})
542-
@assert ptr != C_NULL
543-
obj = new(repo, ptr)
544-
Threads.atomic_add!(REFCOUNT, UInt(1))
545-
finalizer(obj, Base.close)
546-
return obj
533+
if isa(owntyp, Expr) && owntyp.args[1] == :Nullable
534+
@eval begin
535+
$typ(ptr::Ptr{Void}, fin::Bool=true) = $typ($owntyp(), ptr, fin)
536+
$typ(owner::$(owntyp.args[2]), ptr::Ptr{Void}, fin::Bool=true) =
537+
$typ($owntyp(owner), ptr, fin)
547538
end
548539
end
549540
end

‎base/libgit2/walker.jl

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ function Base.sort!(w::GitRevWalker; by::Cint = Consts.SORT_NONE, rev::Bool=fals
4848
return w
4949
end
5050

51-
repository(w::GitRevWalker) = w.repo
51+
repository(w::GitRevWalker) = w.owner
5252

5353
function Base.map(f::Function, walker::GitRevWalker;
5454
oid::GitHash=GitHash(),

0 commit comments

Comments
 (0)