LibGit2: ABI update for 1.4.0 (and later)

[fxcoudert]

From the libgit2 release notes for 1.4.0:

Deprecated APIs

• git_index_checksum is deprecated; this information is now internal to the library and there is no replacement
• git_indexer_hash is deprecated; callers should use git_indexer_name to retrieve the filename
• git_packbuilder_hash is deprecated; callers should use git_packbuilder_name to retrieve the filename

ABI changes

• git_fetch_options now includes the follow_redirects value
• git_push_options now includes the follow_redirects value
• git_status_options now includes the rename_threshold value
• git_transport contains several changed function pointer signatures

The only things that seems used in Julia in this list are the git_fetch_options and git_push_options types, and related constants. This patch adds them.

---

Before this patch, a build julia master, from source, on macOS (aarch64-darwin) against a prebuilt libgit2 1.4.3 (the latest version, which is what Homebrew is shipping) let to a bootstrap failure, with a hang and crash in:

 cd /private/tmp/julia-20220521-36457-1h8lpec/base && if ! JULIA_BINDIR=/private/tmp/julia-20220521-36457-1h8lpec/usr/bin WINEPATH="/private/tmp/julia-20220521-36457-1h8lpec/usr/bin;$WINEPATH" JULIA_NUM_THREADS=1  /private/tmp/julia-20220521-36457-1h8lpec/usr/bin/julia -O3 -C "native" --output-o /private/tmp/julia-20220521-36457-1h8lpec/usr/lib/julia/sys-o.a.tmp  --startup-file=no --warn-overwrite=yes --sysimage /private/tmp/julia-20220521-36457-1h8lpec/usr/lib/julia/sys.ji /private/tmp/julia-20220521-36457-1h8lpec/contrib/generate_precompile.jl 1; then echo '*** This error is usually fixed by running `make clean`. If the error persists, try `make cleanall`. ***'; false; fi 
Generating REPL precompile statements... 25/40

(which involves the use of git through Pkg).

With this patch applied, the build now succeeds.

[fxcoudert]

Yggdrasil PR to build latest libgit2 (1.4.3) is opened at JuliaPackaging/Yggdrasil#4930

[fxcoudert]

So if I understand the process, now that @giordano has merged the Yggdrasil PR, I can update the dependency here.

[giordano]

Yes!

[fxcoudert]

I wasn't entirely sure about that, but it seemed like it needed to match the change you made in the other PR.

[giordano]

I think you have to change also the sonames at

julia/stdlib/LibGit2_jll/src/LibGit2_jll.jl

Lines 24 to 26 in 43786fb

	const libgit2 = "@rpath/libgit2.1.3.dylib"
	else
	const libgit2 = "libgit2.so.1.3"

The new soversion is 1.4 (breaking change, yay)

[fxcoudert]

Not sure what the errors are, and where they come from.

[giordano]

Failures are in libgit2 tests, so the problem looks relevant: https://build.julialang.org/#/builders/20/builds/5846/steps/5/logs/stdio

Error in testset LibGit2/libgit2:
Test Failed at /buildworker/worker/tester_linuxaarch64/build/share/julia/stdlib/v1.9/LibGit2/test/libgit2.jl:170
  Expression: findfirst(isequal(LibGit2.Consts.FEATURE_SSH), f) !== nothing
   Evaluated: nothing !== nothing

[fxcoudert]

After double-checking the code, the only think I can see is that my type and consts should be unsigned, because all values are positive and that's how other types are treated in the LibGit2 julia code. No sure if that will help, but updated the PR anyway.

[giordano]

Changing this to

Suggested change

	version = "1.4.3+0"
	version = "1.4.3+1"

