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

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

Open
ilyagr opened this issue Apr 13, 2024 · 4 comments

Comments

@ilyagr
Copy link
Contributor

ilyagr commented Apr 13, 2024

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 04427a3. 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 ilyagr changed the title The error caused by failed push_negotiation is very strange and unhelpful The error caused by failed push_negotiation is strange: "config value 'pack.windowMemory' was not found" Apr 13, 2024
@ehuss
Copy link
Contributor

ehuss commented Apr 13, 2024

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 #1032 is merged.

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

@ilyagr
Copy link
Contributor Author

ilyagr commented Apr 13, 2024

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
Copy link
Contributor Author

ilyagr commented Apr 14, 2024

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

libgit2/libgit2#6675

@ehuss
Copy link
Contributor

ehuss commented Apr 15, 2024

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

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