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

Regression, calls to !git_error_last() not updated after commit 3618a2a #6801

Closed
jdavid opened this issue Apr 28, 2024 · 3 comments
Closed

Comments

@jdavid
Copy link
Member

jdavid commented Apr 28, 2024

This is a regression in libgit2 v1.8.0
Error introduced in commit 3618a2a
It shows up with Windows (winhttp).
Found in pygit2's test suite, see https://ci.appveyor.com/project/jdavid/pygit2/builds/49513776
It currently blocks the release of pygit2, see libgit2/pygit2#1285

Commit 3618a2a introduces this change:

@@ -184,6 +189,9 @@ const git_error *git_error_last(void)
        if ((threadstate = git_threadstate_get()) == NULL)
                return &tlsdata_error;
 
+       if (!threadstate->last_error)
+               return &no_error;
+
        return threadstate->last_error;
 }

But winhttp.c is not updated:

static int certificate_check(winhttp_stream *s, int valid)
{
    [...]

    if (error < 0 && !git_error_last())    <----
        git_error_set(GIT_ERROR_HTTP, "user cancelled certificate check");

Here since commit 3618a2a !git_error_last() always evaluates to false.

To find out where the issue is I made my fork of libgit2 and reverted the change introduced in commit 3618a2a. The result is that the tests pass, see https://ci.appveyor.com/project/jdavid/pygit2/builds/49705963

Of course I think the proper fix would be to update winhttp.c

A git grep shows that this same error happens in other places in libgit2:

$ git --no-pager grep "\!git_error_last()"
src/libgit2/transports/ssh_libssh2.c:		if (!git_error_last())
src/libgit2/transports/winhttp.c:		if (!git_error_last())
src/libgit2/transports/winhttp.c:	if (error < 0 && !git_error_last())
src/libgit2/transports/winhttp.c:				if (!git_error_last())
src/util/fs_path.c:			if (!git_error_last())

Reproduction steps

Error found in pygit2 CI (AppVeyor), see https://ci.appveyor.com/project/jdavid/pygit2/builds/49513776/job/v4dw1jrduwk69k20

Expected behavior

The test should pass.

Actual behavior

The test fails.

Version of libgit2 (release number or SHA1)

Error introduced in commit 3618a2a

Operating system(s) tested

Windows.

@ethomson
Copy link
Member

Thanks for the report — does https://github.com/libgit2/libgit2/compare/ethomson/cb_err?expand=1 resolve this?

@jdavid
Copy link
Member Author

jdavid commented Apr 29, 2024

@ethomson
Copy link
Member

We'll ship a v1.8.1 🔜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants