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

The error caused by failed `push_negotiation` is strange: `"config value 'pack.windowMemory' was not found"`

Open ilyagr opened this issue 1 year ago • 4 comments

First of all, thank you so much for implementing this!

I'd like to be able to tell whether a push failure is caused by a failed push negotiation. However, the error currently returned by a push negotiation is very strange. For example, I added a simple dbg! to the provided test:

diff --git a/src/remote.rs b/src/remote.rs
index a15a095010...2a4816b245 100644
--- a/src/remote.rs
+++ b/src/remote.rs
@@ -1060,9 +1060,7 @@
             });
             let mut options = PushOptions::new();
             options.remote_callbacks(callbacks);
-            assert!(remote
-                .push(&["refs/heads/main"], Some(&mut options))
-                .is_err());
+            assert!(dbg!(remote.push(&["refs/heads/main"], Some(&mut options))).is_err());
         }
         assert!(updated);
         assert_eq!(remote_repo.branches(None).unwrap().count(), 0);

The dbg! shows that the error is the following unhelpful, and likely unintended, error:

[src/remote.rs:1063:21] remote.push(&["refs/heads/main"], Some(&mut options)) = Err(
    Error {
        code: -1,
        klass: 7,
        message: "config value 'pack.windowMemory' was not found",
    },
)

I'm not sure what it means.

This is at commit 04427a3a7c96603ba3cfef452fb218b1590b5dbb. I'm on Apple Silicon Mac OS Sonoma 14.4.1:

$ rustc -Vv
rustc 1.79.0-nightly (4fd4797c2 2024-04-03)
binary: rustc
commit-hash: 4fd4797c2654977f545c9a91e2aa4e6cdbb38919
commit-date: 2024-04-03
host: aarch64-apple-darwin
release: 1.79.0-nightly
LLVM version: 18.1.2

$ uname -a
Darwin macaw.local 23.4.0 Darwin Kernel Version 23.4.0:
Fri Mar 15 00:12:49 PDT 2024;
root:xnu-10063.101.17~1/RELEASE_ARM64_T6020 arm64 arm Darwin

Cc: @karin0 @ehuss @samueltardieu #926

ilyagr avatar Apr 13 '24 06:04 ilyagr

Thanks for the report! That is indeed weird. It looks like this has been fixed in 1.8, which responds with the error:

Error { code: -1, klass: 26, message: "push_negotiation callback returned -1" }

So I believe this should be resolved once https://github.com/rust-lang/git2-rs/pull/1032 is merged.

It doesn't report the error from the callback, though. I opened #1043 to fix that (and a few other places).

ehuss avatar Apr 13 '24 22:04 ehuss

Sounds great, thank you very much!

How would I check for this error in code? Is there a name for klass: 26?

I also realized that even if there isn't a good way, this should be easy to work around. I can probably just record the error myself (and optionally record any data I want about it):

let mut push_negotiation_error = false;
let mut callbacks = RemoteCallbacks::new();
callbacks.push_negotiation(|updates| {
    if condition {
       push_negotiation_error = true
       Err(crate::Error::from_str("rejected"))
    } else {
       Ok(())
    }
});
let mut options = PushOptions::new();
options.remote_callbacks(callbacks);
let result = remote.push(&["refs/heads/main"], Some(&mut options));

if push_negotiation_error {
  assert!(result.is_err());
  // Handle push negotiation error
}
if let Err(err) = result {
  // Handle other errors
}

ilyagr avatar Apr 13 '24 22:04 ilyagr

Here is the link to the libgit2 PR that fixed this issue (for version 1.8.0); it has some more details.

https://github.com/libgit2/libgit2/pull/6675

ilyagr avatar Apr 14 '24 00:04 ilyagr

I believe class 26 is ErrorClass::Callback, which can be obtained from Error::class.

ehuss avatar Apr 15 '24 17:04 ehuss