Move LibGit2 to stdlib #25706

KristofferC · 2018-01-23T15:26:20Z

Made on top of #25705.

I have a few comments inline.

There was some problems with the documentation generation locally, so we will see what happens here.

KristofferC · 2018-01-23T15:27:02Z

base/methodshow.jl

@@ -212,6 +212,8 @@ function url(m::Method)
            return "https://github.com/JuliaLang/julia/tree/$(Base.GIT_VERSION_INFO.commit)/base/$file#L$line"
        end
    else
+        return fileurl(file)
+        #=


Need to figure out what to do with this one url(::Method)...

KristofferC · 2018-01-23T15:28:52Z

test/docs.jl

 end

 let x = Binding(Iterators, :enumerate)
    @test defined(x) == true
    @test @var(enumerate) == x
    @test @var(Base.enumerate) == x
    @test @var(Iterators.enumerate) == x
-    @test @var(Base.Pkg.Dir.enumerate) == x
+    # @test @var(Base.Pkg.Dir.enumerate) == x


Not sure exactly what this is testing, perhaps just a deep nesting.

vchuravy · 2018-01-23T15:50:35Z

base/deprecated.jl

@@ -243,19 +243,6 @@ DEPRECATED: use @__MODULE__ instead
 end
 export current_module

-# PR #22062


These deprecations were made during the 0.7 cycle so they need to use @deprecate_moved or @deprecate_stdlib after #25692

Hmm, the behavior right now is:

julia> Base.LibGit2.prompt("Hello?") WARNING: Base.LibGit2 is deprecated, run `import LibGit2` instead. in module Base ┌ Warning: `LibGit2.prompt(msg::AbstractString; default::AbstractString="", password::Bool=false)` is deprecated, use `result = Base.prompt(msg, default=default, password=password); result === nothing ? "" : result` instead. │ caller = top-level scope └ @ Core :0 Hello?:yes "yes"

Isn't that ok?

Oh I didn't think that would happen. Neat. You get a deprecation warning for Base.LibGit2 and then a separate one for prompt. So as long as non of these were exported everything should be ok.

omus · 2018-01-23T17:30:37Z

Is it possible to move out the libgit2 C library with this? It would be nice to be able to update the revision of the shared library during 1.0.

waldyrious · 2018-01-23T18:22:36Z

doc/src/manual/packages.md

@@ -3,8 +3,8 @@
 Julia has a built-in package manager for installing add-on functionality written in Julia. It
 can also install external libraries using your operating system's standard system for doing so,
 or by compiling from source. The list of registered Julia packages can be found at [http://pkg.julialang.org](http://pkg.julialang.org).
-All package manager commands are found in the `Pkg` module, included in Julia's `Base`
-install.
+All package manager commands are found in the `Pkg` standard library, it becomes available after using


Suggestion: "it becomes" --> "which becomes", to avoid a comma splice.

StefanKarpinski · 2018-01-23T18:52:54Z

Is it possible to move out the libgit2 C library with this? It would be nice to be able to update the revision of the shared library during 1.0.

1.0 API stability definitely does not include never changing versions of libraries that we depend on.

KristofferC · 2018-01-24T15:24:39Z

So this causes the documentation warnings:

 !! Invalid local link: unresolved path
    '../../devdocs/libgit2/#Base.LibGit2.GitRevWalker' in devdocs/libgit2.md
 !! Invalid local link: unresolved path
    '../../devdocs/libgit2/#Base.LibGit2.GitHash' in devdocs/libgit2.md
 !! Invalid local link: unresolved path
    '../../devdocs/libgit2/#Base.LibGit2.push_head!' in devdocs/libgit2.md
 !! Invalid local link: unresolved path
    '../../devdocs/libgit2/#Base.LibGit2.GitRevWalker' in devdocs/libgit2.md
 !! Invalid local link: unresolved path
    '../../devdocs/libgit2/#Base.LibGit2.GitHash' in devdocs/libgit2.md
 !! Invalid local link: unresolved path
    '../../devdocs/libgit2/#Base.LibGit2.push_head!' in devdocs/libgit2.md

to turn into:

 !! No doc found for reference '[`GitRevWalker`](@ref)'. [src/base/collections.md]
 !! No doc found for reference '[`GitHash`](@ref)'. [src/base/collections.md]
 !! No doc found for reference '[`push_head!`](@ref)'. [src/base/collections.md]
 !! No doc found for reference '[`GitRevWalker`](@ref)'. [src/base/collections.md]
 !! No doc found for reference '[`GitHash`](@ref)'. [src/base/collections.md]
 !! No doc found for reference '[`push_head!`](@ref)'. [src/base/collections.md]

which apparently fails the documentation build... I have no idea how to fix it but I temporarily turned off strict documentation build to see if the other tests pass.

KristofferC · 2018-01-31T14:16:05Z

CI straight flush. 🎉

So what is left to do here is:

Figure out what to do with url(::Method) when the Method is defined in a package:

julia/base/methodshow.jl

Lines 215 to 232 in 0f95988

    
           try 
        
               d = dirname(file) 
        
               return LibGit2.with(LibGit2.GitRepoExt(d)) do repo 
        
                   LibGit2.with(LibGit2.GitConfig(repo)) do cfg 
        
                       u = LibGit2.get(cfg, "remote.origin.url", "") 
        
                       u = match(LibGit2.GITHUB_REGEX,u).captures[1] 
        
                       commit = string(LibGit2.head_oid(repo)) 
        
                       root = LibGit2.path(repo) 
        
                       if startswith(file, root) || startswith(realpath(file), root) 
        
                           "https://github.com/$u/tree/$commit/"*file[length(root)+1:end]*"#L$line" 
        
                       else 
        
                           fileurl(file) 
        
                       end 
        
                   end 
        
               end 
        
           catch 
        
               return fileurl(file) 
        
           end

I suggest that we simply delete it since it assumed that packages are git repos which will not be the case.

Figure out how to get documentation to pass with strict = true (see Move LibGit2 to stdlib #25706 (comment)). I have no idea about this, to be honest. These errors come when trying to reference LibGit2 docstrings from Base.map so one way would be to not have LibGit2 extend Base.map and instead define its own (similar to how it was done with push!).

Opinions?

JeffBezanson · 2018-01-31T15:59:04Z

I certainly wouldn't want to lose method URLs in jupyter; it's a cool feature.

KristofferC · 2018-02-05T15:23:04Z

Is bbf2224 acceptable as a way out of this deadlock?

JeffBezanson · 2018-02-05T17:22:06Z

Sure, let's do that for now.

KristofferC commented Jan 23, 2018

View reviewed changes

KristofferC added libgit2 The libgit2 library or the LibGit2 stdlib module excision Removal of code from Base or the repository stdlib Julia's standard library labels Jan 23, 2018

vchuravy reviewed Jan 23, 2018

View reviewed changes

waldyrious reviewed Jan 23, 2018

View reviewed changes

JeffBezanson added this to the 1.0 milestone Jan 23, 2018

vchuravy force-pushed the kc/excise_libgit2 branch from 8834afe to 9accacc Compare January 23, 2018 23:07

vchuravy mentioned this pull request Jan 23, 2018

Bootstrap Pkg3 and add it to stdlib #25714

Closed

1 task

KristofferC force-pushed the kc/excise_libgit2 branch from 9accacc to 103e875 Compare January 24, 2018 15:21

KristofferC force-pushed the kc/excise_libgit2 branch 5 times, most recently from f767eb8 to 494c570 Compare January 27, 2018 12:59

KristofferC closed this Jan 27, 2018

KristofferC reopened this Jan 27, 2018

maleadt mentioned this pull request Jan 27, 2018

Convenience API and bug fixes for git tree walking #25432

Merged

KristofferC force-pushed the kc/excise_libgit2 branch from 494c570 to 5f909ec Compare January 31, 2018 12:09

KristofferC added 2 commits February 5, 2018 14:57

move LibGit2 to stdlib

87913c6

check for prescence of LibGit2 in the root module in url of method

bbf2224

KristofferC force-pushed the kc/excise_libgit2 branch from 5f909ec to bbf2224 Compare February 5, 2018 13:58

KristofferC merged commit 9e36e01 into master Feb 5, 2018

KristofferC mentioned this pull request Feb 5, 2018

Make documentation pass with strict = true again. #25895

Closed

fredrikekre mentioned this pull request Mar 2, 2018

move Sockets out of Base #25935

Merged

fredrikekre deleted the kc/excise_libgit2 branch March 2, 2018 09:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move LibGit2 to stdlib #25706

Move LibGit2 to stdlib #25706

KristofferC commented Jan 23, 2018 •

edited

Loading

KristofferC Jan 23, 2018

KristofferC Jan 23, 2018

vchuravy Jan 23, 2018

KristofferC Jan 26, 2018

vchuravy Jan 26, 2018

omus commented Jan 23, 2018

waldyrious Jan 23, 2018

StefanKarpinski commented Jan 23, 2018

KristofferC commented Jan 24, 2018

KristofferC commented Jan 31, 2018 •

edited

Loading

JeffBezanson commented Jan 31, 2018

KristofferC commented Feb 5, 2018

JeffBezanson commented Feb 5, 2018

Move LibGit2 to stdlib #25706

Move LibGit2 to stdlib #25706

Conversation

KristofferC commented Jan 23, 2018 • edited Loading

KristofferC Jan 23, 2018

Choose a reason for hiding this comment

KristofferC Jan 23, 2018

Choose a reason for hiding this comment

vchuravy Jan 23, 2018

Choose a reason for hiding this comment

KristofferC Jan 26, 2018

Choose a reason for hiding this comment

vchuravy Jan 26, 2018

Choose a reason for hiding this comment

omus commented Jan 23, 2018

waldyrious Jan 23, 2018

Choose a reason for hiding this comment

StefanKarpinski commented Jan 23, 2018

KristofferC commented Jan 24, 2018

KristofferC commented Jan 31, 2018 • edited Loading

JeffBezanson commented Jan 31, 2018

KristofferC commented Feb 5, 2018

JeffBezanson commented Feb 5, 2018

KristofferC commented Jan 23, 2018 •

edited

Loading

KristofferC commented Jan 31, 2018 •

edited

Loading