(ref: JuliaPackaging/Yggdrasil#4931) and refreshing again the checksums should hopefully fix the test failure.

[giordano]

BTW, I think that JuliaPackaging/Yggdrasil#4931 (comment) means that we probably want to pass USE_SSH=ON also in

julia/deps/libgit2.mk

Line 16 in 390503e

LIBGIT2_OPTS := $(CMAKE_COMMON) -DCMAKE_BUILD_TYPE=Release -DTHREADSAFE=ON -DUSE_BUNDLED_ZLIB=ON

Note also that THREADSAFE is now called USE_THREADS and in

julia/deps/libgit2.mk

Line 33 in 390503e

LIBGIT2_OPTS += -DUSE_HTTPS="mbedTLS" -DSHA1_BACKEND="CollisionDetection" -DCMAKE_INSTALL_RPATH="\$$ORIGIN"

SHA1_BACKEND is now USE_SHA1.

[fxcoudert]

Pull request updated with:

• build-from-source options fixed
• new version of binary, new checksums

[fxcoudert]

Windows-only test failures 😢

[fxcoudert]

A new error code was introduced in libgit2/libgit2#6267, but was not noted in the release notes.

Should be handled now:

diff --git a/stdlib/LibGit2/src/error.jl b/stdlib/LibGit2/src/error.jl
index d742cde160..219b8cdf88 100644
--- a/stdlib/LibGit2/src/error.jl
+++ b/stdlib/LibGit2/src/error.jl
@@ -31,7 +31,8 @@ export GitError
             RETRY           = Cint(-32), # internal only
             EMISMATCH       = Cint(-33), # hashsum mismatch in object
             EINDEXDIRTY     = Cint(-34), # unsaved changes in the index would be overwritten
-            EAPPLYFAIL      = Cint(-35)) # patch application failed
+            EAPPLYFAIL      = Cint(-35), # patch application failed
+            EOWNER          = Cint(-36)) # the object is not owned by the current user
 
 @enum(Class, None,
              NoMemory,

[fxcoudert]

Still a Windows-only error, but different. Now it recognises that error code (EOWNER), but it's not what the test expects, it wants EAUTH.

Error During Test at C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\LibGit2\test\online.jl:38
  Got exception outside of a @test
  GitError(Code:EOWNER, Class:Config, repository path 'C:/Windows/TEMP/jl_eii1Ic/Example.TransferProgress/' is not owned by current user)
  Stacktrace:
    [1] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\LibGit2\src\error.jl:111 [inlined]
    [2] clone(repo_url::String, repo_path::String, clone_opts::LibGit2.CloneOptions)
      @ LibGit2 C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\LibGit2\src\repository.jl:459
    [3] clone(repo_url::String, repo_path::String; branch::String, isbare::Bool, remote_cb::Ptr{Nothing}, credentials::LibGit2.CredentialPayload, callbacks::Dict{Symbol, Tuple{Ptr{Nothing}, Any}})
      @ LibGit2 C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\LibGit2\src\LibGit2.jl:583
    [4] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\LibGit2\test\online.jl:49 [inlined]
    [5] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1385 [inlined]
    [6] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\LibGit2\test\online.jl:39 [inlined]
    [7] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1385 [inlined]
    [8] (::Main.Test72Main_LibGit2_online.LibGit2OnlineTests.var"#1#2")(dir::String)
      @ Main.Test72Main_LibGit2_online.LibGit2OnlineTests C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\LibGit2\test\online.jl:26
    [9] mktempdir(fn::Main.Test72Main_LibGit2_online.LibGit2OnlineTests.var"#1#2", parent::String; prefix::String)
      @ Base.Filesystem .\file.jl:760
   [10] mktempdir(fn::Function, parent::String) (repeats 2 times)
      @ Base.Filesystem .\file.jl:756
   [11] top-level scope
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\LibGit2\test\online.jl:22
   [12] include
      @ .\Base.jl:428 [inlined]
   [13] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:24 [inlined]
   [14] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1385 [inlined]
   [15] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:23 [inlined]
   [16] macro expansion
      @ .\timing.jl:466 [inlined]
   [17] runtests(name::String, path::String, isolate::Bool; seed::UInt128)
      @ Main C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:21
   [18] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, UInt128, Tuple{Symbol}, NamedTuple{(:seed,), Tuple{UInt128}}})
      @ Base .\essentials.jl:801
   [19] (::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}})()
      @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285
   [20] run_work_thunk(thunk::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}}, print_error::Bool)
      @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:70
   [21] macro expansion
      @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285 [inlined]
   [22] (::Distributed.var"#109#111"{Distributed.CallMsg{:call_fetch}, Distributed.MsgHeader, Sockets.TCPSocket})()
      @ Distributed .\task.jl:499
Test Failed at C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\LibGit2\test\online.jl:72
  Expression: ex.code ∈ (LibGit2.Error.EAUTH, LibGit2.Error.ERROR)
   Evaluated: LibGit2.Error.EOWNER ∈ (LibGit2.Error.EAUTH, LibGit2.Error.ERROR)
Test Failed at C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\LibGit2\test\online.jl:87
  Expression: ex.code == LibGit2.Error.EAUTH
   Evaluated: LibGit2.Error.EOWNER == LibGit2.Error.EAUTH
Dates/io                                (10) |    23.23 |   0.32 |  1.4 |     812.85 |   370.40
Artifacts                               (10) |        started at 2022-05-24T11:16:28.817

[fxcoudert]

