Skip to content

Commit

Permalink
Merge branch 'push-wwxrqxuzmolm'
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Apr 22, 2024
2 parents 68fd5b3 + 4faf10e commit 048e43e
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 67 deletions.
2 changes: 1 addition & 1 deletion gix-ref/src/store/file/transaction/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,8 @@ impl<'s, 'p> Transaction<'s, 'p> {
..edit.update.clone()
});
*num_updates += 1;
continue;
}
continue;
}
match edit.update.change {
Change::Update {
Expand Down
67 changes: 35 additions & 32 deletions gix-ref/tests/file/store/find.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
mod existing {
use gix_ref::{PartialName, PartialNameRef};

use crate::{file::store_at, hex_to_id};

#[test]
Expand All @@ -13,12 +11,41 @@ mod existing {
Ok(())
}

// TODO: figure this out
#[test]
fn possible_inputs() -> crate::Result {
let store = crate::file::store()?;
store.find_loose("dt1")?;
store.find_loose(&String::from("dt1"))?; // Owned Strings don't have an impl for PartialName
mod convert {
use gix_ref::{PartialName, PartialNameRef};

// TODO: figure this out
#[test]
fn possible_inputs() -> crate::Result {
let store = crate::file::store()?;
store.find_loose("dt1")?;
store.find_loose(&String::from("dt1"))?; // Owned Strings don't have an impl for PartialName

store.find_loose(&CustomType("dt1".into()))?;

let name = CustomName {
remote: "origin",
branch: "main",
};
store.find_loose(&name.to_partial_name())?;
// TODO: this effectively needs a `Cow<'_, PartialNameRef>`, but we are not allowed to implement conversions for it.
// After having been there, I don't want to have a `PartialNameCow(Cow<'_, PartialNameRef)` anymore, nor
// copies of `TryFrom/TryInto` traits in our crate.
// Make it work once we can implement standard traits for Cow<OurType>.
// store.find_loose(&name)?;
// store.find_loose(name.to_partial_name())?;
store.find_loose(&name.to_partial_name_from_string())?;
store.find_loose(&name.to_partial_name_from_bstring())?;
store.find_loose(&name.to_full_name())?;
store.find_loose(name.to_full_name().as_ref())?;
store.find_loose(name.to_full_name().as_ref().as_partial_name())?;
store.find_loose(&PartialName::try_from(name.remote)?.join(name.branch.into())?)?;
store.find_loose(&PartialName::try_from("origin")?.join("main".into())?)?;
store.find_loose(&PartialName::try_from("origin")?.join(String::from("main").as_str().into())?)?;
store.find_loose(&PartialName::try_from("origin")?.join("main".into())?)?;

Ok(())
}

struct CustomType(String);
impl<'a> TryFrom<&'a CustomType> for &'a PartialNameRef {
Expand All @@ -28,7 +55,6 @@ mod existing {
value.0.as_str().try_into()
}
}
store.find_loose(&CustomType("dt1".into()))?;

struct CustomName {
remote: &'static str,
Expand Down Expand Up @@ -61,29 +87,6 @@ mod existing {
PartialName::try_from(value.to_partial_name())
}
}

let name = CustomName {
remote: "origin",
branch: "main",
};
store.find_loose(&name.to_partial_name())?;
// TODO: this effectively needs a `Cow<'_, PartialNameRef>`, but we are not allowed to implement conversions for it.
// After having been there, I don't want to have a `PartialNameCow(Cow<'_, PartialNameRef)` anymore, nor
// copies of `TryFrom/TryInto` traits in our crate.
// Make it work once we can implement standard traits for Cow<OurType>.
// store.find_loose(&name)?;
// store.find_loose(name.to_partial_name())?;
store.find_loose(&name.to_partial_name_from_string())?;
store.find_loose(&name.to_partial_name_from_bstring())?;
store.find_loose(&name.to_full_name())?;
store.find_loose(name.to_full_name().as_ref())?;
store.find_loose(name.to_full_name().as_ref().as_partial_name())?;
store.find_loose(&PartialName::try_from(name.remote)?.join(name.branch.into())?)?;
store.find_loose(&PartialName::try_from("origin")?.join("main".into())?)?;
store.find_loose(&PartialName::try_from("origin")?.join(String::from("main").as_str().into())?)?;
store.find_loose(&PartialName::try_from("origin")?.join("main".into())?)?;

Ok(())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -813,3 +813,39 @@ fn packed_refs_creation_with_packed_refs_mode_leave_keeps_original_loose_refs()
);
Ok(())
}

#[test]
fn packed_refs_deletion_in_deletions_and_updates_mode() -> crate::Result {
let (_keep, store) = store_writable("make_packed_ref_repository.sh")?;
assert!(
store.try_find_loose("refs/heads/d1")?.is_none(),
"no loose d1 available, it's packed"
);
let odb = gix_odb::at(store.git_dir().join("objects"))?;
let old_id = hex_to_id("134385f6d781b7e97062102c6a483440bfda2a03");
let edits = store
.transaction()
.packed_refs(PackedRefs::DeletionsAndNonSymbolicUpdates(Box::new(odb)))
.prepare(
Some(RefEdit {
change: Change::Delete {
expected: PreviousValue::MustExistAndMatch(Target::Peeled(old_id)),
log: RefLog::AndReference,
},
name: "refs/heads/d1".try_into()?,
deref: false,
}),
Fail::Immediately,
Fail::Immediately,
)?
.commit(committer().to_ref())?;

assert_eq!(edits.len(), 1, "only one edit was performed in the packed refs store");

let packed = store.open_packed_buffer().unwrap().expect("packed refs is available");
assert!(
packed.try_find("refs/heads/d1")?.is_none(),
"d1 should be removed from packed refs"
);
Ok(())
}
69 changes: 35 additions & 34 deletions gix-transport/src/client/blocking_io/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,45 +143,46 @@ struct ReadStdoutFailOnError {
read: std::process::ChildStdout,
}

impl std::io::Read for ReadStdoutFailOnError {
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
let res = self.read.read(buf);
self.swap_err_if_present_in_stderr(buf.len(), res)
}
}

impl ReadStdoutFailOnError {
fn swap_err_if_present_in_stderr(&self, wanted: usize, res: std::io::Result<usize>) -> std::io::Result<usize> {
match self.recv.try_recv().ok() {
Some(err) => Err(err),
None => match res {
Ok(n) if n == wanted => Ok(n),
Ok(n) => {
// TODO: fix this
// When parsing refs this seems to happen legitimately
// (even though we read packet lines only and should always know exactly how much to read)
// Maybe this still happens in `read_exact()` as sometimes we just don't get enough bytes
// despite knowing how many.
// To prevent deadlock, we have to set a timeout which slows down legitimate parts of the protocol.
// This code was specifically written to make the `cargo` test-suite pass, and we can reduce
// the timeouts even more once there is a native ssh transport that is used by `cargo`, it will
// be able to handle these properly.
// Alternatively, one could implement something like `read2` to avoid blocking on stderr entirely.
self.recv
.recv_timeout(std::time::Duration::from_millis(5))
.ok()
.map_or(Ok(n), Err)
}
Err(err) => Err(self.recv.recv().ok().unwrap_or(err)),
},
}
}
}

fn supervise_stderr(
ssh_kind: ssh::ProgramKind,
stderr: std::process::ChildStderr,
stdout: std::process::ChildStdout,
) -> ReadStdoutFailOnError {
impl ReadStdoutFailOnError {
fn swap_err_if_present_in_stderr(&self, wanted: usize, res: std::io::Result<usize>) -> std::io::Result<usize> {
match self.recv.try_recv().ok() {
Some(err) => Err(err),
None => match res {
Ok(n) if n == wanted => Ok(n),
Ok(n) => {
// TODO: fix this
// When parsing refs this seems to happen legitimately
// (even though we read packet lines only and should always know exactly how much to read)
// Maybe this still happens in `read_exact()` as sometimes we just don't get enough bytes
// despite knowing how many.
// To prevent deadlock, we have to set a timeout which slows down legitimate parts of the protocol.
// This code was specifically written to make the `cargo` test-suite pass, and we can reduce
// the timeouts even more once there is a native ssh transport that is used by `cargo`, it will
// be able to handle these properly.
// Alternatively, one could implement something like `read2` to avoid blocking on stderr entirely.
self.recv
.recv_timeout(std::time::Duration::from_millis(5))
.ok()
.map_or(Ok(n), Err)
}
Err(err) => Err(self.recv.recv().ok().unwrap_or(err)),
},
}
}
}
impl std::io::Read for ReadStdoutFailOnError {
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
let res = self.read.read(buf);
self.swap_err_if_present_in_stderr(buf.len(), res)
}
}

let (send, recv) = std::sync::mpsc::sync_channel(1);
std::thread::Builder::new()
.name("supervise ssh stderr".into())
Expand Down

0 comments on commit 048e43e

Please sign in to comment.