Add bindings to git_indexer #911
Conversation
|
Sorry the test does not actually make sense, will amend |
kim
force-pushed
the
indexer
branch
2 times, most recently
from
d042e12 to
12a23d2
Compare
The indexer API is a lower-level interface for storing and indexing pack files, which, unlike `git_odb_write_pack`, allows the ouput to be written to an arbitrary directory. This can be useful when working with unusual validation requirements or non-standard repository layouts.
|
Amended to make odb optional |
|
@ehuss any feedback on this? |
src/indexer.rs
Outdated
| let odb = odb.map(Binding::raw).unwrap_or_else(ptr::null_mut); | ||
|
|
||
| let mut out = ptr::null_mut(); | ||
| let progress = MaybeUninit::uninit(); |
There was a problem hiding this comment.
I'm having trouble seeing where progress is ever initialized. I see it created here, and then a few calls to as_mut_ptr, but I don't see where it gets initialized before as_mut_ptr is called. Can you help me understand how this works?
There was a problem hiding this comment.
I'm not sure, actually, OdbPackwriter does the same. I assumed it works because MaybeUninit<T> has the same size and alignment as T, so the C side can just write to it (i.e. it is initialized after the first time we pass as_mut_ptr over to libgit2).
I don't know why this would be preferable over deriving Default for git_indexer_progress, and casting to a raw pointer where needed. If I made this change, would you want me to do it for OdbPackwriter as well?
There was a problem hiding this comment.
Yea, it looks like OdbPackwriter should also be updated. From what I can tell, the C side is writing to uninitialized memory which is UB to my understanding.
src/indexer.rs
Outdated
| let res = raw::git_indexer_append(self.raw, ptr, len, self.progress.as_mut_ptr()); | ||
|
|
||
| if res < 0 { | ||
| Err(io::Error::new(io::ErrorKind::Other, "Write error")) |
There was a problem hiding this comment.
This is unfortunate that it erases the libgit2 error. I assume you are just copying this from OdbWriter. I'm curious why this was implemented as io::Write instead of an inherent write method that would expose a native error message.
There was a problem hiding this comment.
I think io::Write makes sense because we can just io::copy the pack bytes from e.g. a network socket.
That being said, Error::last_error would indeed give the native error here (while it wouldn't for OdbPackwriter), so I'll change that.
Unlike for `OdbPackwriter`, we always get the native error code here.
The indexer API is a lower-level interface for storing and indexing pack
files, which, unlike
git_odb_write_pack, allows the ouput to bewritten to an arbitrary directory. This can be useful when working with
unusual validation requirements or non-standard repository layouts.