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.

Sign up for GitHub

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

Jump to bottom

LibGit2: add resolve_url to RemoteCallbacksStruct for LibGit2 0.99.0 #35232

Merged
merged 3 commits into from
May 29, 2020
Merged

LibGit2: add resolve_url to RemoteCallbacksStruct for LibGit2 0.99.0 #35232

merged 3 commits into from
May 29, 2020

Conversation

diabonas
Copy link
Contributor

Upstream commit libgit2/libgit2@59647e1 ("remote: add callback to resolve URLs before connecting") introduced a new callback resolve_url in LibGit2 0.99.0. Even though it is not currently used in Julia, it needs to be accounted for to get the correct size for FetchOptionsStruct. An incorrectly aligned FetchOptionsStruct leads to error messages like "invalid version 0 on git_proxy_options" when trying to use the latest LibGit2 version.

Fixes: GH-35043
Fixes: FS#65540

This was referenced Mar 23, 2020
Closed
Closed
Merged
@nalimilan
Copy link
Member

Ah too bad, looks like we were both debugging this at the same time. Though this isn't enough to get tests to pass on 0.99.0, see #35233.

@diabonas
Copy link
Contributor Author

Interesting, I ran the test suite while building the Arch Linux package, but apparently the LibGit2 tests are disabled there... The only reason why I could see the tests failing is the changed error message fa4481b#diff-d9d05c74120373a1756ee4a7e86c7cfeR3046. The other changes in your PR seem to be implemented in a backward-compatible way in LibGit2 0.99.0, so are not strictly required (but certainly make sense for the latest version).