So on Windows, all clone operations error out with repository path not owned by current user. There is only one place in libgit2 that can return this, and it was recently introduced. So either something in julia tests is setting weird ownership/permissions, or the libgit2 check is invalid on Windows. It would probably be visible from a simple C example, but I do not have a windows machine to test.

[fxcoudert]

I've reviewed the code, and I cannot see anything. A lot of libgit2 operations in tests fail with:

  Got exception outside of a @test
  GitError(Code:EOWNER, Class:Config, repository path 'C:/Windows/Temp/jl_cs2KGo/Example/' is not owned by current user)

which happens at every repo initialisation:

            LibGit2.with(LibGit2.init(cache_repo)) do repo

where cache_repo is defined by:

mktempdir() do dir
    dir = realpath(dir)
    # test parameters
    repo_url = "https://github.com/JuliaLang/Example.jl"
    cache_repo = joinpath(dir, "Example")

So either libgit2 ownership checks (recently introduced) are wrong on Windows, or the running process does not have ownership of the temporary directory C:/Windows/Temp/jl_cs2KGo/ (and the .git subdirectory created within it). Maybe a stranger ownership model for subdirectories of C:/Windows/Temp?

Could someone help me debug? Or let me know how we can use Julia CI to check this, by instrumenting one of the tests?

[fxcoudert]

Could someone with access to Julia on windows run this code to check? (on an unmodified julia)

julia> import LibGit2

julia> LibGit2.init("foo")
LibGit2.GitRepo("/private/tmp/foo")

julia> dir = mktempdir()
"/var/folders/h8/9hx_fyj91053ksgdzb2w03vw0000gp/T/jl_znV0dI"

julia> cache_repo = joinpath(dir, "Example")
"/var/folders/h8/9hx_fyj91053ksgdzb2w03vw0000gp/T/jl_znV0dI/Example"

julia> LibGit2.init(cache_repo)
LibGit2.GitRepo("/private/var/folders/h8/9hx_fyj91053ksgdzb2w03vw0000gp/T/jl_znV0dI/Example")

julia> stat(dir)
StatStruct for "/var/folders/h8/9hx_fyj91053ksgdzb2w03vw0000gp/T/jl_znV0dI"
   size: 96 bytes
 device: 16777234
  inode: 65583811
   mode: 0o040700 (drwx------)
  nlink: 3
    uid: 502 (fx)
    gid: 20 (staff)
   rdev: 0
  blksz: 4096
 blocks: 0
  mtime: 2022-06-18T17:00:27+0200 (32 seconds ago)
  ctime: 2022-06-18T17:00:27+0200 (32 seconds ago)

julia> stat(cache_repo)
StatStruct for "/var/folders/h8/9hx_fyj91053ksgdzb2w03vw0000gp/T/jl_znV0dI/Example"
   size: 96 bytes
 device: 16777234
  inode: 65583822
   mode: 0o040755 (drwxr-xr-x)
  nlink: 3
    uid: 502 (fx)
    gid: 20 (staff)
   rdev: 0
  blksz: 4096
 blocks: 0
  mtime: 2022-06-18T17:00:27+0200 (36 seconds ago)
  ctime: 2022-06-18T17:00:27+0200 (36 seconds ago)

---

With this code, I want to confirm that the bug we have is the same as libgit2/libgit2#6279, which is fixed upstream by libgit2/libgit2#6321

[tylerjthomas9]

I just tested this on Julia v1.7.3, Windows 11

julia> import LibGit2

julia> LibGit2.init("foo")
LibGit2.GitRepo("C:/Users/tyler/Downloads/foo")

julia> dir = mktempdir()
"C:\\Users\\tyler\\AppData\\Local\\Temp\\jl_b4EpSG"

julia> cache_repo = joinpath(dir, "Example")
"C:\\Users\\tyler\\AppData\\Local\\Temp\\jl_b4EpSG\\Example"

julia> LibGit2.init(cache_repo)
LibGit2.GitRepo("C:/Users/tyler/AppData/Local/Temp/jl_b4EpSG/Example")

julia> stat(dir)
StatStruct for "C:\\Users\\tyler\\AppData\\Local\\Temp\\jl_b4EpSG"
   size: 0 bytes
 device: 1006757745
  inode: 38768
   mode: 0o040666 (drw-rw-rw-)
  nlink: 1
    uid: 0
    gid: 0
   rdev: 0
  blksz: 4096
 blocks: 0
  mtime:  (5 seconds ago)
  ctime:  (5 seconds ago)

