git2-rs icon indicating copy to clipboard operation
git2-rs copied to clipboard

WIP: ( feat: create function to allow setting commit_create_cb while rebasing )

Open Pranav2612000 opened this issue 1 year ago • 7 comments

Fixes #850

Pranav2612000 avatar Apr 28 '24 13:04 Pranav2612000

I'll make the pipelines pass but I just wanted to get an early review on the approach. Do you think this could work? The commit_create_cb_op expects a callback fn which has arguments slightly different from the one expected by raw.commit_create_cb ( e.g Oid instead of *mut git_oid, Signature instead of *const git_signature ) but the rust typechecker did not throw an error.

Pranav2612000 avatar Apr 28 '24 13:04 Pranav2612000

Just throwing my opinion on here since I was tagged on the issue: I don't think that transmute is safe (the rust parameter types probably don't map cleanly to c ffi). I would use something like this: (example from my job) https://github.com/Vector35/binaryninja-api/blob/dev/rust/src/debuginfo.rs#L181-L246 where Rust puts all of its information into a Box type provided to the userdata (void*) parameter of the callback, and a static callback function which takes the boxed data and interprets it properly.

CouleeApps avatar Apr 29 '24 07:04 CouleeApps

remote_callbacks.rs has some examples for handling callbacks (using an extern "C" callback with a void * payload for the function pointer). That might provide some inspiration.

ehuss avatar May 01 '24 19:05 ehuss

Thank you for the help @CouleeApps @ehuss I'll take a look at the code you shared and take another stab at it soon

Pranav2612000 avatar May 02 '24 02:05 Pranav2612000

@ehuss I've followed an approach similar to the one used in remote_callback.rs. Can you take a look again?

Pranav2612000 avatar May 11 '24 13:05 Pranav2612000

Also, I'm testing the changes through an application I have and passing GITPASSTHROUGH works as expected, but passing other values, which should result in the stopping and returning a failure crashes the app. The error is inside rebase.commit func. Here's a sample code I'm using

                if let Ok(commit_id) = rebase.commit(None, &committer.clone().into(), None) {
                    last_rebase_head = commit_id.into();
                } else {
                    rebase_success = false;
                    break;
                }

I've added print statements and ensured that it doesn't reach the else block before crashing.

Do you know what may be happening?

Pranav2612000 avatar May 11 '24 13:05 Pranav2612000

Bumping this up

Pranav2612000 avatar Jun 02 '24 13:06 Pranav2612000