That said, your PR is a lot more comprehensive than mine, so seems to be the better option going forward, if backwards-compatibility with older LibGit2 versions is not required (I don't know how this is usually handled in Julia).

@diabonas
Copy link
Contributor Author

For completeness I included the two changes from your PR that are necessary to make the unit test pass in order to make this a minimal self-contained patch that works with all versions of LibGit2.

@diabonas
Copy link
Contributor Author

The test failure for the Windows build appears to be unrelated:

Error in testset errorshow:
Test Failed at D:\buildbot\worker-tabularasa\tester_win32\build\share\julia\test\errorshow.jl:640
  Expression: occursin("the last 2 lines are repeated 5000 more times", output[5])
   Evaluated: occursin("the last 2 lines are repeated 5000 more times", " ... (the last 2 lines are repeated 4998 more times)")
Error in testset errorshow:
Test Failed at D:\buildbot\worker-tabularasa\tester_win32\build\share\julia\test\errorshow.jl:641
  Expression: (output[6])[1:8] == " [10003]"
   Evaluated: " [9999] " == " [10003]"

nalimilan
nalimilan approved these changes Mar 23, 2020
View reviewed changes
Copy link
Member

@nalimilan nalimilan left a comment

Choose a reason for hiding this comment

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

Thanks! I guess we should backport this to 1.4, then we can sort out the libgit2 0.99.1 build issues in #35233 for master.

@ivan-timokhin ivan-timokhin mentioned this pull request Apr 2, 2020
Closed
@KristofferC KristofferC mentioned this pull request Apr 4, 2020
Merged
27 tasks
@KristofferC KristofferC mentioned this pull request May 10, 2020
Merged
14 tasks
@simonbyrne simonbyrne added the libgit2 The libgit2 library or the LibGit2 stdlib module label May 28, 2020
@simonbyrne simonbyrne closed this May 28, 2020
@simonbyrne simonbyrne reopened this May 28, 2020
diabonas and others added 3 commits May 28, 2020 14:17
@diabonas @simonbyrne
LibGit2: add resolve_url to RemoteCallbacksStruct for LibGit2 0.99.0
98fa2f3 
Upstream commit
libgit2/libgit2@59647e1
("remote: add callback to resolve URLs before connecting") introduced a
new callback "resolve_url" in LibGit2 0.99.0. Even though it is not
currently used in Julia, it needs to be accounted for to get the correct
size for FetchOptionsStruct. An incorrectly aligned FetchOptionsStruct
leads to error messages like "invalid version 0 on git_proxy_options"
when trying to use the latest LibGit2 version.
@diabonas @simonbyrne
LibGit2: update error message checking for LibGit2 0.99.0
375f838 
Upstream commit
libgit2/libgit2@b9c5b15
("http: use the new httpclient") changed the error message to include
additional quotes. Relax the unit test to allow these if present.
@nalimilan @simonbyrne
LibGit2: amend GitError enum
75b8e14 
Since upstream commit
libgit2/libgit2@e9cef7c
("http: introduce GIT_ERROR_HTTP") an invalid content type yields a
GIT_ERROR_HTTP instead of a GIT_ERROR_NET error. Update the enum to
include this new error so that the unit test for LibGit2 doesn't fail
with "invalid value for Enum Class: 34".
@simonbyrne simonbyrne merged commit 59a315c into JuliaLang:master May 29, 2020
KristofferC pushed a commit that referenced this pull request Jun 1, 2020
@diabonas @KristofferC
LibGit2: add resolve_url to RemoteCallbacksStruct for LibGit2 0.99.0 (#…
53f0393 
…35232)

* LibGit2: amend GitError enum

Since upstream commit
libgit2/libgit2@e9cef7c
("http: introduce GIT_ERROR_HTTP") an invalid content type yields a
GIT_ERROR_HTTP instead of a GIT_ERROR_NET error. Update the enum to
include this new error so that the unit test for LibGit2 doesn't fail
with "invalid value for Enum Class: 34".

* LibGit2: add resolve_url to RemoteCallbacksStruct for LibGit2 0.99.0

Upstream commit
libgit2/libgit2@59647e1
("remote: add callback to resolve URLs before connecting") introduced a
new callback "resolve_url" in LibGit2 0.99.0. Even though it is not
currently used in Julia, it needs to be accounted for to get the correct
size for FetchOptionsStruct. An incorrectly aligned FetchOptionsStruct
leads to error messages like "invalid version 0 on git_proxy_options"
when trying to use the latest LibGit2 version.

* LibGit2: update error message checking for LibGit2 0.99.0

Upstream commit
libgit2/libgit2@b9c5b15
("http: use the new httpclient") changed the error message to include
additional quotes. Relax the unit test to allow these if present.

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
(cherry picked from commit 59a315c)
@KristofferC KristofferC mentioned this pull request Jun 1, 2020
Merged
26 tasks
@diabonas diabonas deleted the fix-libgit2-0.99-abi branch June 2, 2020 19:38
simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 11, 2020
@diabonas @nalimilan @simeonschaub
LibGit2: add resolve_url to RemoteCallbacksStruct for LibGit2 0.99.0 (J…
c0e3caa 
…uliaLang#35232)

* LibGit2: amend GitError enum

Since upstream commit
libgit2/libgit2@e9cef7c
("http: introduce GIT_ERROR_HTTP") an invalid content type yields a
GIT_ERROR_HTTP instead of a GIT_ERROR_NET error. Update the enum to
include this new error so that the unit test for LibGit2 doesn't fail
with "invalid value for Enum Class: 34".

* LibGit2: add resolve_url to RemoteCallbacksStruct for LibGit2 0.99.0

Upstream commit
libgit2/libgit2@59647e1
("remote: add callback to resolve URLs before connecting") introduced a
new callback "resolve_url" in LibGit2 0.99.0. Even though it is not
currently used in Julia, it needs to be accounted for to get the correct
size for FetchOptionsStruct. An incorrectly aligned FetchOptionsStruct
leads to error messages like "invalid version 0 on git_proxy_options"
when trying to use the latest LibGit2 version.

* LibGit2: update error message checking for LibGit2 0.99.0

Upstream commit
libgit2/libgit2@b9c5b15
("http: use the new httpclient") changed the error message to include
additional quotes. Relax the unit test to allow these if present.

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nalimilan nalimilan nalimilan approved these changes

Assignees
No one assigned
Labels
libgit2 The libgit2 library or the LibGit2 stdlib module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: compatibility with http-parser 2.9.3 and libgit2 0.99.0-2
5 participants
@diabonas @nalimilan @JeffBezanson @KristofferC @simonbyrne