julia> stat(cache_repo)
StatStruct for "C:\\Users\\tyler\\AppData\\Local\\Temp\\jl_b4EpSG\\Example"
   size: 0 bytes
 device: 1006757745
  inode: 38763
   mode: 0o040666 (drw-rw-rw-)
  nlink: 1
    uid: 0
    gid: 0
   rdev: 0
  blksz: 4096
 blocks: 0
  mtime:  (15 seconds ago)
  ctime:  (15 seconds ago)

[fxcoudert]

Thanks @tylerjthomas9 everything seems to make sense, I've created a PR to include the upstream patch in yggdrasil builds: JuliaPackaging/Yggdrasil#5042

[fxcoudert]

Windows fixed. Some weird failures on some linux nodes, not sure what/why, it's a mess 😢

[giordano]

I think the test x86_64-linux-gnu simply timed out. I restarted it to see if we have better luck this time. The other failures on aarch64-linux-gnu and x86_64-linux-musl are sadly expected (see #45065 and #45638)

[fxcoudert]

Next x86_64-linux-gnu run had 1 failure in Sockets, not related to this PR.

[giordano]

I rebased on master now that libgit2 tests in musl should have been fixed by #45638, so we can actually test the upgrade there.

[fxcoudert]

There is one failure on x86_64-linux, which corresponds to a comment in #45638:

it appears that on glibc linux now we fail to read in the passphrase

So it appears to be preexisting, but showing up intermittently 🤷

[giordano]

Ok, let's merge this, at least we don't have regressions, and tests on musl are passing. For the record, some of the hangs we see in CI might be related to #45726, for which we have a PR.

Thanks a lot for the great sleuthing work to fix all issues!

[fxcoudert]

Could I ask that this be backported to 1.8.2? Or at least, the parts that provide forward build compatibility with libgit2 >= 1.4.0, i.e., this part:

diff --git a/stdlib/LibGit2/src/consts.jl b/stdlib/LibGit2/src/consts.jl
index 2bc9edaf89..55887ebe2a 100644
--- a/stdlib/LibGit2/src/consts.jl
+++ b/stdlib/LibGit2/src/consts.jl
@@ -247,6 +247,11 @@ const RESET_HARD  = Cint(3) # MIXED plus changes in working tree discarded
                             REBASE_OPERATION_FIXUP  = Cint(4),
                             REBASE_OPERATION_EXEC   = Cint(5))
 
+# git_remote_redirect_t
+const GIT_REMOTE_REDIRECT_NONE    = Cint(0)
+const GIT_REMOTE_REDIRECT_INITIAL = Cint(1)
+const GIT_REMOTE_REDIRECT_ALL     = Cint(2)
+
 # fetch_prune
 const FETCH_PRUNE_UNSPECIFIED = Cint(0)
 const FETCH_PRUNE             = Cint(1)
diff --git a/stdlib/LibGit2/src/types.jl b/stdlib/LibGit2/src/types.jl
index 9ffcaa3646..98d938df65 100644
--- a/stdlib/LibGit2/src/types.jl
+++ b/stdlib/LibGit2/src/types.jl
@@ -343,6 +343,9 @@ The fields represent:
     @static if LibGit2.VERSION >= v"0.25.0"
         proxy_opts::ProxyOptions       = ProxyOptions()
     end
+    @static if LibGit2.VERSION >= v"1.4.0"
+        follow_redirects::Cint         = Consts.GIT_REMOTE_REDIRECT_INITIAL
+    end
     @static if LibGit2.VERSION >= v"0.24.0"
         custom_headers::StrArrayStruct = StrArrayStruct()
     end
@@ -674,6 +677,9 @@ The fields represent:
     @static if LibGit2.VERSION >= v"0.25.0"
         proxy_opts::ProxyOptions       = ProxyOptions()
     end
+    @static if LibGit2.VERSION >= v"1.4.0"
+        follow_redirects::Cint         = Consts.GIT_REMOTE_REDIRECT_INITIAL
+    end
     @static if LibGit2.VERSION >= v"0.24.0"
         custom_headers::StrArrayStruct = StrArrayStruct()
     end

Currently, julia 1.8.x fails to build against recent libgit2 >= 1.4.0. In Homebrew, we have to patch it as above, and linux distros (like archlinux) do it as well. It would be nicer to actually provide this for source build.

In the official builds, since the dependency is not actually upgraded, it will change nothing. But it will make unpatched source builds possible for us.

[giordano]

I'm not sure we want to upgrade the version of a dependency in a patch release unless there is a strong need to, but the ABI compatibility fix should be fine. You can do that by opening a pull request targeting the backports-release-1.8 branch

[fxcoudert]

@giordano I have opened a PR for the ABI backport at #46795