taskolib / libgit4cpp

C++ wrapper for libgit2 with limited functionality
https://taskolib.github.io/libgit4cpp/
GNU Lesser General Public License v2.1
1 stars 0 forks source link

Add missing branch_remote_name #20

Closed Finii closed 3 months ago

Finii commented 5 months ago

Step 1: Add proper Error object

[why]
The libgit2 has a lot of error codes that we migth want to transport.
At the moment that is not possible and we can just generate some random
text messages that are hard to decifer for our users.

[how]
Use std::system_error which is able to transport error codes.

Define the error categories for the git related error codes, so that the
standard library can convert the number to text.

Enable the test_Error.cc and put something meaningful into it.

Add some extra constructors to the Error object that just take a string.
They were used before when a std::exception has been utilized which can
not hold error codes. The extra constructor uses in this case the error
code GIT_EUSER as reserved exactly for this purpose by libgit2.

Step 2: Implement missing function

[why]
The function branch_remote_name() is already used but never defined.
This results in a library with missing symbols and when the function
depending on that symbol is called a crash.

[how]
There already was an almost complete but commented out implementation.
Add evaluation of the error code to that implementation and use the
Error object to raise an exception with code and explanantion.

Step 3 to n

Still missing are tests for the just implemented function.
But that is true for more wrapper functions.

Also the other wrapper functions do not transport the gitlib2 error code and they do not raise Error on errors, but the error handling is somehow rudimentary implemented by return values. That should be fixed.

Finii commented 5 months ago

Forgot to add the new file Error.cc :grimacing: just added the changes. Force push.
Hmm, there is still a (CI) problem with an invalid free, examining...

...

Hmm, inside the libgit2 library?

#4  0x00007ffff767d3d9 in __interceptor_free (ptr=0x555555795920) at ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:127
#5  0x00007ffff6eb3d62 in git_buf_dispose () from /lib/x86_64-linux-gnu/libgit2.so.28
#6  0x00007ffff6eb28fa in git_branch_remote_name () from /lib/x86_64-linux-gnu/libgit2.so.28
#7  0x00007ffff7561309 in git::branch_remote_name (repo=0x612000004b40, branch_name="master") at ../src/wrapper_functions.cc:178
#8  0x0000555555594304 in ____C_A_T_C_H____T_E_S_T____0 () at ../tests/test_GitRepository.cc:100
Finii commented 5 months ago

Fixed by calling the constructor of the git_buf :grimacing:

Also added code to free the buffer in case of a failure, as the docu does not say if or if not the buffer might get filled in that case.

Finii commented 4 months ago

Rebase on main, force push Now addressing things

Finii commented 4 months ago

Sigh. Directly relying on the values of the library is hard...

enum entry since
GIT_EINDEXDIRTY 0.28.0
GIT_EAPPLYFAIL 0.28.0
GIT_EOWNER 1.5.0
GIT_TIMEOUT 1.7.0
GIT_EUNCHANGED >1.7.2

Lets see *flexes fingers*

Finii commented 4 months ago

This has some explanations of the error categories etc (5 part series) http://blog.think-async.com/2010/04/system-error-support-in-c0x-part-1.html

Finii commented 3 months ago

Looks good, I especially like the new example.

The real magic part was missing, the implicit conversion to error_code; added that also to the example (and test).