Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/src/lib/remote-links.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ This means that usually it is not necessary to specify either explicitly in the
The rules are as follows:

* If `repo` _is not_ specified, it is essentially [determined like any other remote link](@ref remotes-for-files), by trying to figure out the repository that contains the `root` path argument of [`makedocs`](@ref) (defaulting to the directory of the `make.jl` script; usually the `docs/` directory).
The [`Remote`](@ref Remotes.Remote) object will one of the `remotes`, which in turn may have been determined automatically via the `origin` URL of the containing Git repository.
The [`Remote`](@ref Remotes.Remote) object will be one of the `remotes`, which in turn may have been determined automatically via the `origin` URL of the containing Git repository, or the 'remotes.pushDefault' remote's URL if `origin` does not exist.

* If `repo` _is_ specified, but the `remotes` for the repository root is not, `repo` will function as a `remotes` entry for the repository root.
This is so that it would not be necessary to specify the same argument twice (i.e. once for general repository links, once for file links).
Expand Down
66 changes: 53 additions & 13 deletions src/utilities/utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,45 @@ const julia_remote = Remotes.GitHub("JuliaLang", "julia")
"""
Stores the memoized results of [`getremote`](@ref).
"""
const GIT_REMOTE_CACHE = Dict{String,Union{Remotes.Remote,Nothing}}()
const GIT_REMOTENAME_CACHE = Dict{String, String}()
const GIT_REMOTEURL_CACHE = Dict{String,Union{Remotes.Remote,Nothing}}()

"""
$(TYPEDSIGNATURES)

Determines the GitHub remote of a directory, returninf 'origin' if that remote exists,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Determines the GitHub remote of a directory, returninf 'origin' if that remote exists,
Determines the GitHub remote of a directory, returning `origin` if that remote exists,

and otherwise returning the result of `git config --get remote.pushDefault`.
If neither exist, throw an error.

The results for a given directory are memoized in [`GIT_REMOTENAME_CACHE`](@ref), since calling
`git` is expensive and it is often called on the same directory over and over again.
"""
function getremotename(dir::AbstractString)
isdir(dir) || return nothing
return get!(GIT_REMOTENAME_CACHE, dir) do
try
originurl = readchomp(setenv(`$(git()) config --get remote.origin.url`; dir=dir))
@assert !isempty(originurl)
return "origin"
catch; end
Comment on lines +440 to +444
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You seem to be using @assert plus try/catch for control flow. That seems like a bad idea for multiple reasons (e.g. future Julia version might allow disabling @assert checks, which is, I think, mainly intended to enforce to check runtime invariants, which is not what this code does).

How about just e.g. this

Suggested change
try
originurl = readchomp(setenv(`$(git()) config --get remote.origin.url`; dir=dir))
@assert !isempty(originurl)
return "origin"
catch; end
originurl = readchomp(setenv(`$(git()) config --get remote.origin.url`; dir=dir))
if !isempty(originurl)
return "origin"
end

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same applies to the rest of the code, of course


try
pushdefault = readchomp(setenv(`$(git()) config --get remote.pushDefault`; dir=dir))
@assert !isempty(pushdefault)
@warn "Remote 'origin' not found. Using `git config --get remote.pushDefault` instead, which is set to '$(pushdefault)'."
return pushdefault
catch ; end
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this PR needs to be reformatted with runic



# If neither was found, throw an error.
try
@assert false
catch e
@warn "Neither remote 'origin' nor 'remotes.pushDefault' found" exception=(e, catch_backtrace())
""
end
end
end

function parse_remote_url(remote::AbstractString)
# TODO: we only match for GitHub repositories automatically. Could we engineer a
Expand All @@ -435,24 +473,25 @@ end
"""
$(TYPEDSIGNATURES)

Determines the GitHub remote of a directory by checking `remote.origin.url` of the
repository. Returns a [`Remotes.GitHub`](@ref), or `nothing` is something has gone wrong
(e.g. it's run on a directory not in a Git repo, or `origin.url` points to a non-GitHub
remote).
Determines the GitHub remote of a directory by checking `remote.\$(remotename).url` of the
repository. `remotename` is determined by [`getremotename`](@ref). Returns a [`Remotes.GitHub`](@ref),
or `nothing` is something has gone wrong (e.g. it's run on a directory not in a Git repo, or the url
points to a non-GitHub remote).

The results for a given directory are memoized in [`GIT_REMOTE_CACHE`](@ref), since calling
The results for a given directory are memoized in [`GIT_REMOTEURL_CACHE`](@ref), since calling
`git` is expensive and it is often called on the same directory over and over again.
"""
function getremote(dir::AbstractString)
isdir(dir) || return nothing
return get!(GIT_REMOTE_CACHE, dir) do
remote = try
readchomp(setenv(`$(git()) config --get remote.origin.url`; dir=dir))
return get!(GIT_REMOTEURL_CACHE, dir) do
remotename = getremotename(dir)
remoteurl = try
readchomp(setenv(`$(git()) config --get remote.$(remotename).url`; dir=dir))
catch e
@debug "git config --get remote.origin.url failed" exception=(e, catch_backtrace())
@debug "git config --get remote.$(remotename).url failed" exception=(e, catch_backtrace())
""
end
return parse_remote_url(remote)
return parse_remote_url(remoteurl)
end
end

