Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A few libgit2 improvements/fixes #51259

Merged
merged 5 commits into from
Sep 18, 2023
Merged

A few libgit2 improvements/fixes #51259

merged 5 commits into from
Sep 18, 2023

Conversation

yuyichao
Copy link
Contributor

  • 1.7.0 support.
  • A few more operations on remote.
  • Getting commit parents.

@yuyichao yuyichao added libgit2 The libgit2 library or the LibGit2 stdlib module stdlib Julia's standard library labels Sep 10, 2023
@yuyichao yuyichao force-pushed the yyc/libgit2 branch 2 times, most recently from e9fe149 to 014ac86 Compare September 11, 2023 13:04
@giordano
Copy link
Contributor

If you change

version = "1.6.4+0"
to

version = "1.7.1+0"

and then run make -j -f contrib/refresh_checksums.mk libgit2 (I hope I got the name right) you can actually test the changes with the latest build of libgit2 we have in Yggdrasil. You'll also need to update deps/libgit2.version accordingly.

@yuyichao
Copy link
Contributor Author

I did the test locally on 1.7.1 already. Do you want me to run the test on the CI?

I haven't really done any extensive testing of the compatibility, just that tests passed and the functions I need myself works so I wasn't going to just bump the default version for everyone else...

@giordano
Copy link
Contributor

Yes, I think it'd be good to also upgrade the version of libgit2 we ship it julia, since we have the new build

@yuyichao
Copy link
Contributor Author

OK, sure. Added a new commit to bump the default version after roughly going through the libgit2 document to check if any other functions have any ABI breakage.

@yuyichao yuyichao force-pushed the yyc/libgit2 branch 2 times, most recently from d64caad to 91d2836 Compare September 12, 2023 02:28
@KristofferC
Copy link
Member

Seems to actually crash in LibGit2 tests on 32-bit windows:

From worker 16:
--
  | From worker 16:	Please submit a bug report with steps to reproduce this fault, and any error messages that follow (in their entirety). Thanks.
  | From worker 16:	Exception: EXCEPTION_ACCESS_VIOLATION at 0x8060004 -- unknown function (ip: 08060004)
  | From worker 16:	in expression starting at C:\buildkite-agent\builds\win2k22-amdci6-6\julialang\julia-master\julia-91d283606e\share\julia\stdlib\v1.11\LibGit2\test\online-tests.jl:22
  | From worker 16:	unknown function (ip: 08060004)
  | From worker 16:	git__clone at C:\buildkite-agent\builds\win2k22-amdci6-6\julialang\julia-master\julia-91d283606e\bin\libgit2.DLL (unknown line)
  | From worker 16:	git_clone at C:\buildkite-agent\builds\win2k22-amdci6-6\julialang\julia-master\julia-91d283606e\bin\libgit2.DLL (unknown line)
  | From worker 16:	macro expansion at C:\workdir\usr\share\julia\stdlib\v1.11\LibGit2\src\error.jl:109 [inlined]
  | From worker 16:	clone at C:\workdir\usr\share\julia\stdlib\v1.11\LibGit2\src\repository.jl:459
  | From worker 16:	#clone#123 at C:\workdir\usr\share\julia\stdlib\v1.11\LibGit2\src\LibGit2.jl:583
  | From worker 16:	clone at C:\workdir\usr\share\julia\stdlib\v1.11\LibGit2\src\LibGit2.jl:556 [inlined]
  | From worker 16:	macro expansion at C:\buildkite-agent\builds\win2k22-amdci6-6\julialang\julia-master\julia-91d283606e\share\julia\stdlib\v1.11\LibGit2\test\online-tests.jl:29 [inlined]
  | From worker 16:	macro expansion at C:\workdir\usr\share\julia\stdlib\v1.11\Test\src\Test.jl:1599 [inlined]
  | From worker 16:	macro expansion at C:\buildkite-agent\builds\win2k22-amdci6-6\julialang\julia-master\julia-91d283606e\share\julia\stdlib\v1.11\LibGit2\test\online-tests.jl:27 [inlined]
  | From worker 16:	macro expansion at C:\workdir\usr\share\julia\stdlib\v1.11\Test\src\Test.jl:1599 [inlined]
  | From worker 16:	#1 at C:\buildkite-agent\builds\win2k22-amdci6-6\julialang\julia-master\julia-91d283606e\share\julia\stdlib\v1.11\LibGit2\test\online-tests.jl:26

@yuyichao
Copy link
Contributor Author

There was indeed a missing ABI breaking change. The fetch options struct got an extra member depth.

I looked through the diff between the 1.7.0 and 1.6.4 header this time and I'm fairly sure there isn't anything missing anymore. OTOH, it seems that some of the constants (enums) aren't up to date. Most of the ones that are changed this time have missing updates from previous versions. I didn't check the enum values that didn't change in this version..

@yuyichao yuyichao force-pushed the yyc/libgit2 branch 3 times, most recently from 2e1001d to 1350ca8 Compare September 12, 2023 14:38
@giordano giordano added building Build system, or building Julia or its dependencies JLLs labels Sep 12, 2023
stdlib/LibGit2/src/commit.jl Outdated Show resolved Hide resolved
@yuyichao
Copy link
Contributor Author

Updated for #51294

@KristofferC KristofferC merged commit 4a311d6 into master Sep 18, 2023
1 check passed
@KristofferC KristofferC deleted the yyc/libgit2 branch September 18, 2023 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies JLLs libgit2 The libgit2 library or the LibGit2 stdlib module stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants