Correct/relax lifetime requirements on getter methods of Commit and Signature

[bcmyers]

This PR fixes #895

It is very similar to https://github.com/rust-lang/git2-rs/pull/302, which addressed a similar issue: https://github.com/rust-lang/git2-rs/issues/299

Overview

On the Commit struct, there are several "getter" methods where the lifetime of the return object is tied to a borrow of Self:

• pub fn author(&self) -> Signature<'_> {
• pub fn body(&self) -> Option<&str> {
• Etc.

In other words, written without elision, these signatures all look like

• pub fn author<'a>(&'a self) -> Signature<'a> {
• pub fn body<'a>(&'a self) -> Option<&'a str> {
• Etc.

But the pointers that are returned by these methods all live as long as the 'repo lifetime if you look at the C code. In other words, they should all have these (more flexible) signatures:

struct Commit<'repo> { ... }

impl<'repo> Commit<'repo> {
    pub fn author(&self) -> Signature<'repo> { .. } 
    pub fn body(&self) -> Option<&'repo str> { .. }
    // Etc.
}

There is a very similar problem happening for the "getter" methods of the Signature struct.

This PR fixes these methods so that they return references with the correct (more flexible) lifetimes.

Motivating example

This code should compile

struct Wrapper<'repo> {
    inner: git2::Commit<'repo>,
}

impl<'repo> Wrapper<'repo> {
    fn email(&self) -> Option<&'repo str> {
        let signature = self.inner.author();
        let email = signature.email();
        email
    }
}

but fails with

error: lifetime may not live long enough
 --> src/lib.rs:9:9
  |
5 | impl<'repo> Wrapper<'repo> {
  |      ----- lifetime `'repo` defined here
6 |     fn email(&self) -> Option<&'repo str> {
  |              - let's call the lifetime of this reference `'1`
...
9 |         email
  |         ^^^^^ associated function was supposed to return data with lifetime `'repo` but it is returning data with lifetime `'1`

error[E0515]: cannot return value referencing local variable `signature`
 --> src/lib.rs:9:9
  |
8 |         let email = signature.email();
  |                     ----------------- `signature` is borrowed here
9 |         email
  |         ^^^^^ returns a value referencing data owned by the current function

For more information about this error, try `rustc --explain E0515`.

The C code

The equivalent "getter" methods are defined in the underlying C code is here:

• https://github.com/libgit2/libgit2/blob/a3841af5eecc6301e87f8302c7fdce6555e39247/src/libgit2/commit.c#L521

The pointers they return live here:

• https://github.com/libgit2/libgit2/blob/main/src/libgit2/commit.h#L19

I believe all these pointers live as long as this raw *mut raw::git_commit pointer here:

• https://github.com/rust-lang/git2-rs/blob/master/src/commit.rs#L15

which has the 'repo lifetime.

[ehuss]

Thanks for the PR! However, I feel like this isn't correct.

Looking at author() specifically, the git_signature is owned by the git_commit object. The function git_commit__free takes care of freeing the signature. Although git objects are reference counted, I believe there are scenarios where those internal caches can get cleared which can evict those objects. Once the object is freed, it will free the signature, leaving the Rust Signature object pointing at already freed memory.

[bcmyers]

Yep. I think you're right. Thanks for looking into this. I'll close this out.