Expand Down Expand Up @@ -616,7 +655,7 @@ it out automatically.
`root` is the the directory where `git` gets run. `varname` is just informational and used
to construct the warning messages.
"""
function git_remote_head_branch(varname, root; remotename = "origin", fallback = "master")
function git_remote_head_branch(varname, root; fallback = "master")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring still references remotename. So either that needs to be adjusted. Alternatively, keep the kwarg here, but with a new default value nothing; and then later only call getremotename if remotename === nothing ?

gitcmd = git(nothrow = true)
if gitcmd === nothing
@warn """
Expand All @@ -628,8 +667,9 @@ function git_remote_head_branch(varname, root; remotename = "origin", fallback =
end
# We need to do addenv() here to merge the new variables with the environment set by
# Git_jll and the git() function.
remotename = getremotename(root)
cmd = addenv(
setenv(`$gitcmd remote show $(remotename)`, dir=root),
setenv(`$gitcmd remote show $remotename`, dir=root),
"GIT_TERMINAL_PROMPT" => "0",
"GIT_SSH_COMMAND" => get(ENV, "GIT_SSH_COMMAND", "ssh -o \"BatchMode yes\""),
)
Expand Down
21 changes: 20 additions & 1 deletion test/repolinks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
module RepoLinkTests
using Test
using Random: randstring
using Documenter: Documenter, Remotes, git, edit_url, source_url, MarkdownAST, walk_navpages, expand
using Documenter: Documenter, Remotes, git, edit_url, source_url, MarkdownAST, walk_navpages, expand, getremote, getremotename, GIT_REMOTENAME_CACHE
using Documenter.HTMLWriter: render_article, HTMLContext, HTML
using Markdown
include("TestUtilities.jl"); using Main.TestUtilities
Expand Down Expand Up @@ -67,6 +67,13 @@ extrepo_commit = init_git_repo(
# A repository outside of the main repository (without remote)
extrepo_noremote = joinpath(tmproot, "extrepo_noremote")
extrepo_noremote_commit = init_git_repo(create_defaultfiles, extrepo_noremote; remote=nothing)
# A repository with a repo that's not called 'origin'
nonoriginrepo = joinpath(tmproot, "nonoriginrepo")
nonoriginrepo_commit = init_git_repo(
create_defaultfiles, nonoriginrepo;
remote=("nonorigin", "[email protected]:TestOrg/NonoriginRepo.jl.git")
)

# Just a directory outside of the main repository
extdirectory = joinpath(tmproot, "extdirectory")
mkpath(extdirectory)
Expand Down Expand Up @@ -218,6 +225,18 @@ end
@test doc.user.remote == Remotes.GitHub("AlternateOrg", "AlternateRepo.jl")
end

@testset "Get remotename and remoteurl" begin
@test getremotename(mainrepo) == "origin"
@test getremote(mainrepo) == Remotes.GitHub("TestOrg", "TestRepo.jl")
@test_warn "Neither remote" getremotename(nonoriginrepo)
@test getremotename(nonoriginrepo) == ""
run(setenv(`$(git()) config --add remote.pushDefault nonorigin`; dir=nonoriginrepo))
pop!(GIT_REMOTENAME_CACHE, nonoriginrepo)
@test Documenter.getremotename(nonoriginrepo) == "nonorigin"
@test Documenter.getremote(nonoriginrepo) == Documenter.Remotes.GitHub("TestOrg","NonoriginRepo.jl")
end


# Setting both the `repo` and also overriding the same path in `remotes` should error
@test_throws Exception Documenter.Document(
root = joinpath(mainrepo, "docs"),
Expand Down