diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 00000000000..e3242898067 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,105 @@ +# Copilot Instructions for Gitoxide + +This repository contains `gitoxide` - a pure Rust implementation of Git. This document provides guidance for GitHub Copilot when working with this codebase. + +## Project Overview + +- **Language**: Rust (MSRV documented in gix/Cargo.toml) +- **Structure**: Cargo workspace with multiple crates (gix-*, gitoxide-core, etc.) +- **Main crates**: `gix` (library entrypoint), `gitoxide` binary (CLI tools: `gix` and `ein`) +- **Purpose**: Provide a high-performance, safe Git implementation with both library and CLI interfaces + +## Development Practices + +### Test-First Development +- Protect against regression and make implementing features easy +- Keep it practical - the Rust compiler handles mundane things +- Use git itself as reference implementation; run same tests against git where feasible +- Never use `.unwrap()` in production code, avoid it in tests in favor of `.expect()` or `?`. Use `gix_testtools::Result` most of the time. +- Use `.expect("why")` with context explaining why expectations should hold, but only if it's relevant to the test. + +### Error Handling +- Handle all errors, never `unwrap()` +- Provide error chains making it easy to understand what went wrong +- Use `thiserror` for libraries generally +- Binaries may use `anyhow::Error` exhaustively (user-facing errors) + +### Commit Messages +Follow "purposeful conventional commits" style: +- Use conventional commit prefixes ONLY if message should appear in changelog +- Breaking changes MUST use suffix `!`: `change!:`, `remove!:`, `rename!:` +- Features/fixes visible to users: `feat:`, `fix:` +- Refactors/chores: no prefix (don't affect users) +- Examples: + - `feat: add Repository::foo() to do great things. (#234)` + - `fix: don't panic when calling foo() in a bare repository. (#456)` + - `change!: rename Foo to Bar. (#123)` + +### Code Style +- Follow existing patterns in the codebase +- No `.unwrap()` - use `.expect("context")` if you are sure this can't fail. +- Prefer references in plumbing crates to avoid expensive clones +- Use `gix_features::threading::*` for interior mutability primitives + +### Path Handling +- Paths are byte-oriented in git (even on Windows via MSYS2 abstraction) +- Use `gix::path::*` utilities to convert git paths (`BString`) to `OsStr`/`Path` or use custom types + +## Building and Testing + +### Quick Commands +- `just test` - Run all tests, clippy, journey tests, and try building docs +- `just check` - Build all code in suitable configurations +- `just clippy` - Run clippy on all crates +- `cargo test` - Run unit tests only + +### Build Variants +- `cargo build --release` - Default build (big but pretty, ~2.5min) +- `cargo build --release --no-default-features --features lean` - Lean build (~1.5min) +- `cargo build --release --no-default-features --features small` - Minimal deps (~46s) + +### Test Best Practices +- Run tests before making changes to understand existing issues +- Use `GIX_TEST_IGNORE_ARCHIVES=1` when testing on macOS/Windows +- Journey tests validate CLI behavior end-to-end + +## Architecture Decisions + +### Plumbing vs Porcelain +- **Plumbing crates**: Low-level, take references, expose mutable parts as arguments +- **Porcelain (gix)**: High-level, convenient, may clone Repository for user convenience +- Platforms: cheap to create, keep reference to Repository +- Caches: more expensive, clone `Repository` or free of lifetimes + +### Options vs Context +- Use `Options` for branching behavior configuration (can be defaulted) +- Use `Context` for data required for operation (cannot be defaulted) + +## Crate Organization + +### Common Crates +- `gix`: Main library entrypoint (porcelain) +- `gix-object`, `gix-ref`, `gix-config`: Core git data structures +- `gix-odb`, `gix-pack`: Object database and pack handling +- `gix-diff`, `gix-merge`, `gix-status`: Operations +- `gitoxide-core`: Shared CLI functionality + +## Documentation +- High-level docs: README.md, CONTRIBUTING.md, DEVELOPMENT.md +- Crate status: crate-status.md +- Stability guide: STABILITY.md +- Always update docs if directly related to code changes + +## CI and Releases +- Ubuntu-latest git version is the compatibility target +- `cargo smart-release` for releases (driven by commit messages) +- Split breaking changes into separate commits per affected crate if one commit-message wouldn't be suitable for all changed crates. +- First commit: breaking change only; second commit: adaptations + +## When Suggesting Changes +1. Understand the plumbing vs porcelain distinction +2. Check existing patterns in similar crates +3. Follow error handling conventions strictly +4. Ensure changes work with feature flags (small, lean, max, max-pure) +5. Consider impact on both library and CLI users +6. Test against real git repositories when possible diff --git a/.github/workflows/cifuzz.yml b/.github/workflows/cifuzz.yml index 6f6c70b39a4..0526523e1f3 100644 --- a/.github/workflows/cifuzz.yml +++ b/.github/workflows/cifuzz.yml @@ -39,7 +39,7 @@ jobs: fuzz-seconds: 600 - name: Upload Crash - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v5 if: failure() && steps.build.outcome == 'success' with: name: artifacts diff --git a/examples/log.rs b/examples/log.rs index 0ca592cb57d..293d358529e 100644 --- a/examples/log.rs +++ b/examples/log.rs @@ -150,7 +150,7 @@ fn run(args: Args) -> anyhow::Result<()> { commit_ref.author.actor().write_to(&mut buf)?; buf.into() }, - time: commit_ref.author.time()?.format(format::DEFAULT), + time: commit_ref.author.time()?.format_or_unix(format::DEFAULT), message: commit_ref.message.to_owned(), }) }), diff --git a/gitoxide-core/src/net.rs b/gitoxide-core/src/net.rs index e5c62d52927..533b8ec8758 100644 --- a/gitoxide-core/src/net.rs +++ b/gitoxide-core/src/net.rs @@ -1,5 +1,10 @@ use std::str::FromStr; +#[cfg(feature = "async-client")] +use gix::protocol::transport::client::async_io as io_mode; +#[cfg(feature = "blocking-client")] +use gix::protocol::transport::client::blocking_io as io_mode; + #[derive(Default, Clone, Eq, PartialEq, Debug)] pub enum Protocol { V1, @@ -39,17 +44,14 @@ mod impls { #[gix::protocol::maybe_async::maybe_async] pub async fn connect( url: Url, - options: gix::protocol::transport::client::connect::Options, -) -> Result< - gix::protocol::SendFlushOnDrop>, - gix::protocol::transport::client::connect::Error, -> + options: io_mode::connect::Options, +) -> Result>, io_mode::connect::Error> where Url: TryInto, gix::url::parse::Error: From, { Ok(gix::protocol::SendFlushOnDrop::new( - gix::protocol::transport::connect(url, options).await?, + io_mode::connect::connect(url, options).await?, false, )) } diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index 66b64d95f24..0be29d7afd0 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -4,6 +4,11 @@ use std::{ sync::{atomic::AtomicBool, Arc}, }; +use crate::{net, pack::receive::protocol::fetch::negotiate, OutputFormat}; +#[cfg(feature = "async-client")] +use gix::protocol::transport::client::async_io::connect; +#[cfg(feature = "blocking-client")] +use gix::protocol::transport::client::blocking_io::connect; use gix::{config::tree::Key, protocol::maybe_async, remote::fetch::Error, DynNestedProgress}; pub use gix::{ hash::ObjectId, @@ -19,8 +24,6 @@ pub use gix::{ NestedProgress, Progress, }; -use crate::{net, pack::receive::protocol::fetch::negotiate, OutputFormat}; - pub const PROGRESS_RANGE: std::ops::RangeInclusive = 1..=3; pub struct Context { pub thread_limit: Option, @@ -47,7 +50,7 @@ where { let mut transport = net::connect( url, - gix::protocol::transport::client::connect::Options { + connect::Options { version: protocol.unwrap_or_default().into(), ..Default::default() }, @@ -61,8 +64,9 @@ where .is_some(); let agent = gix::protocol::agent(gix::env::agent()); - let mut handshake = gix::protocol::fetch::handshake( + let mut handshake = gix::protocol::handshake( &mut transport.inner, + transport::Service::UploadPack, gix::protocol::credentials::builtin, vec![("agent".into(), Some(agent.clone()))], &mut progress, @@ -78,18 +82,21 @@ where }) .collect::>()?; let user_agent = ("agent", Some(agent.clone().into())); - let refmap = gix::protocol::fetch::RefMap::new( - &mut progress, - &fetch_refspecs, - gix::protocol::fetch::Context { - handshake: &mut handshake, - transport: &mut transport.inner, - user_agent: user_agent.clone(), + + let context = gix::protocol::fetch::refmap::init::Context { + fetch_refspecs: fetch_refspecs.clone(), + extra_refspecs: vec![], + }; + let refmap = handshake + .fetch_or_extract_refmap( + &mut progress, + &mut transport.inner, + user_agent.clone(), trace_packetlines, - }, - gix::protocol::fetch::refmap::init::Options::default(), - ) - .await?; + true, + context, + ) + .await?; if refmap.mappings.is_empty() && !refmap.remote_refs.is_empty() { return Err(Error::NoMapping { diff --git a/gitoxide-core/src/query/engine/command.rs b/gitoxide-core/src/query/engine/command.rs index 1b020030821..05bdc48a024 100644 --- a/gitoxide-core/src/query/engine/command.rs +++ b/gitoxide-core/src/query/engine/command.rs @@ -180,7 +180,7 @@ mod trace_path { out, "{}| {} | {} {} {} ➡ {}", self.diff.unwrap_or_default().format(max_diff_lines), - self.commit_time.format(gix::date::time::format::SHORT), + self.commit_time.format_or_unix(gix::date::time::format::SHORT), id.shorten_or_id(), self.mode.as_str(), path_by_id[&source_id], @@ -192,7 +192,7 @@ mod trace_path { out, "{}| {} | {} {} {}", self.diff.unwrap_or_default().format(max_diff_lines), - self.commit_time.format(gix::date::time::format::SHORT), + self.commit_time.format_or_unix(gix::date::time::format::SHORT), id.shorten_or_id(), self.mode.as_str(), path_by_id[&self.file_id] diff --git a/gitoxide-core/src/repository/credential.rs b/gitoxide-core/src/repository/credential.rs index 9684250bd4d..451e453b7c9 100644 --- a/gitoxide-core/src/repository/credential.rs +++ b/gitoxide-core/src/repository/credential.rs @@ -6,9 +6,11 @@ enum Error { Configuration(#[from] gix::config::credential_helpers::Error), #[error(transparent)] Protocol(#[from] gix::credentials::protocol::Error), + #[error(transparent)] + ConfigLoad(#[from] gix::config::file::init::from_paths::Error), } -pub fn function(repo: gix::Repository, action: gix::credentials::program::main::Action) -> anyhow::Result<()> { +pub fn function(repo: Option, action: gix::credentials::program::main::Action) -> anyhow::Result<()> { use gix::credentials::program::main::Action::*; gix::credentials::program::main( Some(action.as_str().into()), @@ -20,9 +22,24 @@ pub fn function(repo: gix::Repository, action: gix::credentials::program::main:: .clone() .or_else(|| context.to_url()) .ok_or(Error::Protocol(gix::credentials::protocol::Error::UrlMissing))?; - let (mut cascade, _action, prompt_options) = repo - .config_snapshot() - .credential_helpers(gix::url::parse(url.as_ref())?)?; + + let (mut cascade, _action, prompt_options) = match repo { + Some(ref repo) => repo + .config_snapshot() + .credential_helpers(gix::url::parse(url.as_ref())?)?, + None => { + let config = gix::config::File::from_globals()?; + let environment = gix::open::permissions::Environment::all(); + gix::config::credential_helpers( + gix::url::parse(url.as_ref())?, + &config, + false, /* lenient config */ + |_| true, /* section filter */ + environment, + false, /* use http path (override, uses configuration now)*/ + )? + } + }; cascade .invoke( match action { diff --git a/gitoxide-core/src/repository/fetch.rs b/gitoxide-core/src/repository/fetch.rs index 21198349bb3..2f143a9ec8e 100644 --- a/gitoxide-core/src/repository/fetch.rs +++ b/gitoxide-core/src/repository/fetch.rs @@ -320,7 +320,7 @@ pub(crate) mod function { err, "server sent {} tips, {} were filtered due to {} refspec(s).", map.remote_refs.len(), - map.remote_refs.len() - map.mappings.len(), + map.remote_refs.len().saturating_sub(map.mappings.len()), refspecs.len() )?; } diff --git a/gitoxide-core/src/repository/revision/explain.rs b/gitoxide-core/src/repository/revision/explain.rs index 07bee89cb89..45e67427b48 100644 --- a/gitoxide-core/src/repository/revision/explain.rs +++ b/gitoxide-core/src/repository/revision/explain.rs @@ -88,7 +88,7 @@ impl delegate::Revision for Explain<'_> { ReflogLookup::Date(time) => writeln!( self.out, "Find entry closest to time {} in reflog of '{}' reference", - time.format(gix::date::time::format::ISO8601), + time.format_or_unix(gix::date::time::format::ISO8601), ref_name ) .ok(), diff --git a/gix-blame/src/error.rs b/gix-blame/src/error.rs index d90a05127cc..f78c02f85a4 100644 --- a/gix-blame/src/error.rs +++ b/gix-blame/src/error.rs @@ -30,7 +30,7 @@ pub enum Error { #[error(transparent)] DiffTreeWithRewrites(#[from] gix_diff::tree_with_rewrites::Error), #[error("Invalid line range was given, line range is expected to be a 1-based inclusive range in the format ','")] - InvalidLineRange, + InvalidOneBasedLineRange, #[error("Failure to decode commit during traversal")] DecodeCommit(#[from] gix_object::decode::Error), #[error("Failed to get parent from commitgraph during traversal")] diff --git a/gix-blame/src/file/function.rs b/gix-blame/src/file/function.rs index 8d914d3b7ee..773ac1425cb 100644 --- a/gix-blame/src/file/function.rs +++ b/gix-blame/src/file/function.rs @@ -24,14 +24,12 @@ use crate::{types::BlamePathEntry, BlameEntry, Error, Options, Outcome, Statisti /// - The first commit to be responsible for parts of `file_path`. /// * `cache` /// - Optionally, the commitgraph cache. -/// * `file_path` -/// - A *slash-separated* worktree-relative path to the file to blame. -/// * `range` -/// - A 1-based inclusive range, in order to mirror `git`’s behaviour. `Some(20..40)` represents -/// 21 lines, spanning from line 20 up to and including line 40. This will be converted to -/// `19..40` internally as the algorithm uses 0-based ranges that are exclusive at the end. /// * `resource_cache` /// - Used for diffing trees. +/// * `file_path` +/// - A *slash-separated* worktree-relative path to the file to blame. +/// * `options` +/// - An instance of [`Options`]. /// /// ## The algorithm /// @@ -95,16 +93,11 @@ pub fn file( return Ok(Outcome::default()); } - let ranges = options.range.to_zero_based_exclusive(num_lines_in_blamed)?; - let mut hunks_to_blame = Vec::with_capacity(ranges.len()); - - for range in ranges { - hunks_to_blame.push(UnblamedHunk { - range_in_blamed_file: range.clone(), - suspects: [(suspect, range)].into(), - source_file_name: None, - }); - } + let ranges_to_blame = options.ranges.to_zero_based_exclusive_ranges(num_lines_in_blamed); + let mut hunks_to_blame = ranges_to_blame + .into_iter() + .map(|range| UnblamedHunk::new(range, suspect)) + .collect::>(); let (mut buf, mut buf2) = (Vec::new(), Vec::new()); let commit = find_commit(cache.as_ref(), &odb, &suspect, &mut buf)?; diff --git a/gix-blame/src/file/tests.rs b/gix-blame/src/file/tests.rs index 6eabafbb4c4..2e605242059 100644 --- a/gix-blame/src/file/tests.rs +++ b/gix-blame/src/file/tests.rs @@ -985,3 +985,129 @@ mod process_changes { ); } } + +mod blame_ranges { + use crate::{BlameRanges, Error}; + + #[test] + fn create_with_invalid_range() { + let ranges = BlameRanges::from_one_based_inclusive_range(0..=10); + + assert!(matches!(ranges, Err(Error::InvalidOneBasedLineRange))); + } + + #[test] + fn create_from_single_range() { + let ranges = BlameRanges::from_one_based_inclusive_range(20..=40).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![19..40]); + } + + #[test] + fn create_from_multiple_ranges() { + let ranges = BlameRanges::from_one_based_inclusive_ranges(vec![1..=4, 10..=14]).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![0..4, 9..14]); + } + + #[test] + fn create_with_empty_ranges() { + let ranges = BlameRanges::from_one_based_inclusive_ranges(vec![]).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![0..100]); + } + + #[test] + fn add_range_merges_overlapping() { + let mut ranges = BlameRanges::from_one_based_inclusive_range(1..=5).unwrap(); + ranges.add_one_based_inclusive_range(3..=7).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![0..7]); + } + + #[test] + fn add_range_merges_overlapping_both() { + let mut ranges = BlameRanges::from_one_based_inclusive_range(1..=3).unwrap(); + ranges.add_one_based_inclusive_range(5..=7).unwrap(); + ranges.add_one_based_inclusive_range(2..=6).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![0..7]); + } + + #[test] + fn add_range_non_sorted() { + let mut ranges = BlameRanges::from_one_based_inclusive_range(5..=7).unwrap(); + ranges.add_one_based_inclusive_range(1..=3).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![0..3, 4..7]); + } + + #[test] + fn add_range_merges_adjacent() { + let mut ranges = BlameRanges::from_one_based_inclusive_range(1..=5).unwrap(); + ranges.add_one_based_inclusive_range(6..=10).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![0..10]); + } + + #[test] + fn non_sorted_ranges() { + let ranges = BlameRanges::from_one_based_inclusive_ranges(vec![10..=15, 1..=5]).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![0..5, 9..15]); + } + + #[test] + fn convert_to_zero_based_exclusive() { + let ranges = BlameRanges::from_one_based_inclusive_ranges(vec![1..=5, 10..=15]).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![0..5, 9..15]); + } + + #[test] + fn convert_full_file_to_zero_based() { + let ranges = BlameRanges::WholeFile; + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![0..100]); + } + + #[test] + fn adding_a_range_turns_whole_file_into_partial_file() { + let mut ranges = BlameRanges::default(); + + ranges.add_one_based_inclusive_range(1..=10).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(100), vec![0..10]); + } + + #[test] + fn to_zero_based_exclusive_ignores_range_past_max_lines() { + let mut ranges = BlameRanges::from_one_based_inclusive_range(1..=5).unwrap(); + ranges.add_one_based_inclusive_range(16..=20).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(7), vec![0..5]); + } + + #[test] + fn to_zero_based_exclusive_range_doesnt_exceed_max_lines() { + let mut ranges = BlameRanges::from_one_based_inclusive_range(1..=5).unwrap(); + ranges.add_one_based_inclusive_range(6..=10).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(7), vec![0..7]); + } + + #[test] + fn to_zero_based_exclusive_merged_ranges_dont_exceed_max_lines() { + let mut ranges = BlameRanges::from_one_based_inclusive_range(1..=4).unwrap(); + ranges.add_one_based_inclusive_range(6..=10).unwrap(); + + assert_eq!(ranges.to_zero_based_exclusive_ranges(7), vec![0..4, 5..7]); + } + + #[test] + fn default_is_full_file() { + let ranges = BlameRanges::default(); + + assert!(matches!(ranges, BlameRanges::WholeFile)); + } +} diff --git a/gix-blame/src/types.rs b/gix-blame/src/types.rs index bbd54591eb9..79c49b7a62c 100644 --- a/gix-blame/src/types.rs +++ b/gix-blame/src/types.rs @@ -21,12 +21,14 @@ use crate::Error; /// use gix_blame::BlameRanges; /// /// // Blame lines 20 through 40 (inclusive) -/// let range = BlameRanges::from_range(20..=40); +/// let range = BlameRanges::from_one_based_inclusive_range(20..=40); /// /// // Blame multiple ranges -/// let mut ranges = BlameRanges::new(); -/// ranges.add_range(1..=4); // Lines 1-4 -/// ranges.add_range(10..=14); // Lines 10-14 +/// let mut ranges = BlameRanges::from_one_based_inclusive_ranges(vec![ +/// 1..=4, // Lines 1-4 +/// 10..=14, // Lines 10-14 +/// ] +/// ); /// ``` /// /// # Line Number Representation @@ -36,105 +38,115 @@ use crate::Error; /// - This will be converted to `19..40` internally as the algorithm uses 0-based ranges that are exclusive at the end /// /// # Empty Ranges -/// -/// An empty `BlameRanges` (created via `BlameRanges::new()` or `BlameRanges::default()`) means -/// to blame the entire file, similar to running `git blame` without line number arguments. +/// You can blame the entire file by calling `BlameRanges::default()`, or by passing an empty vector to `from_one_based_inclusive_ranges`. #[derive(Debug, Clone, Default)] -pub struct BlameRanges { - /// The ranges to blame, stored as 1-based inclusive ranges - /// An empty Vec means blame the entire file - ranges: Vec>, +pub enum BlameRanges { + /// Blame the entire file. + #[default] + WholeFile, + /// Blame ranges in 0-based exclusive format. + PartialFile(Vec>), } /// Lifecycle impl BlameRanges { - /// Create a new empty BlameRanges instance. + /// Create from a single 0-based range. /// - /// An empty instance means to blame the entire file. - pub fn new() -> Self { - Self::default() + /// Note that the input range is 1-based inclusive, as used by git, and + /// the output is a zero-based `BlameRanges` instance. + pub fn from_one_based_inclusive_range(range: RangeInclusive) -> Result { + let zero_based_range = Self::inclusive_to_zero_based_exclusive(range)?; + Ok(Self::PartialFile(vec![zero_based_range])) } - /// Create from a single range. + /// Create from multiple 0-based ranges. + /// + /// Note that the input ranges are 1-based inclusive, as used by git, and + /// the output is a zero-based `BlameRanges` instance. /// - /// The range is 1-based, similar to git's line number format. - pub fn from_range(range: RangeInclusive) -> Self { - Self { ranges: vec![range] } + /// If the input vector is empty, the result will be `WholeFile`. + pub fn from_one_based_inclusive_ranges(ranges: Vec>) -> Result { + if ranges.is_empty() { + return Ok(Self::WholeFile); + } + + let zero_based_ranges = ranges + .into_iter() + .map(Self::inclusive_to_zero_based_exclusive) + .collect::>(); + let mut result = Self::PartialFile(vec![]); + for range in zero_based_ranges { + result.merge_zero_based_exclusive_range(range?); + } + Ok(result) } - /// Create from multiple ranges. - /// - /// All ranges are 1-based. - /// Overlapping or adjacent ranges will be merged. - pub fn from_ranges(ranges: Vec>) -> Self { - let mut result = Self::new(); - for range in ranges { - result.merge_range(range); + /// Convert a 1-based inclusive range to a 0-based exclusive range. + fn inclusive_to_zero_based_exclusive(range: RangeInclusive) -> Result, Error> { + if range.start() == &0 { + return Err(Error::InvalidOneBasedLineRange); } - result + let start = range.start() - 1; + let end = *range.end(); + Ok(start..end) } } impl BlameRanges { /// Add a single range to blame. /// - /// The range should be 1-based inclusive. - /// If the new range overlaps with or is adjacent to an existing range, - /// they will be merged into a single range. - pub fn add_range(&mut self, new_range: RangeInclusive) { - self.merge_range(new_range); - } + /// The new range will be merged with any overlapping existing ranges. + pub fn add_one_based_inclusive_range(&mut self, new_range: RangeInclusive) -> Result<(), Error> { + let zero_based_range = Self::inclusive_to_zero_based_exclusive(new_range)?; + self.merge_zero_based_exclusive_range(zero_based_range); - /// Attempts to merge the new range with any existing ranges. - /// If no merge is possible, add it as a new range. - fn merge_range(&mut self, new_range: RangeInclusive) { - // Check if this range can be merged with any existing range - for range in &mut self.ranges { - // Check if ranges overlap or are adjacent - if new_range.start() <= range.end() && range.start() <= new_range.end() { - *range = *range.start().min(new_range.start())..=*range.end().max(new_range.end()); - return; - } - } - // If no overlap found, add it as a new range - self.ranges.push(new_range); + Ok(()) } - /// Convert the 1-based inclusive ranges to 0-based exclusive ranges. - /// - /// This is used internally by the blame algorithm to convert from git's line number format - /// to the internal format used for processing. - /// - /// # Errors - /// - /// Returns `Error::InvalidLineRange` if: - /// - Any range starts at 0 (must be 1-based) - /// - Any range extends beyond the file's length - /// - Any range has the same start and end - pub fn to_zero_based_exclusive(&self, max_lines: u32) -> Result>, Error> { - if self.ranges.is_empty() { - let range = 0..max_lines; - return Ok(vec![range]); - } + /// Adds a new ranges, merging it with any existing overlapping ranges. + fn merge_zero_based_exclusive_range(&mut self, new_range: Range) { + match self { + Self::PartialFile(ref mut ranges) => { + // Partition ranges into those that don't overlap and those that do. + let (mut non_overlapping, overlapping): (Vec<_>, Vec<_>) = ranges + .drain(..) + .partition(|range| new_range.end < range.start || range.end < new_range.start); - let mut result = Vec::with_capacity(self.ranges.len()); - for range in &self.ranges { - if *range.start() == 0 { - return Err(Error::InvalidLineRange); - } - let start = range.start() - 1; - let end = *range.end(); - if start >= max_lines || end > max_lines || start == end { - return Err(Error::InvalidLineRange); + let merged_range = overlapping.into_iter().fold(new_range, |acc, range| { + acc.start.min(range.start)..acc.end.max(range.end) + }); + + non_overlapping.push(merged_range); + + *ranges = non_overlapping; + ranges.sort_by(|a, b| a.start.cmp(&b.start)); } - result.push(start..end); + Self::WholeFile => *self = Self::PartialFile(vec![new_range]), } - Ok(result) } - /// Returns true if no specific ranges are set (meaning blame entire file) - pub fn is_empty(&self) -> bool { - self.ranges.is_empty() + /// Gets zero-based exclusive ranges. + pub fn to_zero_based_exclusive_ranges(&self, max_lines: u32) -> Vec> { + match self { + Self::WholeFile => { + let full_range = 0..max_lines; + vec![full_range] + } + Self::PartialFile(ranges) => ranges + .iter() + .filter_map(|range| { + if range.end < max_lines { + return Some(range.clone()); + } + + if range.start < max_lines { + Some(range.start..max_lines) + } else { + None + } + }) + .collect(), + } } } @@ -144,7 +156,7 @@ pub struct Options { /// The algorithm to use for diffing. pub diff_algorithm: gix_diff::blob::Algorithm, /// The ranges to blame in the file. - pub range: BlameRanges, + pub ranges: BlameRanges, /// Don't consider commits before the given date. pub since: Option, /// Determine if rename tracking should be performed, and how. @@ -380,6 +392,17 @@ pub struct UnblamedHunk { } impl UnblamedHunk { + pub(crate) fn new(from_range_in_blamed_file: Range, suspect: ObjectId) -> Self { + let range_start = from_range_in_blamed_file.start; + let range_end = from_range_in_blamed_file.end; + + UnblamedHunk { + range_in_blamed_file: range_start..range_end, + suspects: [(suspect, range_start..range_end)].into(), + source_file_name: None, + } + } + pub(crate) fn has_suspect(&self, suspect: &ObjectId) -> bool { self.suspects.iter().any(|entry| entry.0 == *suspect) } diff --git a/gix-blame/tests/blame.rs b/gix-blame/tests/blame.rs index 6b1f33edfb6..bd9e1f50c35 100644 --- a/gix-blame/tests/blame.rs +++ b/gix-blame/tests/blame.rs @@ -229,7 +229,7 @@ macro_rules! mktest { source_file_name.as_ref(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: BlameRanges::default(), + ranges: BlameRanges::default(), since: None, rewrites: Some(gix_diff::Rewrites::default()), debug_track_path: false, @@ -315,7 +315,7 @@ fn diff_disparity() { source_file_name.as_ref(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: BlameRanges::default(), + ranges: BlameRanges::default(), since: None, rewrites: Some(gix_diff::Rewrites::default()), debug_track_path: false, @@ -382,7 +382,7 @@ fn since() -> gix_testtools::Result { source_file_name.as_ref(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: BlameRanges::default(), + ranges: BlameRanges::default(), since: Some(gix_date::parse("2025-01-31", None)?), rewrites: Some(gix_diff::Rewrites::default()), debug_track_path: false, @@ -422,7 +422,7 @@ mod blame_ranges { source_file_name.as_ref(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: BlameRanges::from_range(1..=2), + ranges: BlameRanges::from_one_based_inclusive_range(1..=2).unwrap(), since: None, rewrites: Some(gix_diff::Rewrites::default()), debug_track_path: false, @@ -448,10 +448,12 @@ mod blame_ranges { suspect, } = Fixture::new()?; - let mut ranges = BlameRanges::new(); - ranges.add_range(1..=2); // Lines 1-2 - ranges.add_range(1..=1); // Duplicate range, should be ignored - ranges.add_range(4..=4); // Line 4 + let ranges = BlameRanges::from_one_based_inclusive_ranges(vec![ + 1..=2, // Lines 1-2 + 1..=1, // Duplicate range, should be ignored + 4..=4, // Line 4 + ]) + .unwrap(); let source_file_name: gix_object::bstr::BString = "simple.txt".into(); @@ -463,7 +465,7 @@ mod blame_ranges { source_file_name.as_ref(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: ranges, + ranges, since: None, rewrites: None, debug_track_path: false, @@ -492,7 +494,7 @@ mod blame_ranges { suspect, } = Fixture::new()?; - let ranges = BlameRanges::from_ranges(vec![1..=2, 1..=1, 4..=4]); + let ranges = BlameRanges::from_one_based_inclusive_ranges(vec![1..=2, 1..=1, 4..=4]).unwrap(); let source_file_name: gix_object::bstr::BString = "simple.txt".into(); @@ -504,7 +506,7 @@ mod blame_ranges { source_file_name.as_ref(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: ranges, + ranges, since: None, rewrites: None, debug_track_path: false, @@ -550,7 +552,7 @@ mod rename_tracking { source_file_name.into(), gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, - range: BlameRanges::default(), + ranges: BlameRanges::default(), since: None, rewrites: Some(gix_diff::Rewrites::default()), debug_track_path: false, diff --git a/gix-date/src/time/format.rs b/gix-date/src/time/format.rs index eeb4bf706b7..b318f129366 100644 --- a/gix-date/src/time/format.rs +++ b/gix-date/src/time/format.rs @@ -36,24 +36,32 @@ impl Time { /// /// Use [`Format::Unix`], [`Format::Raw`] or one of the custom formats /// defined in the [`format`](mod@crate::time::format) submodule. - pub fn format(&self, format: impl Into) -> String { + /// + /// Note that this can fail if the timezone isn't valid and the format requires a conversion to [`jiff::Zoned`]. + pub fn format(&self, format: impl Into) -> Result { + self.format_inner(format.into()) + } + + /// Like [`Self::format()`], but on time conversion error, produce the [UNIX] format instead + /// to make it infallible. + pub fn format_or_unix(&self, format: impl Into) -> String { self.format_inner(format.into()) + .unwrap_or_else(|_| self.seconds.to_string()) } - fn format_inner(&self, format: Format) -> String { - match format { - Format::Custom(CustomFormat(format)) => self.to_time().strftime(format).to_string(), + fn format_inner(&self, format: Format) -> Result { + Ok(match format { + Format::Custom(CustomFormat(format)) => self.to_zoned()?.strftime(format).to_string(), Format::Unix => self.seconds.to_string(), Format::Raw => self.to_string(), - } + }) } } impl Time { - fn to_time(self) -> jiff::Zoned { - let offset = jiff::tz::Offset::from_seconds(self.offset).expect("valid offset"); - jiff::Timestamp::from_second(self.seconds) - .expect("always valid unix time") - .to_zoned(offset.to_time_zone()) + /// Produce a `Zoned` time for complex time computations and limitless formatting. + pub fn to_zoned(self) -> Result { + let offset = jiff::tz::Offset::from_seconds(self.offset)?; + Ok(jiff::Timestamp::from_second(self.seconds)?.to_zoned(offset.to_time_zone())) } } diff --git a/gix-date/tests/time/baseline.rs b/gix-date/tests/time/baseline.rs index 4eff7cef066..67b9e95f8cd 100644 --- a/gix-date/tests/time/baseline.rs +++ b/gix-date/tests/time/baseline.rs @@ -66,15 +66,17 @@ fn parse_compare_format() { "{pattern:?} disagrees with baseline seconds since epoch: {actual:?}" ); if let Some(format_name) = format_name { - let reformatted = t.format(match format_name.as_str() { - "RFC2822" => Format::Custom(format::RFC2822), - "ISO8601" => Format::Custom(format::ISO8601), - "ISO8601_STRICT" => Format::Custom(format::ISO8601_STRICT), - "GITOXIDE" => Format::Custom(format::GITOXIDE), - "UNIX" => Format::Unix, - "RAW" => Format::Raw, - unknown => unreachable!("All formats should be well-known and implemented: {unknown:?}"), - }); + let reformatted = t + .format(match format_name.as_str() { + "RFC2822" => Format::Custom(format::RFC2822), + "ISO8601" => Format::Custom(format::ISO8601), + "ISO8601_STRICT" => Format::Custom(format::ISO8601_STRICT), + "GITOXIDE" => Format::Custom(format::GITOXIDE), + "UNIX" => Format::Unix, + "RAW" => Format::Raw, + unknown => unreachable!("All formats should be well-known and implemented: {unknown:?}"), + }) + .expect("valid input time"); assert_eq!( reformatted, *pattern, "{reformatted:?} disagrees with baseline pattern: {pattern:?}" diff --git a/gix-date/tests/time/format.rs b/gix-date/tests/time/format.rs index 06e5b8b774c..2713eb78757 100644 --- a/gix-date/tests/time/format.rs +++ b/gix-date/tests/time/format.rs @@ -4,19 +4,21 @@ use gix_date::{ }; #[test] -fn short() { - assert_eq!(time().format(format::SHORT), "1973-11-30"); +fn short() -> gix_testtools::Result { + assert_eq!(time().format(format::SHORT)?, "1973-11-30"); + Ok(()) } #[test] -fn unix() { +fn unix() -> gix_testtools::Result { let expected = "123456789"; - assert_eq!(time().format(Format::Unix), expected); - assert_eq!(time().format(format::UNIX), expected); + assert_eq!(time().format(Format::Unix)?, expected); + assert_eq!(time().format(format::UNIX)?, expected); + Ok(()) } #[test] -fn raw() { +fn raw() -> gix_testtools::Result { for (time, expected) in [ (time(), "123456789 +0230"), ( @@ -27,58 +29,71 @@ fn raw() { "1112911993 +0100", ), ] { - assert_eq!(time.format(Format::Raw), expected); - assert_eq!(time.format(format::RAW), expected); + assert_eq!(time.format(Format::Raw)?, expected); + assert_eq!(time.format(format::RAW)?, expected); } + Ok(()) } #[test] -fn iso8601() { - assert_eq!(time().format(format::ISO8601), "1973-11-30 00:03:09 +0230"); +fn iso8601() -> gix_testtools::Result { + assert_eq!(time().format(format::ISO8601)?, "1973-11-30 00:03:09 +0230"); + Ok(()) } #[test] -fn iso8601_strict() { - assert_eq!(time().format(format::ISO8601_STRICT), "1973-11-30T00:03:09+02:30"); +fn iso8601_strict() -> gix_testtools::Result { + assert_eq!(time().format(format::ISO8601_STRICT)?, "1973-11-30T00:03:09+02:30"); + Ok(()) } #[test] -fn rfc2822() { - assert_eq!(time().format(format::RFC2822), "Fri, 30 Nov 1973 00:03:09 +0230"); - assert_eq!(time_dec1().format(format::RFC2822), "Sat, 01 Dec 1973 00:03:09 +0230"); +fn rfc2822() -> gix_testtools::Result { + assert_eq!(time().format(format::RFC2822)?, "Fri, 30 Nov 1973 00:03:09 +0230"); + assert_eq!(time_dec1().format(format::RFC2822)?, "Sat, 01 Dec 1973 00:03:09 +0230"); + Ok(()) } #[test] -fn git_rfc2822() { - assert_eq!(time().format(format::GIT_RFC2822), "Fri, 30 Nov 1973 00:03:09 +0230"); +fn git_rfc2822() -> gix_testtools::Result { + assert_eq!(time().format(format::GIT_RFC2822)?, "Fri, 30 Nov 1973 00:03:09 +0230"); assert_eq!( - time_dec1().format(format::GIT_RFC2822), + time_dec1().format(format::GIT_RFC2822)?, "Sat, 1 Dec 1973 00:03:09 +0230" ); + Ok(()) } #[test] -fn default() { - assert_eq!( - time().format(gix_date::time::format::GITOXIDE), - "Fri Nov 30 1973 00:03:09 +0230" - ); +fn default() -> gix_testtools::Result { + assert_eq!(time().format(format::GITOXIDE)?, "Fri Nov 30 1973 00:03:09 +0230"); + assert_eq!(time_dec1().format(format::GITOXIDE)?, "Sat Dec 01 1973 00:03:09 +0230"); + Ok(()) +} + +#[test] +fn format_or_unix() { assert_eq!( - time_dec1().format(gix_date::time::format::GITOXIDE), - "Sat Dec 01 1973 00:03:09 +0230" + Time { + seconds: 42, + offset: 7200 * 60 + } + .format_or_unix(format::GITOXIDE), + "42" ); } #[test] -fn git_default() { +fn git_default() -> gix_testtools::Result { assert_eq!( - time().format(gix_date::time::format::DEFAULT), + time().format(gix_date::time::format::DEFAULT)?, "Fri Nov 30 00:03:09 1973 +0230" ); assert_eq!( - time_dec1().format(gix_date::time::format::DEFAULT), + time_dec1().format(gix_date::time::format::DEFAULT)?, "Sat Dec 1 00:03:09 1973 +0230" ); + Ok(()) } fn time() -> Time { diff --git a/gix-diff/src/blob/unified_diff/impls.rs b/gix-diff/src/blob/unified_diff/impls.rs index 959560fba7f..16512f3ebfa 100644 --- a/gix-diff/src/blob/unified_diff/impls.rs +++ b/gix-diff/src/blob/unified_diff/impls.rs @@ -260,7 +260,8 @@ where } impl DiffLineKind { - const fn to_prefix(self) -> char { + /// Returns a one-character representation for use in unified diffs. + pub const fn to_prefix(self) -> char { match self { DiffLineKind::Context => ' ', DiffLineKind::Add => '+', diff --git a/gix-diff/tests/README.md b/gix-diff/tests/README.md new file mode 100644 index 00000000000..59e1101a413 --- /dev/null +++ b/gix-diff/tests/README.md @@ -0,0 +1,26 @@ +## How to run diff-slider tests + +The idea is to use https://github.com/mhagger/diff-slider-tools to create slider information for use to generate a test which invokes our own implementation and compares it to Git itself. +Follow these instructions to set it up. + +1. DIFF_SLIDER_TOOLS=/your/anticipated/path/to/diff-slider-tools +2. `git clone https://github.com/mhagger/diff-slider-tools $DIFF_SLIDER_TOOLS` +3. `pushd $DIFF_SLIDER_TOOLS` +4. Follow [these instructions](https://github.com/mhagger/diff-slider-tools/blob/b59ed13d7a2a6cfe14a8f79d434b6221cc8b04dd/README.md?plain=1#L122-L146) to generate a file containing the slider information. Be sure to set the `repo` variable as it's used in later script invocations. + - Note that `get-corpus` must be run with `./get-corpus`. + - You can use an existing repository, for instance via `repo=git-human`, so there is no need to find your own repository to test. + - The script suite is very slow, and it's recommended to only set a range of commits, or use a small repository for testing. + +Finally, run the `internal-tools` program to turn that file into a fixture called `make_diff_for_sliders_repo.sh`. + +```shell +# run inside `gitoxide` +popd +cargo run --package internal-tools -- \ + create-diff-cases \ + --sliders-file $DIFF_SLIDER_TOOLS/corpus/$repo.sliders \ + --worktree-dir $DIFF_SLIDER_TOOLS/corpus/$repo.git/ \ + --destination-dir gix-diff/tests/fixtures/ +``` + +Finally, run `cargo test -p gix-diff-tests sliders -- --nocapture` to execute the actual tests to compare. diff --git a/gix-diff/tests/diff/blob/mod.rs b/gix-diff/tests/diff/blob/mod.rs index 1959c4e6fdb..5742dfd4350 100644 --- a/gix-diff/tests/diff/blob/mod.rs +++ b/gix-diff/tests/diff/blob/mod.rs @@ -1,3 +1,4 @@ pub(crate) mod pipeline; mod platform; +mod slider; mod unified_diff; diff --git a/gix-diff/tests/diff/blob/slider.rs b/gix-diff/tests/diff/blob/slider.rs new file mode 100644 index 00000000000..f152f00ffb2 --- /dev/null +++ b/gix-diff/tests/diff/blob/slider.rs @@ -0,0 +1,253 @@ +use gix_diff::blob::intern::TokenSource; +use gix_diff::blob::unified_diff::ContextSize; +use gix_diff::blob::{Algorithm, UnifiedDiff}; +use gix_testtools::bstr::{BString, ByteVec}; + +#[test] +fn baseline() -> gix_testtools::Result { + let worktree_path = gix_testtools::scripted_fixture_read_only_standalone("make_diff_for_sliders_repo.sh")?; + let asset_dir = worktree_path.join("assets"); + + let dir = std::fs::read_dir(&worktree_path)?; + + let mut count = 0; + for entry in dir { + let entry = entry?; + let file_name = entry.file_name().into_string().expect("to be string"); + + if !file_name.ends_with(".baseline") { + continue; + } + count += 1; + + let parts: Vec<_> = file_name.split('.').collect(); + let [name, algorithm, ..] = parts[..] else { + unreachable!() + }; + let algorithm = match algorithm { + "myers" => Algorithm::Myers, + "histogram" => Algorithm::Histogram, + _ => unreachable!(), + }; + + let parts: Vec<_> = name.split('-').collect(); + let [old_blob_id, new_blob_id] = parts[..] else { + unreachable!(); + }; + + let old_data = std::fs::read(asset_dir.join(format!("{old_blob_id}.blob")))?; + let new_data = std::fs::read(asset_dir.join(format!("{new_blob_id}.blob")))?; + + let interner = gix_diff::blob::intern::InternedInput::new( + tokens_for_diffing(old_data.as_slice()), + tokens_for_diffing(new_data.as_slice()), + ); + + let actual = gix_diff::blob::diff( + algorithm, + &interner, + UnifiedDiff::new( + &interner, + baseline::DiffHunkRecorder::new(), + ContextSize::symmetrical(3), + ), + )?; + + let baseline_path = worktree_path.join(file_name); + let baseline = std::fs::read(baseline_path)?; + let baseline = baseline::Baseline::new(&baseline); + + let actual = actual + .iter() + .fold(BString::default(), |mut acc, diff_hunk| { + acc.push_str(diff_hunk.header.to_string().as_str()); + acc.push(b'\n'); + + acc.extend_from_slice(&diff_hunk.lines); + + acc + }) + .to_string(); + + let baseline = baseline + .fold(BString::default(), |mut acc, diff_hunk| { + acc.push_str(diff_hunk.header.to_string().as_str()); + acc.push(b'\n'); + + acc.extend_from_slice(&diff_hunk.lines); + + acc + }) + .to_string(); + + pretty_assertions::assert_eq!(actual, baseline); + } + + if count == 0 { + eprintln!("Slider baseline isn't setup - look at ./gix-diff/tests/README.md for instructions"); + } + + Ok(()) +} + +fn tokens_for_diffing(data: &[u8]) -> impl TokenSource { + gix_diff::blob::sources::byte_lines(data) +} + +mod baseline { + use gix_object::bstr::ByteSlice; + use std::iter::Peekable; + + use gix_diff::blob::unified_diff::{ConsumeHunk, HunkHeader}; + use gix_object::bstr::{self, BString}; + + static START_OF_HEADER: &[u8; 4] = b"@@ -"; + + #[derive(Debug, PartialEq)] + pub struct DiffHunk { + pub header: HunkHeader, + pub lines: BString, + } + + pub struct DiffHunkRecorder { + inner: Vec, + } + + impl DiffHunkRecorder { + pub fn new() -> Self { + Self { inner: Vec::new() } + } + } + + impl ConsumeHunk for DiffHunkRecorder { + type Out = Vec; + + fn consume_hunk( + &mut self, + header: HunkHeader, + lines: &[(gix_diff::blob::unified_diff::DiffLineKind, &[u8])], + ) -> std::io::Result<()> { + let mut buf = Vec::new(); + + for &(kind, line) in lines { + buf.push(kind.to_prefix() as u8); + buf.extend_from_slice(line); + buf.push(b'\n'); + } + + let diff_hunk = DiffHunk { + header, + lines: buf.into(), + }; + + self.inner.push(diff_hunk); + + Ok(()) + } + + fn finish(self) -> Self::Out { + self.inner + } + } + + type Lines<'a> = Peekable>; + + pub struct Baseline<'a> { + lines: Lines<'a>, + } + + impl<'a> Baseline<'a> { + pub fn new(content: &'a [u8]) -> Baseline<'a> { + let mut lines = content.lines().peekable(); + skip_header(&mut lines); + Baseline { lines } + } + } + + impl Iterator for Baseline<'_> { + type Item = DiffHunk; + + fn next(&mut self) -> Option { + let mut hunk_header = None; + let mut hunk_lines = Vec::new(); + + while let Some(line) = self.lines.next() { + if line.starts_with(START_OF_HEADER) { + assert!(hunk_header.is_none(), "should not overwrite existing hunk_header"); + hunk_header = parse_hunk_header(line).ok(); + + continue; + } + + match line[0] { + b' ' | b'+' | b'-' => { + hunk_lines.extend_from_slice(line); + hunk_lines.push(b'\n'); + } + _ => unreachable!("BUG: expecting unified diff format"), + } + + match self.lines.peek() { + Some(next_line) if next_line.starts_with(START_OF_HEADER) => break, + None => break, + _ => {} + } + } + + hunk_header.map(|hunk_header| DiffHunk { + header: hunk_header, + lines: hunk_lines.into(), + }) + } + } + + fn skip_header(lines: &mut Lines) { + // diff --git a/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa b/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + // index ccccccc..ddddddd 100644 + // --- a/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + // +++ b/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb + + let line = lines.next().expect("line to be present"); + assert!(line.starts_with(b"diff --git ")); + + let line = lines.next().expect("line to be present"); + assert!(line.starts_with(b"index ")); + + let line = lines.next().expect("line to be present"); + assert!(line.starts_with(b"--- ")); + + let line = lines.next().expect("line to be present"); + assert!(line.starts_with(b"+++ ")); + } + + /// Parse diff hunk headers that conform to the unified diff hunk header format. + /// + /// The parser is very primitive and relies on the fact that `+18` is parsed as `18`. This + /// allows us to split the input on ` ` and `,` only. + /// + /// @@ -18,6 +18,7 @@ abc def ghi + /// @@ -{before_hunk_start},{before_hunk_len} +{after_hunk_start},{after_hunk_len} @@ + fn parse_hunk_header(line: &[u8]) -> gix_testtools::Result { + let line = line.strip_prefix(START_OF_HEADER).unwrap(); + + let parts: Vec<_> = line.split(|b| *b == b' ' || *b == b',').collect(); + let [before_hunk_start, before_hunk_len, after_hunk_start, after_hunk_len, ..] = parts[..] else { + unreachable!() + }; + + Ok(HunkHeader { + before_hunk_start: parse_number(before_hunk_start), + before_hunk_len: parse_number(before_hunk_len), + after_hunk_start: parse_number(after_hunk_start), + after_hunk_len: parse_number(after_hunk_len), + }) + } + + fn parse_number(bytes: &[u8]) -> u32 { + bytes + .to_str() + .expect("to be a valid UTF-8 string") + .parse::() + .expect("to be a number") + } +} diff --git a/gix-diff/tests/fixtures/.gitignore b/gix-diff/tests/fixtures/.gitignore new file mode 100644 index 00000000000..1f3f8401d34 --- /dev/null +++ b/gix-diff/tests/fixtures/.gitignore @@ -0,0 +1,2 @@ +# The slider assets that are needed for that slider fixture script to work and build the fixture. +/assets/ \ No newline at end of file diff --git a/gix-diff/tests/fixtures/generated-archives/.gitignore b/gix-diff/tests/fixtures/generated-archives/.gitignore new file mode 100644 index 00000000000..d08ec9b1c7a --- /dev/null +++ b/gix-diff/tests/fixtures/generated-archives/.gitignore @@ -0,0 +1,2 @@ +# The auto-generated sliders fixtures. For now it's experimental, but we may store it later once it's all working. +/make_diff_for_sliders_repo.tar \ No newline at end of file diff --git a/gix-diff/tests/fixtures/make_diff_for_sliders_repo.sh b/gix-diff/tests/fixtures/make_diff_for_sliders_repo.sh new file mode 100755 index 00000000000..deab2a921e7 --- /dev/null +++ b/gix-diff/tests/fixtures/make_diff_for_sliders_repo.sh @@ -0,0 +1,8 @@ +#!/usr/bin/env bash +set -eu -o pipefail + +# This script is a placeholder for a script generated by the +# `create-diff-cases` subcommand of `internal-tools`. The placeholder does +# nothing except making the associated test trivially pass. +# +# See the `gix-diff/tests/README.md` file for more information. diff --git a/gix-macros/src/momo.rs b/gix-macros/src/momo.rs index a007c1b2082..f8b8047dab4 100644 --- a/gix-macros/src/momo.rs +++ b/gix-macros/src/momo.rs @@ -184,13 +184,13 @@ fn convert( let pat_ident = match &mut *pat_type.pat { Pat::Ident(pat_ident) if pat_ident.by_ref.is_none() && pat_ident.subpat.is_none() => pat_ident, _ => { - pat_type.pat = Box::new(Pat::Ident(PatIdent { + *pat_type.pat = Pat::Ident(PatIdent { ident: Ident::new(&format!("arg_{i}_gen_by_momo_"), proc_macro2::Span::call_site()), attrs: Default::default(), by_ref: None, mutability: None, subpat: None, - })); + }); if let Pat::Ident(pat_ident) = &mut *pat_type.pat { pat_ident diff --git a/gix-protocol/src/fetch/arguments/async_io.rs b/gix-protocol/src/fetch/arguments/async_io.rs index e92d8faab44..cd702720504 100644 --- a/gix-protocol/src/fetch/arguments/async_io.rs +++ b/gix-protocol/src/fetch/arguments/async_io.rs @@ -1,15 +1,18 @@ use futures_lite::io::AsyncWriteExt; -use gix_transport::{client, client::TransportV2Ext}; +use gix_transport::client::{ + self, + async_io::{ExtendedBufRead, Transport, TransportV2Ext}, +}; use crate::{fetch::Arguments, Command}; impl Arguments { /// Send fetch arguments to the server, and indicate this is the end of negotiations only if `add_done_argument` is present. - pub async fn send<'a, T: client::Transport + 'a>( + pub async fn send<'a, T: Transport + 'a>( &mut self, transport: &'a mut T, add_done_argument: bool, - ) -> Result + Unpin + 'a>, client::Error> { + ) -> Result + Unpin + 'a>, client::Error> { if self.haves.is_empty() { assert!(add_done_argument, "If there are no haves, is_done must be true."); } diff --git a/gix-protocol/src/fetch/arguments/blocking_io.rs b/gix-protocol/src/fetch/arguments/blocking_io.rs index 0852f5a888b..9ccee601220 100644 --- a/gix-protocol/src/fetch/arguments/blocking_io.rs +++ b/gix-protocol/src/fetch/arguments/blocking_io.rs @@ -1,16 +1,19 @@ use std::io::Write; -use gix_transport::{client, client::TransportV2Ext}; +use gix_transport::client::{ + self, + blocking_io::{ExtendedBufRead, Transport, TransportV2Ext}, +}; use crate::{fetch::Arguments, Command}; impl Arguments { /// Send fetch arguments to the server, and indicate this is the end of negotiations only if `add_done_argument` is present. - pub fn send<'a, T: client::Transport + 'a>( + pub fn send<'a, T: Transport + 'a>( &mut self, transport: &'a mut T, add_done_argument: bool, - ) -> Result + Unpin + 'a>, client::Error> { + ) -> Result + Unpin + 'a>, client::Error> { if self.haves.is_empty() { assert!(add_done_argument, "If there are no haves, is_done must be true."); } diff --git a/gix-protocol/src/fetch/function.rs b/gix-protocol/src/fetch/function.rs index 1ac524cd21b..8a52e8ace2f 100644 --- a/gix-protocol/src/fetch/function.rs +++ b/gix-protocol/src/fetch/function.rs @@ -5,6 +5,10 @@ use std::{ use gix_features::progress::DynNestedProgress; +#[cfg(feature = "async-client")] +use crate::transport::client::async_io::{ExtendedBufRead, HandleProgress, Transport}; +#[cfg(feature = "blocking-client")] +use crate::transport::client::blocking_io::{ExtendedBufRead, HandleProgress, Transport}; use crate::{ fetch::{ negotiate, Arguments, Context, Error, Negotiate, NegotiateOutcome, Options, Outcome, ProgressId, Shallow, Tags, @@ -56,7 +60,7 @@ pub async fn fetch( where P: gix_features::progress::NestedProgress, P::SubProgress: 'static, - T: gix_transport::client::Transport, + T: Transport, E: Into>, { let _span = gix_trace::coarse!("gix_protocol::fetch()"); @@ -251,10 +255,9 @@ fn add_shallow_args( fn setup_remote_progress<'a>( progress: &mut dyn gix_features::progress::DynNestedProgress, - reader: &mut Box + Unpin + 'a>, + reader: &mut Box + Unpin + 'a>, should_interrupt: &'a AtomicBool, ) { - use crate::transport::client::ExtendedBufRead; reader.set_progress_handler(Some(Box::new({ let mut remote_progress = progress.add_child_with_id("remote".to_string(), ProgressId::RemoteProgress.into()); move |is_err: bool, data: &[u8]| { @@ -265,5 +268,5 @@ fn setup_remote_progress<'a>( ProgressAction::Continue } } - }) as crate::transport::client::HandleProgress<'a>)); + }) as HandleProgress<'a>)); } diff --git a/gix-protocol/src/fetch/handshake.rs b/gix-protocol/src/fetch/handshake.rs deleted file mode 100644 index 9ffc184a8ba..00000000000 --- a/gix-protocol/src/fetch/handshake.rs +++ /dev/null @@ -1,27 +0,0 @@ -use gix_features::progress::Progress; -use gix_transport::{client, Service}; -use maybe_async::maybe_async; - -use crate::{ - credentials, - handshake::{Error, Outcome}, -}; - -/// Perform a handshake with the server on the other side of `transport`, with `authenticate` being used if authentication -/// turns out to be required. `extra_parameters` are the parameters `(name, optional value)` to add to the handshake, -/// each time it is performed in case authentication is required. -/// `progress` is used to inform about what's currently happening. -#[allow(clippy::result_large_err)] -#[maybe_async] -pub async fn upload_pack( - transport: T, - authenticate: AuthFn, - extra_parameters: Vec<(String, Option)>, - progress: &mut impl Progress, -) -> Result -where - AuthFn: FnMut(credentials::helper::Action) -> credentials::protocol::Result, - T: client::Transport, -{ - crate::handshake(transport, Service::UploadPack, authenticate, extra_parameters, progress).await -} diff --git a/gix-protocol/src/fetch/mod.rs b/gix-protocol/src/fetch/mod.rs index 99f8df15162..1f8eb8445eb 100644 --- a/gix-protocol/src/fetch/mod.rs +++ b/gix-protocol/src/fetch/mod.rs @@ -5,7 +5,7 @@ /// /// * [handshake](handshake()) /// * **ls-refs** -/// * [get available refs by refspecs](RefMap::new()) +/// * [get available refs by refspecs](RefMap::fetch()) /// * **fetch pack** /// * `negotiate` until a pack can be received (TBD) /// * [officially terminate the connection](crate::indicate_end_of_interaction()) @@ -34,13 +34,6 @@ pub mod response; #[cfg(feature = "fetch")] pub(crate) mod function; -#[cfg(any(feature = "blocking-client", feature = "async-client"))] -#[cfg(feature = "handshake")] -mod handshake; -#[cfg(any(feature = "blocking-client", feature = "async-client"))] -#[cfg(feature = "handshake")] -pub use handshake::upload_pack as handshake; - #[cfg(feature = "fetch")] pub mod negotiate; diff --git a/gix-protocol/src/fetch/negotiate.rs b/gix-protocol/src/fetch/negotiate.rs index 1cd8ba9c666..7480aea9960 100644 --- a/gix-protocol/src/fetch/negotiate.rs +++ b/gix-protocol/src/fetch/negotiate.rs @@ -111,7 +111,7 @@ pub struct Round { /// * `graph` /// - The commit-graph for use by the `negotiator` - we populate it with tips to initialize the graph traversal. /// * `ref_map` -/// - The references known on the remote, as previously obtained with [`RefMap::new()`]. +/// - The references known on the remote, as previously obtained with [`RefMap::fetch()`]. /// * `shallow` /// - How to deal with shallow repositories. It does affect how negotiations are performed. /// * `mapping_is_ignored` diff --git a/gix-protocol/src/fetch/refmap/init.rs b/gix-protocol/src/fetch/refmap/init.rs index 7db8ea86dfd..abcf027cfb1 100644 --- a/gix-protocol/src/fetch/refmap/init.rs +++ b/gix-protocol/src/fetch/refmap/init.rs @@ -1,18 +1,22 @@ -use std::collections::HashSet; +use std::{borrow::Cow, collections::HashSet}; -use bstr::{BString, ByteVec}; +use bstr::{BString, ByteSlice, ByteVec}; use gix_features::progress::Progress; +use gix_transport::client::Capabilities; +#[cfg(feature = "async-client")] +use crate::transport::client::async_io::Transport; +#[cfg(feature = "blocking-client")] +use crate::transport::client::blocking_io::Transport; use crate::{ - fetch, fetch::{ refmap::{Mapping, Source, SpecIndex}, RefMap, }, - transport::client::Transport, + handshake::Ref, }; -/// The error returned by [`RefMap::new()`]. +/// The error returned by [`RefMap::fetch()`]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { @@ -24,96 +28,82 @@ pub enum Error { ListRefs(#[from] crate::ls_refs::Error), } -/// For use in [`RefMap::new()`]. +/// For use in [`RefMap::fetch()`]. #[derive(Debug, Clone)] -pub struct Options { - /// Use a two-component prefix derived from the ref-spec's source, like `refs/heads/` to let the server pre-filter refs - /// with great potential for savings in traffic and local CPU time. Defaults to `true`. - pub prefix_from_spec_as_filter_on_remote: bool, +pub struct Context { + /// All explicit refspecs to identify references on the remote that you are interested in. + /// Note that these are copied to [`RefMap::refspecs`] for convenience, as `RefMap::mappings` refer to them by index. + pub fetch_refspecs: Vec, /// A list of refspecs to use as implicit refspecs which won't be saved or otherwise be part of the remote in question. /// /// This is useful for handling `remote..tagOpt` for example. pub extra_refspecs: Vec, } -impl Default for Options { - fn default() -> Self { - Options { - prefix_from_spec_as_filter_on_remote: true, - extra_refspecs: Vec::new(), - } +impl Context { + fn aggregate_refspecs(&self) -> Vec { + let mut all_refspecs = self.fetch_refspecs.clone(); + all_refspecs.extend(self.extra_refspecs.iter().cloned()); + all_refspecs } } impl RefMap { - /// Create a new instance by obtaining all references on the remote that have been filtered through our remote's + /// Create a new instance by obtaining all references on the remote that have been filtered through our remote's specs /// for _fetching_. /// - /// A [context](fetch::Context) is provided to bundle what would be additional parameters, - /// and [options](Options) are used to further configure the call. - /// /// * `progress` is used if `ls-refs` is invoked on the remote. Always the case when V2 is used. - /// * `fetch_refspecs` are all explicit refspecs to identify references on the remote that you are interested in. - /// Note that these are copied to [`RefMap::refspecs`] for convenience, as `RefMap::mappings` refer to them by index. + /// * `capabilities` are the capabilities of the server, obtained by a [handshake](crate::handshake()). + /// * `transport` is a way to communicate with the server to obtain the reference listing. + /// * `user_agent` is passed to the server. + /// * `trace_packetlines` traces all packet lines if `true`, for debugging primarily. + /// * `prefix_from_spec_as_filter_on_remote` + /// - Use a two-component prefix derived from the ref-spec's source, like `refs/heads/` to let the server pre-filter refs + /// with great potential for savings in traffic and local CPU time. + /// * `context` to provide more [configuration](Context). #[allow(clippy::result_large_err)] #[maybe_async::maybe_async] - pub async fn new( + pub async fn fetch( mut progress: impl Progress, - fetch_refspecs: &[gix_refspec::RefSpec], - fetch::Context { - handshake, - transport, - user_agent, - trace_packetlines, - }: fetch::Context<'_, T>, - Options { - prefix_from_spec_as_filter_on_remote, - extra_refspecs, - }: Options, + capabilities: &Capabilities, + transport: &mut T, + user_agent: (&'static str, Option>), + trace_packetlines: bool, + prefix_from_spec_as_filter_on_remote: bool, + context: Context, ) -> Result where T: Transport, { let _span = gix_trace::coarse!("gix_protocol::fetch::RefMap::new()"); - let null = gix_hash::ObjectId::null(gix_hash::Kind::Sha1); // OK to hardcode Sha1, it's not supposed to match, ever. + let all_refspecs = context.aggregate_refspecs(); + let remote_refs = crate::ls_refs( + transport, + capabilities, + |_capabilities, arguments| { + push_prefix_arguments(prefix_from_spec_as_filter_on_remote, arguments, &all_refspecs); + Ok(crate::ls_refs::Action::Continue) + }, + &mut progress, + trace_packetlines, + user_agent, + ) + .await?; - let all_refspecs = { - let mut s: Vec<_> = fetch_refspecs.to_vec(); - s.extend(extra_refspecs.clone()); - s - }; - let remote_refs = match handshake.refs.take() { - Some(refs) => refs, - None => { - crate::ls_refs( - transport, - &handshake.capabilities, - |_capabilities, arguments, features| { - features.push(user_agent); - if prefix_from_spec_as_filter_on_remote { - let mut seen = HashSet::new(); - for spec in &all_refspecs { - let spec = spec.to_ref(); - if seen.insert(spec.instruction()) { - let mut prefixes = Vec::with_capacity(1); - spec.expand_prefixes(&mut prefixes); - for mut prefix in prefixes { - prefix.insert_str(0, "ref-prefix "); - arguments.push(prefix); - } - } - } - } - Ok(crate::ls_refs::Action::Continue) - }, - &mut progress, - trace_packetlines, - ) - .await? - } - }; + Self::from_refs(remote_refs, capabilities, context) + } + + /// Create a ref-map from already obtained `remote_refs`. Use `context` to pass in refspecs. + /// `capabilities` are used to determine the object format. + pub fn from_refs(remote_refs: Vec, capabilities: &Capabilities, context: Context) -> Result { + let all_refspecs = context.aggregate_refspecs(); + let Context { + fetch_refspecs, + extra_refspecs, + } = context; let num_explicit_specs = fetch_refspecs.len(); let group = gix_refspec::MatchGroup::from_fetch_specs(all_refspecs.iter().map(gix_refspec::RefSpec::to_ref)); + let null = gix_hash::ObjectId::null(gix_hash::Kind::Sha1); // OK to hardcode Sha1, it's not supposed to match, ever. let (res, fixes) = group .match_lhs(remote_refs.iter().map(|r| { let (full_ref_name, target, object) = r.unpack(); @@ -147,24 +137,9 @@ impl RefMap { }) .collect(); - let object_hash = extract_object_format(handshake)?; - Ok(RefMap { - mappings, - refspecs: fetch_refspecs.to_vec(), - extra_refspecs, - fixes, - remote_refs, - object_hash, - }) - } -} - -/// Assume sha1 if server says nothing, otherwise configure anything beyond sha1 in the local repo configuration -#[allow(clippy::result_large_err)] -fn extract_object_format(outcome: &crate::handshake::Outcome) -> Result { - use bstr::ByteSlice; - let object_hash = - if let Some(object_format) = outcome.capabilities.capability("object-format").and_then(|c| c.value()) { + // Assume sha1 if server says nothing, otherwise configure anything beyond sha1 in the local repo configuration + let object_hash = if let Some(object_format) = capabilities.capability("object-format").and_then(|c| c.value()) + { let object_format = object_format.to_str().map_err(|_| Error::UnknownObjectFormat { format: object_format.into(), })?; @@ -175,5 +150,37 @@ fn extract_object_format(outcome: &crate::handshake::Outcome) -> Result, + all_refspecs: &[gix_refspec::RefSpec], +) { + if !prefix_from_spec_as_filter_on_remote { + return; + } + + let mut seen = HashSet::new(); + for spec in all_refspecs { + let spec = spec.to_ref(); + if seen.insert(spec.instruction()) { + let mut prefixes = Vec::with_capacity(1); + spec.expand_prefixes(&mut prefixes); + for mut prefix in prefixes { + prefix.insert_str(0, "ref-prefix "); + arguments.push(prefix); + } + } + } } diff --git a/gix-protocol/src/fetch/response/async_io.rs b/gix-protocol/src/fetch/response/async_io.rs index 42be8b6e606..f5b3239e32a 100644 --- a/gix-protocol/src/fetch/response/async_io.rs +++ b/gix-protocol/src/fetch/response/async_io.rs @@ -1,5 +1,6 @@ use std::io; +use crate::transport::client::async_io::ExtendedBufRead; use gix_transport::{client, Protocol}; use crate::fetch::{ @@ -10,7 +11,7 @@ use crate::fetch::{ async fn parse_v2_section( line: &mut String, - reader: &mut (impl client::ExtendedBufRead<'_> + Unpin), + reader: &mut (impl ExtendedBufRead<'_> + Unpin), res: &mut Vec, parse: impl Fn(&str) -> Result, ) -> Result { @@ -44,7 +45,7 @@ impl Response { /// that `git` has to use to predict how many acks are supposed to be read. We also genuinely hope that this covers it all…. pub async fn from_line_reader( version: Protocol, - reader: &mut (impl client::ExtendedBufRead<'_> + Unpin), + reader: &mut (impl ExtendedBufRead<'_> + Unpin), client_expects_pack: bool, wants_to_negotiate: bool, ) -> Result { diff --git a/gix-protocol/src/fetch/response/blocking_io.rs b/gix-protocol/src/fetch/response/blocking_io.rs index 4143e93fb7f..80118a9c650 100644 --- a/gix-protocol/src/fetch/response/blocking_io.rs +++ b/gix-protocol/src/fetch/response/blocking_io.rs @@ -1,6 +1,7 @@ use std::io; -use gix_transport::{client, Protocol}; +use crate::transport::client::blocking_io::ExtendedBufRead; +use crate::transport::{client::MessageKind, Protocol}; use crate::fetch::{ response, @@ -10,7 +11,7 @@ use crate::fetch::{ fn parse_v2_section<'a, T>( line: &mut String, - reader: &mut impl client::ExtendedBufRead<'a>, + reader: &mut impl ExtendedBufRead<'a>, res: &mut Vec, parse: impl Fn(&str) -> Result, ) -> Result { @@ -20,7 +21,7 @@ fn parse_v2_section<'a, T>( line.clear(); } // End of message, or end of section? - Ok(if reader.stopped_at() == Some(client::MessageKind::Delimiter) { + Ok(if reader.stopped_at() == Some(MessageKind::Delimiter) { // try reading more sections reader.reset(Protocol::V2); false @@ -44,7 +45,7 @@ impl Response { /// that `git` has to use to predict how many acks are supposed to be read. We also genuinely hope that this covers it all…. pub fn from_line_reader<'a>( version: Protocol, - reader: &mut impl client::ExtendedBufRead<'a>, + reader: &mut impl ExtendedBufRead<'a>, client_expects_pack: bool, wants_to_negotiate: bool, ) -> Result { @@ -71,7 +72,7 @@ impl Response { // maybe we saw a shallow flush packet, let's reset and retry debug_assert_eq!( reader.stopped_at(), - Some(client::MessageKind::Flush), + Some(MessageKind::Flush), "If this isn't a flush packet, we don't know what's going on" ); reader.readline_str(&mut line)?; diff --git a/gix-protocol/src/fetch/response/mod.rs b/gix-protocol/src/fetch/response/mod.rs index 1bc3fa163a1..804e07f7d4a 100644 --- a/gix-protocol/src/fetch/response/mod.rs +++ b/gix-protocol/src/fetch/response/mod.rs @@ -185,7 +185,7 @@ impl Response { } /// Append the given `updates` which may have been obtained from a - /// (handshake::Outcome)[crate::handshake::Outcome::v1_shallow_updates]. + /// (handshake::Outcome)[crate::Handshake::v1_shallow_updates]. /// /// In V2, these are received as part of the pack, but V1 sends them early, so we /// offer to re-integrate them here. diff --git a/gix-protocol/src/fetch/types.rs b/gix-protocol/src/fetch/types.rs index f7d3415a783..549451f4703 100644 --- a/gix-protocol/src/fetch/types.rs +++ b/gix-protocol/src/fetch/types.rs @@ -18,18 +18,18 @@ pub struct Options<'a> { pub reject_shallow_remote: bool, } -/// For use in [`RefMap::new()`] and [`fetch`](crate::fetch()). +/// For use in [`RefMap::fetch()`] and [`fetch`](crate::fetch()). #[cfg(feature = "handshake")] pub struct Context<'a, T> { /// The outcome of the handshake performed with the remote. /// /// Note that it's mutable as depending on the protocol, it may contain refs that have been sent unconditionally. - pub handshake: &'a mut crate::handshake::Outcome, + pub handshake: &'a mut crate::Handshake, /// The transport to use when making an `ls-refs` or `fetch` call. /// /// This is always done if the underlying protocol is V2, which is implied by the absence of refs in the `handshake` outcome. pub transport: &'a mut T, - /// How to self-identify during the `ls-refs` call in [`RefMap::new()`] or the `fetch` call in [`fetch()`](crate::fetch()). + /// How to self-identify during the `ls-refs` call in [`RefMap::fetch()`] or the `fetch` call in [`fetch()`](crate::fetch()). /// /// This could be read from the `gitoxide.userAgent` configuration variable. pub user_agent: (&'static str, Option>), @@ -39,10 +39,7 @@ pub struct Context<'a, T> { #[cfg(feature = "fetch")] mod with_fetch { - use crate::{ - fetch, - fetch::{negotiate, refmap}, - }; + use crate::fetch::{self, negotiate, refmap}; /// For use in [`fetch`](crate::fetch()). pub struct NegotiateContext<'a, 'b, 'c, Objects, Alternates, AlternatesOut, AlternatesErr, Find> @@ -126,7 +123,7 @@ mod with_fetch { /// [`refmap::SpecIndex::ExplicitInRemote`] in [`refmap::Mapping`]. pub refspecs: Vec, /// Refspecs which have been added implicitly due to settings of the `remote`, usually pre-initialized from - /// [`extra_refspecs` in RefMap options](refmap::init::Options). + /// [`extra_refspecs` in RefMap options](refmap::init::Context). /// They are referred to by [`refmap::SpecIndex::Implicit`] in [`refmap::Mapping`]. /// /// They are never persisted nor are they typically presented to the user. diff --git a/gix-protocol/src/handshake/function.rs b/gix-protocol/src/handshake/function.rs index 6e5a6cc041f..3eac8f50d3e 100644 --- a/gix-protocol/src/handshake/function.rs +++ b/gix-protocol/src/handshake/function.rs @@ -1,14 +1,20 @@ use gix_features::{progress, progress::Progress}; -use gix_transport::{client, client::SetServiceResponse, Service}; +use gix_transport::{client, Service}; use maybe_async::maybe_async; -use super::{Error, Outcome}; +use super::Error; +#[cfg(feature = "async-client")] +use crate::transport::client::async_io::{SetServiceResponse, Transport}; +#[cfg(feature = "blocking-client")] +use crate::transport::client::blocking_io::{SetServiceResponse, Transport}; +use crate::Handshake; use crate::{credentials, handshake::refs}; /// Perform a handshake with the server on the other side of `transport`, with `authenticate` being used if authentication /// turns out to be required. `extra_parameters` are the parameters `(name, optional value)` to add to the handshake, /// each time it is performed in case authentication is required. /// `progress` is used to inform about what's currently happening. +/// The `service` tells the server whether to be in 'send' or 'receive' mode. #[allow(clippy::result_large_err)] #[maybe_async] pub async fn handshake( @@ -17,10 +23,10 @@ pub async fn handshake( mut authenticate: AuthFn, extra_parameters: Vec<(String, Option)>, progress: &mut impl Progress, -) -> Result +) -> Result where AuthFn: FnMut(credentials::helper::Action) -> credentials::protocol::Result, - T: client::Transport, + T: Transport, { let _span = gix_features::trace::detail!("gix_protocol::handshake()", service = ?service, extra_parameters = ?extra_parameters); let (server_protocol_version, refs, capabilities) = { @@ -99,7 +105,7 @@ where .map(|(refs, shallow)| (Some(refs), Some(shallow))) .unwrap_or_default(); - Ok(Outcome { + Ok(Handshake { server_protocol_version, refs, v1_shallow_updates, diff --git a/gix-protocol/src/handshake/mod.rs b/gix-protocol/src/handshake/mod.rs index 64ba19aa0eb..31128063a52 100644 --- a/gix-protocol/src/handshake/mod.rs +++ b/gix-protocol/src/handshake/mod.rs @@ -50,20 +50,70 @@ pub enum Ref { }, } -/// The result of the [`handshake()`][super::handshake()] function. -#[derive(Default, Debug, Clone)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg(feature = "handshake")] -pub struct Outcome { - /// The protocol version the server responded with. It might have downgraded the desired version. - pub server_protocol_version: gix_transport::Protocol, - /// The references reported as part of the `Protocol::V1` handshake, or `None` otherwise as V2 requires a separate request. - pub refs: Option>, - /// Shallow updates as part of the `Protocol::V1`, to shallow a particular object. - /// Note that unshallowing isn't supported here. - pub v1_shallow_updates: Option>, - /// The server capabilities. - pub capabilities: gix_transport::client::Capabilities, +pub(crate) mod hero { + use crate::handshake::Ref; + + /// The result of the [`handshake()`](crate::handshake()) function. + #[derive(Default, Debug, Clone)] + #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] + pub struct Handshake { + /// The protocol version the server responded with. It might have downgraded the desired version. + pub server_protocol_version: gix_transport::Protocol, + /// The references reported as part of the `Protocol::V1` handshake, or `None` otherwise as V2 requires a separate request. + pub refs: Option>, + /// Shallow updates as part of the `Protocol::V1`, to shallow a particular object. + /// Note that unshallowing isn't supported here. + pub v1_shallow_updates: Option>, + /// The server capabilities. + pub capabilities: gix_transport::client::Capabilities, + } + + #[cfg(feature = "fetch")] + mod fetch { + #[cfg(feature = "async-client")] + use crate::transport::client::async_io::Transport; + #[cfg(feature = "blocking-client")] + use crate::transport::client::blocking_io::Transport; + use crate::Handshake; + use gix_features::progress::Progress; + use std::borrow::Cow; + + impl Handshake { + /// Obtain a [refmap](crate::fetch::RefMap) either from this instance, taking it out in the process, if the handshake was + /// created from a V1 connection, or use `transport` to fetch the refmap as a separate command invocation. + #[allow(clippy::result_large_err)] + #[maybe_async::maybe_async] + pub async fn fetch_or_extract_refmap( + &mut self, + mut progress: impl Progress, + transport: &mut T, + user_agent: (&'static str, Option>), + trace_packetlines: bool, + prefix_from_spec_as_filter_on_remote: bool, + refmap_context: crate::fetch::refmap::init::Context, + ) -> Result + where + T: Transport, + { + Ok(match self.refs.take() { + Some(refs) => crate::fetch::RefMap::from_refs(refs, &self.capabilities, refmap_context)?, + None => { + crate::fetch::RefMap::fetch( + &mut progress, + &self.capabilities, + transport, + user_agent, + trace_packetlines, + prefix_from_spec_as_filter_on_remote, + refmap_context, + ) + .await? + } + }) + } + } + } } #[cfg(feature = "handshake")] @@ -73,7 +123,7 @@ mod error { use crate::{credentials, handshake::refs}; - /// The error returned by [`handshake()`][crate::fetch::handshake()]. + /// The error returned by [`handshake()`][crate::handshake()]. #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { diff --git a/gix-protocol/src/handshake/refs/async_io.rs b/gix-protocol/src/handshake/refs/async_io.rs index 996b83bca69..6dbecb304dc 100644 --- a/gix-protocol/src/handshake/refs/async_io.rs +++ b/gix-protocol/src/handshake/refs/async_io.rs @@ -1,10 +1,11 @@ +use crate::transport::client::async_io::ReadlineBufRead; use crate::{ fetch::response::ShallowUpdate, handshake::{refs, refs::parse::Error, Ref}, }; /// Parse refs from the given input line by line. Protocol V2 is required for this to succeed. -pub async fn from_v2_refs(in_refs: &mut dyn gix_transport::client::ReadlineBufRead) -> Result, Error> { +pub async fn from_v2_refs(in_refs: &mut dyn ReadlineBufRead) -> Result, Error> { let mut out_refs = Vec::new(); while let Some(line) = in_refs .readline() @@ -27,7 +28,7 @@ pub async fn from_v2_refs(in_refs: &mut dyn gix_transport::client::ReadlineBufRe /// Symbolic refs are shoe-horned into server capabilities whereas refs (without symbolic ones) are sent automatically as /// part of the handshake. Both symbolic and peeled refs need to be combined to fit into the [`Ref`] type provided here. pub async fn from_v1_refs_received_as_part_of_handshake_and_capabilities<'a>( - in_refs: &mut dyn gix_transport::client::ReadlineBufRead, + in_refs: &mut dyn ReadlineBufRead, capabilities: impl Iterator>, ) -> Result<(Vec, Vec), refs::parse::Error> { let mut out_refs = refs::shared::from_capabilities(capabilities)?; diff --git a/gix-protocol/src/handshake/refs/blocking_io.rs b/gix-protocol/src/handshake/refs/blocking_io.rs index 151f769586e..7f95ab0d322 100644 --- a/gix-protocol/src/handshake/refs/blocking_io.rs +++ b/gix-protocol/src/handshake/refs/blocking_io.rs @@ -1,10 +1,11 @@ +use crate::transport::client::blocking_io::ReadlineBufRead; use crate::{ fetch::response::ShallowUpdate, handshake::{refs, refs::parse::Error, Ref}, }; /// Parse refs from the given input line by line. Protocol V2 is required for this to succeed. -pub fn from_v2_refs(in_refs: &mut dyn gix_transport::client::ReadlineBufRead) -> Result, Error> { +pub fn from_v2_refs(in_refs: &mut dyn ReadlineBufRead) -> Result, Error> { let mut out_refs = Vec::new(); while let Some(line) = in_refs.readline().transpose()?.transpose()?.and_then(|l| l.as_bstr()) { out_refs.push(refs::shared::parse_v2(line)?); @@ -21,7 +22,7 @@ pub fn from_v2_refs(in_refs: &mut dyn gix_transport::client::ReadlineBufRead) -> /// Symbolic refs are shoe-horned into server capabilities whereas refs (without symbolic ones) are sent automatically as /// part of the handshake. Both symbolic and peeled refs need to be combined to fit into the [`Ref`] type provided here. pub fn from_v1_refs_received_as_part_of_handshake_and_capabilities<'a>( - in_refs: &mut dyn gix_transport::client::ReadlineBufRead, + in_refs: &mut dyn ReadlineBufRead, capabilities: impl Iterator>, ) -> Result<(Vec, Vec), Error> { let mut out_refs = refs::shared::from_capabilities(capabilities)?; diff --git a/gix-protocol/src/lib.rs b/gix-protocol/src/lib.rs index 2eea5911774..9765b102d61 100644 --- a/gix-protocol/src/lib.rs +++ b/gix-protocol/src/lib.rs @@ -2,7 +2,7 @@ //! //! Generally, there is the following order of operations. //! -//! * create a [`Transport`](gix_transport::client::Transport) +//! * create a `Transport`, either blocking or async //! * perform a [`handshake()`] //! * execute a [`Command`] //! - [list references](ls_refs()) @@ -61,6 +61,8 @@ pub mod handshake; #[cfg(any(feature = "blocking-client", feature = "async-client"))] #[cfg(feature = "handshake")] pub use handshake::function::handshake; +#[cfg(feature = "handshake")] +pub use handshake::hero::Handshake; /// pub mod ls_refs; diff --git a/gix-protocol/src/ls_refs.rs b/gix-protocol/src/ls_refs.rs index 6bd15108124..d9fd8c95c5d 100644 --- a/gix-protocol/src/ls_refs.rs +++ b/gix-protocol/src/ls_refs.rs @@ -47,34 +47,37 @@ pub(crate) mod function { use bstr::BString; use gix_features::progress::Progress; - use gix_transport::client::{Capabilities, Transport, TransportV2Ext}; + use gix_transport::client::Capabilities; use maybe_async::maybe_async; use super::{Action, Error}; + #[cfg(feature = "async-client")] + use crate::transport::client::async_io::{Transport, TransportV2Ext}; + #[cfg(feature = "blocking-client")] + use crate::transport::client::blocking_io::{Transport, TransportV2Ext}; use crate::{ handshake::{refs::from_v2_refs, Ref}, indicate_end_of_interaction, Command, }; - /// Invoke an ls-refs V2 command on `transport`, which requires a prior handshake that yielded - /// server `capabilities`. `prepare_ls_refs(capabilities, arguments, features)` can be used to alter the _ls-refs_. `progress` is used to provide feedback. - /// Note that `prepare_ls_refs()` is expected to add the `(agent, Some(name))` to the list of `features`. + /// Invoke a ls-refs V2 command on `transport`, which requires a prior handshake that yielded + /// server `capabilities`. `prepare_ls_refs(capabilities, arguments)` can be used to alter the _ls-refs_. + /// `progress` is used to provide feedback. + /// The `agent` information will be added to the features sent to the server. /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate. #[maybe_async] pub async fn ls_refs( mut transport: impl Transport, capabilities: &Capabilities, - prepare_ls_refs: impl FnOnce( - &Capabilities, - &mut Vec, - &mut Vec<(&str, Option>)>, - ) -> std::io::Result, + prepare_ls_refs: impl FnOnce(&Capabilities, &mut Vec) -> std::io::Result, progress: &mut impl Progress, trace: bool, + agent: (&'static str, Option>), ) -> Result, Error> { let _span = gix_features::trace::detail!("gix_protocol::ls_refs()", capabilities = ?capabilities); let ls_refs = Command::LsRefs; let mut ls_features = ls_refs.default_features(gix_transport::Protocol::V2, capabilities); + ls_features.push(agent); let mut ls_args = ls_refs.initial_v2_arguments(&ls_features); if capabilities .capability("ls-refs") @@ -83,7 +86,7 @@ pub(crate) mod function { { ls_args.push("unborn".into()); } - let refs = match prepare_ls_refs(capabilities, &mut ls_args, &mut ls_features) { + let refs = match prepare_ls_refs(capabilities, &mut ls_args) { Ok(Action::Skip) => Vec::new(), Ok(Action::Continue) => { ls_refs.validate_argument_prefixes( diff --git a/gix-protocol/src/util.rs b/gix-protocol/src/util.rs index adba4bb7784..920395e20b1 100644 --- a/gix-protocol/src/util.rs +++ b/gix-protocol/src/util.rs @@ -8,13 +8,16 @@ pub fn agent(name: impl Into) -> String { } #[cfg(any(feature = "blocking-client", feature = "async-client"))] mod with_transport { - use gix_transport::client::Transport; + #[cfg(feature = "async-client")] + use gix_transport::client::async_io::Transport; + #[cfg(feature = "blocking-client")] + use gix_transport::client::blocking_io::Transport; /// Send a message to indicate the remote side that there is nothing more to expect from us, indicating a graceful shutdown. /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate. #[maybe_async::maybe_async] pub async fn indicate_end_of_interaction( - mut transport: impl gix_transport::client::Transport, + mut transport: impl Transport, trace: bool, ) -> Result<(), gix_transport::client::Error> { // An empty request marks the (early) end of the interaction. Only relevant in stateful transports though. diff --git a/gix-protocol/tests/protocol/fetch/_impl.rs b/gix-protocol/tests/protocol/fetch/_impl.rs index c4171e20e80..c4484fda920 100644 --- a/gix-protocol/tests/protocol/fetch/_impl.rs +++ b/gix-protocol/tests/protocol/fetch/_impl.rs @@ -7,7 +7,10 @@ mod fetch_fn { fetch::{Arguments, Response}, indicate_end_of_interaction, Command, }; - use gix_transport::client; + #[cfg(feature = "async-client")] + use gix_transport::client::async_io::{ExtendedBufRead, HandleProgress, Transport}; + #[cfg(feature = "blocking-client")] + use gix_transport::client::blocking_io::{ExtendedBufRead, HandleProgress, Transport}; use maybe_async::maybe_async; use super::{Action, Delegate}; @@ -67,17 +70,18 @@ mod fetch_fn { where F: FnMut(credentials::helper::Action) -> credentials::protocol::Result, D: Delegate, - T: client::Transport, + T: Transport, P: NestedProgress + 'static, P::SubProgress: 'static, { - let gix_protocol::handshake::Outcome { + let gix_protocol::Handshake { server_protocol_version: protocol_version, refs, v1_shallow_updates: _ignored_shallow_updates_as_it_is_deprecated, capabilities, - } = gix_protocol::fetch::handshake( + } = gix_protocol::handshake( &mut transport, + gix_transport::Service::UploadPack, authenticate, delegate.handshake_extra_parameters(), &mut progress, @@ -91,13 +95,10 @@ mod fetch_fn { gix_protocol::ls_refs( &mut transport, &capabilities, - |a, b, c| { - let res = delegate.prepare_ls_refs(a, b, c); - c.push(("agent", Some(Cow::Owned(agent.clone())))); - res - }, + |a, b| delegate.prepare_ls_refs(a, b), &mut progress, trace, + ("agent", Some(Cow::Owned(agent.clone()))), ) .await? } @@ -171,10 +172,8 @@ mod fetch_fn { Ok(()) } - fn setup_remote_progress

( - progress: &mut P, - reader: &mut Box + Unpin + '_>, - ) where + fn setup_remote_progress<'a, P>(progress: &mut P, reader: &mut Box + Unpin + 'a>) + where P: NestedProgress, P::SubProgress: 'static, { @@ -184,7 +183,7 @@ mod fetch_fn { gix_protocol::RemoteProgress::translate_to_progress(is_err, data, &mut remote_progress); gix_transport::packetline::read::ProgressAction::Continue } - }) as gix_transport::client::HandleProgress<'_>)); + }) as HandleProgress<'a>)); } } pub use fetch_fn::{legacy_fetch as fetch, FetchConnection}; @@ -239,7 +238,6 @@ mod delegate { &mut self, _server: &Capabilities, _arguments: &mut Vec, - _features: &mut Vec<(&str, Option>)>, ) -> std::io::Result { Ok(ls_refs::Action::Continue) } @@ -309,9 +307,8 @@ mod delegate { &mut self, _server: &Capabilities, _arguments: &mut Vec, - _features: &mut Vec<(&str, Option>)>, ) -> io::Result { - self.deref_mut().prepare_ls_refs(_server, _arguments, _features) + self.deref_mut().prepare_ls_refs(_server, _arguments) } fn prepare_fetch( @@ -343,9 +340,8 @@ mod delegate { &mut self, _server: &Capabilities, _arguments: &mut Vec, - _features: &mut Vec<(&str, Option>)>, ) -> io::Result { - self.deref_mut().prepare_ls_refs(_server, _arguments, _features) + self.deref_mut().prepare_ls_refs(_server, _arguments) } fn prepare_fetch( diff --git a/gix-protocol/tests/protocol/fetch/arguments.rs b/gix-protocol/tests/protocol/fetch/arguments.rs index 9debb359557..52b82075c67 100644 --- a/gix-protocol/tests/protocol/fetch/arguments.rs +++ b/gix-protocol/tests/protocol/fetch/arguments.rs @@ -2,6 +2,10 @@ use bstr::ByteSlice; use gix_transport::Protocol; use crate::fetch; +#[cfg(feature = "async-client")] +use gix_transport::client::git::async_io::Connection; +#[cfg(feature = "blocking-client")] +use gix_transport::client::git::blocking_io::Connection; fn arguments_v1(features: impl IntoIterator) -> fetch::Arguments { fetch::Arguments::new(Protocol::V1, features.into_iter().map(|n| (n, None)).collect(), false) @@ -23,7 +27,10 @@ mod impls { use bstr::BStr; use gix_transport::{ client, - client::{Error, MessageKind, RequestWriter, SetServiceResponse, WriteMode}, + client::{ + blocking_io::{RequestWriter, SetServiceResponse}, + Error, MessageKind, WriteMode, + }, Protocol, Service, }; @@ -54,7 +61,7 @@ mod impls { } } - impl client::Transport for Transport { + impl client::blocking_io::Transport for Transport { fn handshake<'a>( &mut self, service: Service, @@ -81,12 +88,16 @@ mod impls { use async_trait::async_trait; use bstr::BStr; use gix_transport::{ - client, - client::{Error, MessageKind, RequestWriter, SetServiceResponse, WriteMode}, + client::{ + self, + async_io::{RequestWriter, SetServiceResponse}, + Error, MessageKind, WriteMode, + }, Protocol, Service, }; use super::Transport; + impl client::TransportWithoutIO for Transport { fn set_identity(&mut self, identity: client::Account) -> Result<(), Error> { self.inner.set_identity(identity) @@ -113,7 +124,7 @@ mod impls { } #[async_trait(?Send)] - impl client::Transport for Transport { + impl client::async_io::Transport for Transport { async fn handshake<'a>( &mut self, service: Service, @@ -133,12 +144,9 @@ mod impls { } } -fn transport( - out: &mut Vec, - stateful: bool, -) -> Transport>> { +fn transport(out: &mut Vec, stateful: bool) -> Transport>> { Transport { - inner: gix_transport::client::git::Connection::new( + inner: Connection::new( &[], out, Protocol::V1, // does not matter diff --git a/gix-protocol/tests/protocol/fetch/mod.rs b/gix-protocol/tests/protocol/fetch/mod.rs index 355afbb2850..6725959a58f 100644 --- a/gix-protocol/tests/protocol/fetch/mod.rs +++ b/gix-protocol/tests/protocol/fetch/mod.rs @@ -106,7 +106,6 @@ impl DelegateBlocking for CloneRefInWantDelegate { &mut self, _server: &Capabilities, _arguments: &mut Vec, - _features: &mut Vec<(&str, Option>)>, ) -> io::Result { Ok(ls_refs::Action::Skip) } @@ -150,7 +149,6 @@ impl DelegateBlocking for LsRemoteDelegate { &mut self, _server: &Capabilities, _arguments: &mut Vec, - _features: &mut Vec<(&str, Option>)>, ) -> std::io::Result { match self.abort_with.take() { Some(err) => Err(err), @@ -302,9 +300,9 @@ pub fn transport( path: &str, desired_version: gix_transport::Protocol, mode: gix_transport::client::git::ConnectMode, -) -> gix_transport::client::git::Connection { +) -> gix_transport::client::git::async_io::Connection { let response = fixture_bytes(path); - gix_transport::client::git::Connection::new( + gix_transport::client::git::async_io::Connection::new( Cursor::new(response), out, desired_version, @@ -321,9 +319,9 @@ pub fn transport( path: &str, version: gix_transport::Protocol, mode: gix_transport::client::git::ConnectMode, -) -> gix_transport::client::git::Connection { +) -> gix_transport::client::git::blocking_io::Connection { let response = fixture_bytes(path); - gix_transport::client::git::Connection::new( + gix_transport::client::git::blocking_io::Connection::new( Cursor::new(response), out, version, diff --git a/gix-protocol/tests/protocol/fetch/response.rs b/gix-protocol/tests/protocol/fetch/response.rs index 981ae139c83..db54e89be0a 100644 --- a/gix-protocol/tests/protocol/fetch/response.rs +++ b/gix-protocol/tests/protocol/fetch/response.rs @@ -215,6 +215,10 @@ mod v2 { self, response::{Acknowledgement, ShallowUpdate}, }; + #[cfg(feature = "async-client")] + use gix_transport::client::async_io::HandleProgress; + #[cfg(feature = "blocking-client")] + use gix_transport::client::blocking_io::HandleProgress; use gix_transport::Protocol; use crate::fetch::response::{id, mock_reader}; @@ -320,7 +324,7 @@ mod v2 { reader.set_progress_handler(Some(Box::new(|is_err: bool, _data: &[u8]| { assert!(!is_err, "fixture does not have an error"); ProgressAction::Continue - }) as gix_transport::client::HandleProgress)); + }) as HandleProgress)); let bytes_read = reader.read_to_end(&mut buf).await?; assert_eq!(bytes_read, 1643, "should be able to read the whole pack"); assert_eq!(&buf[..4], b"PACK"); @@ -376,7 +380,7 @@ mod v2 { reader.set_progress_handler(Some(Box::new(|a: bool, b: &[u8]| { gix_protocol::RemoteProgress::translate_to_progress(a, b, &mut gix_features::progress::Discard); ProgressAction::Continue - }) as gix_transport::client::HandleProgress)); + }) as HandleProgress)); let bytes_read = reader.read_to_end(&mut buf).await?; assert_eq!(bytes_read, 5360, "should be able to read the whole pack"); Ok(()) diff --git a/gix-protocol/tests/protocol/handshake.rs b/gix-protocol/tests/protocol/handshake.rs index b0d8e42d0a7..6aae74984a1 100644 --- a/gix-protocol/tests/protocol/handshake.rs +++ b/gix-protocol/tests/protocol/handshake.rs @@ -202,7 +202,7 @@ impl std::io::BufRead for Fixture<'_> { } #[cfg(feature = "blocking-client")] -impl gix_transport::client::ReadlineBufRead for Fixture<'_> { +impl gix_transport::client::blocking_io::ReadlineBufRead for Fixture<'_> { fn readline( &mut self, ) -> Option, gix_packetline::decode::Error>>> { @@ -266,7 +266,7 @@ impl futures_io::AsyncBufRead for Fixture<'_> { #[cfg(feature = "async-client")] #[async_trait::async_trait(?Send)] -impl gix_transport::client::ReadlineBufRead for Fixture<'_> { +impl gix_transport::client::async_io::ReadlineBufRead for Fixture<'_> { async fn readline( &mut self, ) -> Option, gix_packetline::decode::Error>>> { diff --git a/gix-ref/src/fullname.rs b/gix-ref/src/fullname.rs index 423baf5a26b..40cf7bc8661 100644 --- a/gix-ref/src/fullname.rs +++ b/gix-ref/src/fullname.rs @@ -1,8 +1,7 @@ +use crate::{bstr::ByteVec, name::is_pseudo_ref, Category, FullName, FullNameRef, Namespace, PartialNameRef}; use gix_object::bstr::{BStr, BString, ByteSlice}; use std::{borrow::Borrow, path::Path}; -use crate::{bstr::ByteVec, name::is_pseudo_ref, Category, FullName, FullNameRef, Namespace, PartialNameRef}; - impl TryFrom<&str> for FullName { type Error = gix_validate::reference::name::Error; @@ -165,6 +164,8 @@ impl FullNameRef { impl Category<'_> { /// As the inverse of [`FullNameRef::category_and_short_name()`], use the prefix of this category alongside /// the `short_name` to create a valid fully qualified [reference name](FullName). + /// + /// If `short_name` already contains the prefix that it would receive (and is thus a full name), no duplication will occur. pub fn to_full_name<'a>(&self, short_name: impl Into<&'a BStr>) -> Result { let mut out: BString = self.prefix().into(); let short_name = short_name.into(); @@ -185,8 +186,12 @@ impl Category<'_> { | Category::PseudoRef | Category::MainPseudoRef => short_name, }; - out.extend_from_slice(partial_name); - FullName::try_from(out) + if out.is_empty() || !partial_name.starts_with(&out) { + out.extend_from_slice(partial_name); + FullName::try_from(out) + } else { + FullName::try_from(partial_name.as_bstr()) + } } } diff --git a/gix-ref/tests/refs/fullname.rs b/gix-ref/tests/refs/fullname.rs index 3208b19dde6..ebc7f8fe01b 100644 --- a/gix-ref/tests/refs/fullname.rs +++ b/gix-ref/tests/refs/fullname.rs @@ -117,6 +117,25 @@ fn shorten_and_category() { ); } +#[test] +fn to_full_name() -> gix_testtools::Result { + assert_eq!( + Category::LocalBranch.to_full_name("refs/heads/full")?.as_bstr(), + "refs/heads/full", + "prefixes aren't duplicated" + ); + + assert_eq!( + Category::LocalBranch + .to_full_name("refs/remotes/origin/other")? + .as_bstr(), + "refs/heads/refs/remotes/origin/other", + "full names with a different category will be prefixed, to support 'main-worktree' special cases" + ); + + Ok(()) +} + #[test] fn prefix_with_namespace_and_stripping() { let ns = gix_ref::namespace::expand("foo").unwrap(); diff --git a/gix-transport/Cargo.toml b/gix-transport/Cargo.toml index 735851388e3..a73c202b379 100644 --- a/gix-transport/Cargo.toml +++ b/gix-transport/Cargo.toml @@ -17,11 +17,7 @@ doctest = false [features] default = [] -#! ### _Mutually Exclusive Client_ -#! The _client_ portion of transport can be blocking or async. If none is selected, it will be missing entirely. -#! Specifying both causes a compile error, preventing the use of `--all-features`. - -## If set, blocking implementations of the typical git transports become available in `crate::client` +## If set, blocking implementations of the typical git transports become available in `crate::client::blocking_io` blocking-client = ["gix-packetline/blocking-io"] ## Implies `blocking-client`, and adds support for the http and https transports. http-client = [ @@ -31,10 +27,12 @@ http-client = [ "gix-credentials", ] ## Implies `http-client`, and adds support for the http and https transports using the Rust bindings for `libcurl`. +## +## If used in conjunction with other blocking implementations like `http-client-reqwest`, this one takes precedence. http-client-curl = ["curl", "http-client"] ## Implies `http-client-curl` and enables `rustls` for creating `https://` connections. http-client-curl-rust-tls = ["http-client-curl", "curl/rustls"] -### Implies `http-client` and adds support for http and https transports using the blocking version of `reqwest`. +## Implies `http-client` and adds support for http and https transports using the blocking version of `reqwest`. http-client-reqwest = ["reqwest", "http-client"] ## Stacks with `blocking-http-transport-reqwest` and enables `https://` via the `rustls` crate. http-client-reqwest-rust-tls = ["http-client-reqwest", "reqwest/rustls-tls"] @@ -49,7 +47,7 @@ http-client-reqwest-rust-tls-trust-dns = [ http-client-reqwest-native-tls = ["http-client-reqwest", "reqwest/default-tls"] ## Allows sending credentials over cleartext HTTP. For testing purposes only. http-client-insecure-credentials = [] -## If set, an async implementations of the git transports becomes available in `crate::client`. +## If set, an async implementations of the git transports becomes available in `crate::client::async_io`. ## Suitable for implementing your own transports while using git's way of communication, typically in conjunction with a custom server. ## **Note** that the _blocking_ client has a wide range of available transports, with the _async_ version of it supporting only the TCP based `git` transport leaving you ## with the responsibility to providing such an implementation of `futures-io::AsyncRead/AsyncWrite` yourself. @@ -63,7 +61,7 @@ async-client = [ #! ### Other ## Data structures implement `serde::Serialize` and `serde::Deserialize`. -serde = ["dep:serde"] +serde = ["dep:serde", "bstr/serde"] [[test]] name = "blocking-transport" diff --git a/gix-transport/src/client/async_io/connect.rs b/gix-transport/src/client/async_io/connect.rs index 72ba2289613..cd5a3a7fda1 100644 --- a/gix-transport/src/client/async_io/connect.rs +++ b/gix-transport/src/client/async_io/connect.rs @@ -2,7 +2,7 @@ pub use crate::client::non_io_types::connect::{Error, Options}; #[cfg(feature = "async-std")] pub(crate) mod function { - use crate::client::{git, non_io_types::connect::Error}; + use crate::client::{async_io::Transport, git::async_io::Connection, non_io_types::connect::Error}; /// A general purpose connector connecting to a repository identified by the given `url`. /// @@ -10,10 +10,7 @@ pub(crate) mod function { /// [git daemons][crate::client::git::connect()] only at the moment. /// /// Use `options` to further control specifics of the transport resulting from the connection. - pub async fn connect( - url: Url, - options: super::Options, - ) -> Result, Error> + pub async fn connect(url: Url, options: super::Options) -> Result, Error> where Url: TryInto, gix_url::parse::Error: From, @@ -29,7 +26,7 @@ pub(crate) mod function { } let path = std::mem::take(&mut url.path); Box::new( - git::Connection::new_tcp( + Connection::new_tcp( url.host().expect("host is present in url"), url.port, path, @@ -44,3 +41,6 @@ pub(crate) mod function { }) } } + +#[cfg(feature = "async-std")] +pub use function::connect; diff --git a/gix-transport/src/client/blocking_io/connect.rs b/gix-transport/src/client/blocking_io/connect.rs index ba9519d4b27..ffc4c5b2a53 100644 --- a/gix-transport/src/client/blocking_io/connect.rs +++ b/gix-transport/src/client/blocking_io/connect.rs @@ -3,7 +3,7 @@ pub use crate::client::non_io_types::connect::{Error, Options}; pub(crate) mod function { #[cfg(feature = "http-client-curl")] use crate::client::blocking_io::http::curl::Curl; - #[cfg(feature = "http-client-reqwest")] + #[cfg(all(feature = "http-client-reqwest", not(feature = "http-client-curl")))] use crate::client::blocking_io::http::reqwest::Remote as Reqwest; use crate::client::{blocking_io::Transport, non_io_types::connect::Error}; @@ -12,8 +12,8 @@ pub(crate) mod function { /// This includes connections to /// [local repositories](crate::client::blocking_io::file::connect()), /// [repositories over ssh](crate::client::blocking_io::ssh::connect()), - /// [git daemons](crate::client::git::connect()), - /// and if compiled in connections to [git repositories over https](crate::client::http::connect()). + /// [git daemons](crate::client::blocking_io::connect::connect()), + /// and if compiled in connections to [git repositories over https](crate::client::blocking_io::http::connect()). /// /// Use `options` to further control specifics of the transport resulting from the connection. pub fn connect(url: Url, options: super::Options) -> Result, Error> @@ -74,3 +74,5 @@ pub(crate) mod function { }) } } + +pub use function::connect; diff --git a/gix-transport/src/client/blocking_io/file.rs b/gix-transport/src/client/blocking_io/file.rs index 520e160f847..b91f1c1a281 100644 --- a/gix-transport/src/client/blocking_io/file.rs +++ b/gix-transport/src/client/blocking_io/file.rs @@ -41,7 +41,7 @@ const ENV_VARS_TO_REMOVE: &[&str] = &[ /// A utility to spawn a helper process to actually transmit data, possibly over `ssh`. /// -/// It can only be instantiated using the local [`connect()`] or [ssh connect][crate::client::ssh::connect()]. +/// It can only be instantiated using the local [`connect()`] or [ssh connect][super::ssh::connect()]. pub struct SpawnProcessOnDemand { desired_version: Protocol, url: gix_url::Url, diff --git a/gix-transport/src/client/blocking_io/http/mod.rs b/gix-transport/src/client/blocking_io/http/mod.rs index 454af724a55..df4bda43512 100644 --- a/gix-transport/src/client/blocking_io/http/mod.rs +++ b/gix-transport/src/client/blocking_io/http/mod.rs @@ -19,7 +19,8 @@ use crate::{ http::options::{HttpVersion, SslVersionRangeInclusive}, ExtendedBufRead, HandleProgress, RequestWriter, SetServiceResponse, }, - capabilities, Capabilities, MessageKind, + capabilities::blocking_recv::Handshake, + MessageKind, }, packetline::{blocking_io::StreamingPeekableIter, PacketLineRef}, Protocol, Service, @@ -402,11 +403,11 @@ impl blocking_io::Transport for Transport { line_reader.as_read().read_to_end(&mut Vec::new())?; } - let capabilities::recv::Outcome { + let Handshake { capabilities, refs, protocol: actual_protocol, - } = Capabilities::from_lines_with_version_detection(line_reader)?; + } = Handshake::from_lines_with_version_detection(line_reader)?; self.actual_version = actual_protocol; self.service = Some(service); Ok(SetServiceResponse { diff --git a/gix-transport/src/client/blocking_io/http/reqwest/mod.rs b/gix-transport/src/client/blocking_io/http/reqwest/mod.rs index 7c68b166ea2..1ca3113479c 100644 --- a/gix-transport/src/client/blocking_io/http/reqwest/mod.rs +++ b/gix-transport/src/client/blocking_io/http/reqwest/mod.rs @@ -7,7 +7,7 @@ pub struct Remote { /// A channel to receive the result of the prior request. response: std::sync::mpsc::Receiver, /// A mechanism for configuring the remote. - config: crate::client::http::Options, + config: crate::client::blocking_io::http::Options, } /// A function to configure a single request prior to sending it, support most complex configuration beyond what's possible with diff --git a/gix-transport/src/client/blocking_io/http/reqwest/remote.rs b/gix-transport/src/client/blocking_io/http/reqwest/remote.rs index 40b480b6ee7..50e1c36b2a7 100644 --- a/gix-transport/src/client/blocking_io/http/reqwest/remote.rs +++ b/gix-transport/src/client/blocking_io/http/reqwest/remote.rs @@ -7,7 +7,9 @@ use std::{ use gix_features::io::pipe; -use crate::client::http::{self, options::FollowRedirects, redirect, reqwest::Remote, traits::PostBodyDataKind}; +use crate::client::blocking_io::http::{ + self, options::FollowRedirects, redirect, reqwest::Remote, traits::PostBodyDataKind, +}; /// The error returned by the 'remote' helper, a purely internal construct to perform http requests. #[derive(Debug, thiserror::Error)] diff --git a/gix-transport/src/client/blocking_io/http/traits.rs b/gix-transport/src/client/blocking_io/http/traits.rs index b13473a38b0..fd59ef957ca 100644 --- a/gix-transport/src/client/blocking_io/http/traits.rs +++ b/gix-transport/src/client/blocking_io/http/traits.rs @@ -25,7 +25,7 @@ impl crate::IsSpuriousError for Error { return err.is_spurious(); } #[cfg(feature = "http-client-reqwest")] - if let Some(err) = source.downcast_ref::() { + if let Some(err) = source.downcast_ref::() { return err.is_spurious(); } false diff --git a/gix-transport/src/client/capabilities.rs b/gix-transport/src/client/capabilities.rs index ddb3682d070..79bf34a6347 100644 --- a/gix-transport/src/client/capabilities.rs +++ b/gix-transport/src/client/capabilities.rs @@ -165,9 +165,9 @@ impl Capabilities { } } -#[cfg(feature = "blocking-client")] /// -pub mod recv { +#[cfg(feature = "blocking-client")] +pub mod blocking_recv { use std::io; use bstr::ByteVec; @@ -178,8 +178,8 @@ pub mod recv { Protocol, }; - /// Success outcome of [`Capabilities::from_lines_with_version_detection`]. - pub struct Outcome<'a> { + /// The information provided by the server upon first connection. + pub struct Handshake<'a> { /// The [`Capabilities`] the remote advertised. pub capabilities: Capabilities, /// The remote refs as a [`io::BufRead`]. @@ -191,14 +191,14 @@ pub mod recv { pub protocol: Protocol, } - impl Capabilities { + impl Handshake<'_> { /// Read the capabilities and version advertisement from the given packetline reader. /// /// If [`Protocol::V1`] was requested, or the remote decided to downgrade, the remote refs - /// advertisement will also be included in the [`Outcome`]. + /// advertisement will also be included in the [`Handshake`]. pub fn from_lines_with_version_detection( rd: &mut StreamingPeekableIter, - ) -> Result, client::Error> { + ) -> Result, client::Error> { // NOTE that this is vitally important - it is turned on and stays on for all following requests so // we automatically abort if the server sends an ERR line anywhere. // We are sure this can't clash with binary data when sent due to the way the PACK @@ -214,13 +214,13 @@ pub mod recv { Protocol::V1 => { let (capabilities, delimiter_position) = Capabilities::from_bytes(line.0)?; rd.peek_buffer_replace_and_truncate(delimiter_position, b'\n'); - Outcome { + Handshake { capabilities, refs: Some(Box::new(rd.as_read())), protocol: Protocol::V1, } } - Protocol::V2 => Outcome { + Protocol::V2 => Handshake { capabilities: { let mut rd = rd.as_read(); let mut buf = Vec::new(); @@ -243,7 +243,7 @@ pub mod recv { }, } } - None => Outcome { + None => Handshake { capabilities: Capabilities::default(), refs: Some(Box::new(rd.as_read())), protocol: Protocol::V0, @@ -253,10 +253,10 @@ pub mod recv { } } -#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] -#[allow(missing_docs)] /// -pub mod recv { +#[cfg(feature = "async-client")] +#[allow(missing_docs)] +pub mod async_recv { use bstr::ByteVec; use futures_io::AsyncRead; @@ -266,8 +266,8 @@ pub mod recv { Protocol, }; - /// Success outcome of [`Capabilities::from_lines_with_version_detection`]. - pub struct Outcome<'a> { + /// The information provided by the server upon first connection. + pub struct Handshake<'a> { /// The [`Capabilities`] the remote advertised. pub capabilities: Capabilities, /// The remote refs as an [`AsyncBufRead`]. @@ -279,14 +279,14 @@ pub mod recv { pub protocol: Protocol, } - impl Capabilities { + impl Handshake<'_> { /// Read the capabilities and version advertisement from the given packetline reader. /// /// If [`Protocol::V1`] was requested, or the remote decided to downgrade, the remote refs - /// advertisement will also be included in the [`Outcome`]. + /// advertisement will also be included in the [`Handshake`]. pub async fn from_lines_with_version_detection( rd: &mut StreamingPeekableIter, - ) -> Result, client::Error> { + ) -> Result, client::Error> { // NOTE that this is vitally important - it is turned on and stays on for all following requests so // we automatically abort if the server sends an ERR line anywhere. // We are sure this can't clash with binary data when sent due to the way the PACK @@ -302,13 +302,13 @@ pub mod recv { Protocol::V1 => { let (capabilities, delimiter_position) = Capabilities::from_bytes(line.0)?; rd.peek_buffer_replace_and_truncate(delimiter_position, b'\n'); - Outcome { + Handshake { capabilities, refs: Some(Box::new(rd.as_read())), protocol: Protocol::V1, } } - Protocol::V2 => Outcome { + Protocol::V2 => Handshake { capabilities: { let mut rd = rd.as_read(); let mut buf = Vec::new(); @@ -331,7 +331,7 @@ pub mod recv { }, } } - None => Outcome { + None => Handshake { capabilities: Capabilities::default(), refs: Some(Box::new(rd.as_read())), protocol: Protocol::V0, diff --git a/gix-transport/src/client/git/async_io.rs b/gix-transport/src/client/git/async_io.rs index 9ecbfd84976..74feeddc7fb 100644 --- a/gix-transport/src/client/git/async_io.rs +++ b/gix-transport/src/client/git/async_io.rs @@ -9,9 +9,8 @@ use crate::{ client::{ self, async_io::{RequestWriter, SetServiceResponse}, - capabilities, + capabilities::async_recv::Handshake, git::{self, ConnectionState}, - Capabilities, }, packetline::{ async_io::{StreamingPeekableIter, Writer}, @@ -97,11 +96,11 @@ where line_writer.flush().await?; } - let capabilities::recv::Outcome { + let Handshake { capabilities, refs, protocol: actual_protocol, - } = Capabilities::from_lines_with_version_detection(&mut self.line_provider).await?; + } = Handshake::from_lines_with_version_detection(&mut self.line_provider).await?; Ok(SetServiceResponse { actual_protocol, capabilities, @@ -164,9 +163,12 @@ mod async_net { use async_std::net::TcpStream; - use crate::client::{git, Error}; + use crate::client::{ + git::{async_io::Connection, ConnectMode}, + Error, + }; - impl git::Connection { + impl Connection { /// Create a new TCP connection using the `git` protocol of `desired_version`, and make a connection to `host` /// at `port` for accessing the repository at `path` on the server side. /// If `trace` is `true`, all packetlines received or sent will be passed to the facilities of the `gix-trace` crate. @@ -176,20 +178,20 @@ mod async_net { path: bstr::BString, desired_version: crate::Protocol, trace: bool, - ) -> Result, Error> { + ) -> Result { let read = async_std::io::timeout( Duration::from_secs(5), TcpStream::connect(&(host, port.unwrap_or(9418))), ) .await?; let write = read.clone(); - Ok(git::Connection::new( + Ok(Self::new( read, write, desired_version, path, None::<(String, _)>, - git::ConnectMode::Daemon, + ConnectMode::Daemon, trace, )) } diff --git a/gix-transport/src/client/git/blocking_io.rs b/gix-transport/src/client/git/blocking_io.rs index 1f61bd970ec..92fe5b61a95 100644 --- a/gix-transport/src/client/git/blocking_io.rs +++ b/gix-transport/src/client/git/blocking_io.rs @@ -6,9 +6,8 @@ use crate::{ client::{ self, blocking_io::{RequestWriter, SetServiceResponse}, - capabilities, + capabilities::blocking_recv::Handshake, git::{self, ConnectionState}, - Capabilities, }, packetline::{ blocking_io::{StreamingPeekableIter, Writer}, @@ -19,8 +18,8 @@ use crate::{ /// A TCP connection to either a `git` daemon or a spawned `git` process. /// -/// When connecting to a daemon, additional context information is sent with the first line of the handshake. Otherwise, that -/// context is passed using command line arguments to a [spawned `git` process](crate::client::file::SpawnProcessOnDemand). +/// When connecting to a daemon, additional context information is sent with the first line of the handshake. Otherwise that +/// context is passed using command line arguments to a [spawned `git` process][crate::client::blocking_io::file::SpawnProcessOnDemand]. pub struct Connection { pub(in crate::client) writer: W, pub(in crate::client) line_provider: StreamingPeekableIter, @@ -92,11 +91,11 @@ where line_writer.flush()?; } - let capabilities::recv::Outcome { + let Handshake { capabilities, refs, protocol: actual_protocol, - } = Capabilities::from_lines_with_version_detection(&mut self.line_provider)?; + } = Handshake::from_lines_with_version_detection(&mut self.line_provider)?; Ok(SetServiceResponse { actual_protocol, capabilities, diff --git a/gix-transport/src/client/git/mod.rs b/gix-transport/src/client/git/mod.rs index 37d6193f5d5..02179060ca4 100644 --- a/gix-transport/src/client/git/mod.rs +++ b/gix-transport/src/client/git/mod.rs @@ -161,12 +161,10 @@ mod message { } } -#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] -pub(crate) mod async_io; -#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] -pub use async_io::Connection; +/// +#[cfg(feature = "async-client")] +pub mod async_io; +/// #[cfg(feature = "blocking-client")] -pub(crate) mod blocking_io; -#[cfg(feature = "blocking-client")] -pub use blocking_io::{connect, Connection}; +pub mod blocking_io; diff --git a/gix-transport/src/client/mod.rs b/gix-transport/src/client/mod.rs index b8970f99d6a..038030a2855 100644 --- a/gix-transport/src/client/mod.rs +++ b/gix-transport/src/client/mod.rs @@ -1,26 +1,13 @@ -#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] -mod async_io; -#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] -pub use async_io::{ - connect, ExtendedBufRead, HandleProgress, ReadlineBufRead, RequestWriter, SetServiceResponse, Transport, - TransportV2Ext, -}; +/// +#[cfg(feature = "async-client")] +pub mod async_io; mod traits; pub use traits::TransportWithoutIO; +/// #[cfg(feature = "blocking-client")] -mod blocking_io; -#[cfg(feature = "http-client")] -pub use blocking_io::http; -#[cfg(feature = "blocking-client")] -pub use blocking_io::{ - connect, file, ssh, ExtendedBufRead, HandleProgress, ReadlineBufRead, RequestWriter, SetServiceResponse, Transport, - TransportV2Ext, -}; -#[cfg(feature = "blocking-client")] -#[doc(inline)] -pub use connect::function::connect; +pub mod blocking_io; /// pub mod capabilities; diff --git a/gix-transport/src/client/non_io_types.rs b/gix-transport/src/client/non_io_types.rs index c28e628c34d..98f79020a46 100644 --- a/gix-transport/src/client/non_io_types.rs +++ b/gix-transport/src/client/non_io_types.rs @@ -1,4 +1,4 @@ -/// Configure how the [`RequestWriter`][crate::client::RequestWriter] behaves when writing bytes. +/// Configure how a `RequestWriter` behaves when writing bytes. #[derive(Default, PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum WriteMode { @@ -15,8 +15,9 @@ pub enum WriteMode { OneLfTerminatedLinePerWriteCall, } -/// The kind of packet line to write when transforming a [`RequestWriter`][crate::client::RequestWriter] into an -/// [`ExtendedBufRead`][crate::client::ExtendedBufRead]. +/// The kind of packet line to write when transforming a `RequestWriter` into an `ExtendedBufRead`. +/// +/// Both the type and the trait have different implementations for blocking vs async I/O. #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone, Copy)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum MessageKind { @@ -44,7 +45,9 @@ pub(crate) mod connect { pub trace: bool, } - /// The error used in [`connect()`][crate::connect()]. + /// The error used in `connect()`. + /// + /// (Both blocking and async I/O use the same error type.) #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] pub enum Error { diff --git a/gix-transport/src/lib.rs b/gix-transport/src/lib.rs index 5e44b00ae63..e29026a252e 100644 --- a/gix-transport/src/lib.rs +++ b/gix-transport/src/lib.rs @@ -1,5 +1,6 @@ -//! An implementation of the `git` transport layer, abstracting over all of its [versions][Protocol], providing -//! [`connect()`] to establish a connection given a repository URL. +//! An implementation of the `git` transport layer, abstracting over all of its [versions][Protocol]. +//! +//! Use `client::blocking_io::connect()` or `client::async_io::connect()` to establish a connection. //! //! All git transports are supported, including `ssh`, `git`, `http` and `https`, as well as local repository paths. //! ## Feature Flags @@ -83,10 +84,3 @@ pub use traits::IsSpuriousError; /// pub mod client; - -#[doc(inline)] -#[cfg(any(feature = "blocking-client", all(feature = "async-client", feature = "async-std")))] -pub use client::connect; - -#[cfg(all(feature = "async-client", feature = "blocking-client"))] -compile_error!("Cannot set both 'blocking-client' and 'async-client' features as they are mutually exclusive"); diff --git a/gix-transport/tests/client/capabilities.rs b/gix-transport/tests/client/capabilities.rs index cd7d9405e00..6b6d356e8f4 100644 --- a/gix-transport/tests/client/capabilities.rs +++ b/gix-transport/tests/client/capabilities.rs @@ -3,6 +3,10 @@ use bstr::ByteSlice; use gix_packetline::async_io::{encode, StreamingPeekableIter}; #[cfg(all(feature = "blocking-client", not(feature = "async-client")))] use gix_packetline::blocking_io::{encode, StreamingPeekableIter}; +#[cfg(all(feature = "async-client", not(feature = "blocking-client")))] +use gix_transport::client::capabilities::async_recv::Handshake; +#[cfg(all(feature = "blocking-client", not(feature = "async-client")))] +use gix_transport::client::capabilities::blocking_recv::Handshake; use gix_transport::client::Capabilities; #[test] @@ -59,7 +63,7 @@ async fn from_lines_with_version_detection_v0() -> crate::Result { let mut buf = Vec::::new(); encode::flush_to_write(&mut buf).await?; let mut stream = StreamingPeekableIter::new(buf.as_slice(), &[gix_packetline::PacketLineRef::Flush], false); - let caps = Capabilities::from_lines_with_version_detection(&mut stream) + let caps = Handshake::from_lines_with_version_detection(&mut stream) .await .expect("we can parse V0 as very special case, useful for testing stateful connections in other crates") .capabilities; diff --git a/gix-transport/tests/client/git.rs b/gix-transport/tests/client/git.rs index cc8c5c40af7..9afee70146c 100644 --- a/gix-transport/tests/client/git.rs +++ b/gix-transport/tests/client/git.rs @@ -9,9 +9,19 @@ use bstr::ByteSlice; #[cfg(feature = "async-client")] use futures_lite::{AsyncBufReadExt, AsyncWriteExt, StreamExt}; use gix_packetline::read::ProgressAction; +#[cfg(feature = "async-client")] +use gix_transport::client::{ + async_io::{Transport, TransportV2Ext}, + git::async_io::Connection, +}; +#[cfg(feature = "blocking-client")] +use gix_transport::client::{ + blocking_io::{Transport, TransportV2Ext}, + git::blocking_io::Connection, +}; use gix_transport::{ client, - client::{git, Transport, TransportV2Ext, TransportWithoutIO}, + client::{git, TransportWithoutIO}, Protocol, Service, }; @@ -21,7 +31,7 @@ use crate::fixture_bytes; async fn handshake_v1_and_request() -> crate::Result { let mut out = Vec::new(); let server_response = fixture_bytes("v1/clone.response"); - let c = git::Connection::new( + let c = Connection::new( server_response.as_slice(), &mut out, Protocol::V1, @@ -153,7 +163,7 @@ async fn handshake_v1_and_request() -> crate::Result { async fn push_v1_simulated() -> crate::Result { let mut out = Vec::new(); let server_response = fixture_bytes("v1/push.response"); - let mut c = git::Connection::new( + let mut c = Connection::new( server_response.as_slice(), &mut out, Protocol::V1, @@ -219,7 +229,7 @@ async fn push_v1_simulated() -> crate::Result { async fn handshake_v1_process_mode() -> crate::Result { let mut out = Vec::new(); let server_response = fixture_bytes("v1/clone.response"); - let mut c = git::Connection::new( + let mut c = Connection::new( server_response.as_slice(), &mut out, Protocol::V1, @@ -242,7 +252,7 @@ async fn handshake_v1_process_mode() -> crate::Result { async fn handshake_v2_downgrade_to_v1() -> crate::Result { let mut out = Vec::new(); let input = fixture_bytes("v1/clone.response"); - let mut c = git::Connection::new( + let mut c = Connection::new( input.as_slice(), &mut out, Protocol::V2, @@ -289,7 +299,7 @@ async fn handshake_v2_and_request() -> crate::Result { async fn handshake_v2_and_request_inner() -> crate::Result { let mut out = Vec::new(); let input = fixture_bytes("v2/clone.response"); - let mut c = git::Connection::new( + let mut c = Connection::new( input.as_slice(), &mut out, Protocol::V2, diff --git a/gix/Cargo.toml b/gix/Cargo.toml index 2fdce440009..3549c848983 100644 --- a/gix/Cargo.toml +++ b/gix/Cargo.toml @@ -145,7 +145,7 @@ blob-diff = ["gix-diff/blob", "attributes"] merge = ["tree-editor", "blob-diff", "dep:gix-merge", "attributes"] ## Add blame command similar to `git blame`. -blame = ["dep:gix-blame"] +blame = ["dep:gix-blame", "blob-diff"] ## Make it possible to turn a tree into a stream of bytes, which can be decoded to entries and turned into various other formats. worktree-stream = ["gix-worktree-stream", "attributes"] diff --git a/gix/src/clone/fetch/mod.rs b/gix/src/clone/fetch/mod.rs index 34641a89fb1..46453d960e9 100644 --- a/gix/src/clone/fetch/mod.rs +++ b/gix/src/clone/fetch/mod.rs @@ -2,6 +2,7 @@ use crate::{ bstr::{BString, ByteSlice}, clone::PrepareFetch, }; +use gix_ref::Category; /// The error returned by [`PrepareFetch::fetch_only()`]. #[derive(Debug, thiserror::Error)] @@ -47,6 +48,10 @@ pub enum Error { }, #[error(transparent)] CommitterOrFallback(#[from] crate::config::time::Error), + #[error(transparent)] + RefMap(#[from] crate::remote::ref_map::Error), + #[error(transparent)] + ReferenceName(#[from] gix_validate::reference::name::Error), } /// Modification @@ -101,14 +106,81 @@ impl PrepareFetch { }; let mut remote = repo.remote_at(self.url.clone())?; + + // For shallow clones without custom configuration, we'll use a single-branch refspec + // to match git's behavior (matching git's single-branch behavior for shallow clones). + let use_single_branch_for_shallow = self.shallow != remote::fetch::Shallow::NoChange + && remote.fetch_specs.is_empty() + && self.fetch_options.extra_refspecs.is_empty(); + + let target_ref = if use_single_branch_for_shallow { + // Determine target branch from user-specified ref_name or default branch + if let Some(ref_name) = &self.ref_name { + Some(Category::LocalBranch.to_full_name(ref_name.as_ref().as_bstr())?) + } else { + // For shallow clones without a specified ref, we need to determine the default branch. + // We'll connect to get HEAD information. For Protocol V2, we need to explicitly list refs. + let mut connection = remote.connect(remote::Direction::Fetch).await?; + + // Perform handshake and try to get HEAD from it (works for Protocol V1) + let _ = connection.ref_map_by_ref(&mut progress, Default::default()).await?; + + let target = if let Some(handshake) = &connection.handshake { + // Protocol V1: refs are in handshake + handshake.refs.as_ref().and_then(|refs| { + refs.iter().find_map(|r| match r { + gix_protocol::handshake::Ref::Symbolic { + full_ref_name, target, .. + } if full_ref_name == "HEAD" => gix_ref::FullName::try_from(target).ok(), + _ => None, + }) + }) + } else { + None + }; + + // For Protocol V2 or if we couldn't determine HEAD, use the configured default branch + let fallback_branch = target + .or_else(|| { + repo.config + .resolved + .string(crate::config::tree::Init::DEFAULT_BRANCH) + .and_then(|name| Category::LocalBranch.to_full_name(name.as_bstr()).ok()) + }) + .unwrap_or_else(|| gix_ref::FullName::try_from("refs/heads/main").expect("known to be valid")); + + // Drop the connection explicitly to release the borrow on remote + drop(connection); + + Some(fallback_branch) + } + } else { + None + }; + + // Set up refspec based on whether we're doing a single-branch shallow clone, + // which requires a single ref to match Git unless it's overridden. if remote.fetch_specs.is_empty() { - remote = remote - .with_refspecs( - Some(format!("+refs/heads/*:refs/remotes/{remote_name}/*").as_str()), - remote::Direction::Fetch, - ) - .expect("valid static spec"); + if let Some(target_ref) = &target_ref { + // Single-branch refspec for shallow clones + let short_name = target_ref.shorten(); + remote = remote + .with_refspecs( + Some(format!("+{target_ref}:refs/remotes/{remote_name}/{short_name}").as_str()), + remote::Direction::Fetch, + ) + .expect("valid refspec"); + } else { + // Wildcard refspec for non-shallow clones or when target couldn't be determined + remote = remote + .with_refspecs( + Some(format!("+refs/heads/*:refs/remotes/{remote_name}/*").as_str()), + remote::Direction::Fetch, + ) + .expect("valid static spec"); + } } + let mut clone_fetch_tags = None; if let Some(f) = self.configure_remote.as_mut() { remote = f(remote).map_err(Error::RemoteConfiguration)?; @@ -133,6 +205,7 @@ impl PrepareFetch { .expect("valid") .to_owned(); let pending_pack: remote::fetch::Prepare<'_, '_, _> = { + // For shallow clones, we already connected once, so we need to connect again let mut connection = remote.connect(remote::Direction::Fetch).await?; if let Some(f) = self.configure_connection.as_mut() { f(&mut connection).map_err(Error::RemoteConnection)?; diff --git a/gix/src/clone/mod.rs b/gix/src/clone/mod.rs index 23903943724..c3d50ad3d9b 100644 --- a/gix/src/clone/mod.rs +++ b/gix/src/clone/mod.rs @@ -1,12 +1,17 @@ #![allow(clippy::result_large_err)] use crate::{bstr::BString, remote}; +#[cfg(feature = "async-network-client")] +use gix_transport::client::async_io::Transport; +#[cfg(feature = "blocking-network-client")] +use gix_transport::client::blocking_io::Transport; + type ConfigureRemoteFn = Box) -> Result, Box>>; #[cfg(any(feature = "async-network-client", feature = "blocking-network-client"))] type ConfigureConnectionFn = Box< dyn FnMut( - &mut remote::Connection<'_, '_, Box>, + &mut remote::Connection<'_, '_, Box>, ) -> Result<(), Box>, >; @@ -137,6 +142,7 @@ pub struct PrepareCheckout { // once async and clone are a thing. #[cfg(any(feature = "async-network-client", feature = "blocking-network-client"))] mod access_feat { + use super::Transport; use crate::clone::PrepareFetch; /// Builder @@ -148,7 +154,7 @@ mod access_feat { pub fn configure_connection( mut self, f: impl FnMut( - &mut crate::remote::Connection<'_, '_, Box>, + &mut crate::remote::Connection<'_, '_, Box>, ) -> Result<(), Box> + 'static, ) -> Self { diff --git a/gix/src/config/tree/sections/http.rs b/gix/src/config/tree/sections/http.rs index 4fc733564de..af5e80472c2 100644 --- a/gix/src/config/tree/sections/http.rs +++ b/gix/src/config/tree/sections/http.rs @@ -125,10 +125,10 @@ mod key_impls { value: std::borrow::Cow<'_, crate::bstr::BStr>, boolean: impl FnOnce() -> Result, gix_config::value::Error>, ) -> Result< - crate::protocol::transport::client::http::options::FollowRedirects, + crate::protocol::transport::client::blocking_io::http::options::FollowRedirects, crate::config::key::GenericErrorWithValue, > { - use crate::{bstr::ByteSlice, protocol::transport::client::http::options::FollowRedirects}; + use crate::{bstr::ByteSlice, protocol::transport::client::blocking_io::http::options::FollowRedirects}; Ok(if value.as_ref().as_bytes() == b"initial" { FollowRedirects::Initial } else if let Some(value) = boolean().map_err(|err| { @@ -172,10 +172,10 @@ mod key_impls { &'static self, value: std::borrow::Cow<'_, crate::bstr::BStr>, ) -> Result< - gix_protocol::transport::client::http::options::HttpVersion, + gix_protocol::transport::client::blocking_io::http::options::HttpVersion, crate::config::key::GenericErrorWithValue, > { - use gix_protocol::transport::client::http::options::HttpVersion; + use gix_protocol::transport::client::blocking_io::http::options::HttpVersion; use crate::bstr::ByteSlice; Ok(match value.as_ref().as_bytes() { @@ -200,10 +200,10 @@ mod key_impls { &'static self, value: std::borrow::Cow<'_, crate::bstr::BStr>, ) -> Result< - gix_protocol::transport::client::http::options::ProxyAuthMethod, + gix_protocol::transport::client::blocking_io::http::options::ProxyAuthMethod, crate::config::key::GenericErrorWithValue, > { - use gix_protocol::transport::client::http::options::ProxyAuthMethod; + use gix_protocol::transport::client::blocking_io::http::options::ProxyAuthMethod; use crate::bstr::ByteSlice; Ok(match value.as_ref().as_bytes() { @@ -230,9 +230,11 @@ mod key_impls { pub fn try_into_ssl_version( &'static self, value: std::borrow::Cow<'_, crate::bstr::BStr>, - ) -> Result - { - use gix_protocol::transport::client::http::options::SslVersion::*; + ) -> Result< + gix_protocol::transport::client::blocking_io::http::options::SslVersion, + crate::config::ssl_version::Error, + > { + use gix_protocol::transport::client::blocking_io::http::options::SslVersion::*; use crate::bstr::ByteSlice; Ok(match value.as_ref().as_bytes() { diff --git a/gix/src/config/tree/sections/init.rs b/gix/src/config/tree/sections/init.rs index de42d3b6257..453a906a168 100644 --- a/gix/src/config/tree/sections/init.rs +++ b/gix/src/config/tree/sections/init.rs @@ -5,6 +5,7 @@ use crate::{ impl Init { /// The `init.defaultBranch` key. + // TODO: review its usage for cases where this key is not set - sometimes it's 'master', sometimes it's 'main'. pub const DEFAULT_BRANCH: keys::Any = keys::Any::new("defaultBranch", &config::Tree::INIT) .with_deviation("If not set, we use `main` instead of `master`"); } diff --git a/gix/src/config/tree/sections/ssh.rs b/gix/src/config/tree/sections/ssh.rs index 600ee663b59..72bc0105444 100644 --- a/gix/src/config/tree/sections/ssh.rs +++ b/gix/src/config/tree/sections/ssh.rs @@ -23,9 +23,11 @@ mod variant { pub fn try_into_variant( &'static self, value: Cow<'_, BStr>, - ) -> Result, config::key::GenericErrorWithValue> - { - use gix_protocol::transport::client::ssh::ProgramKind; + ) -> Result< + Option, + config::key::GenericErrorWithValue, + > { + use gix_protocol::transport::client::blocking_io::ssh::ProgramKind; use crate::bstr::ByteSlice; Ok(Some(match value.as_ref().as_bytes() { diff --git a/gix/src/remote/connect.rs b/gix/src/remote/connect.rs index 1c70928c5ec..c020d573ee0 100644 --- a/gix/src/remote/connect.rs +++ b/gix/src/remote/connect.rs @@ -2,11 +2,15 @@ use std::borrow::Cow; -use gix_protocol::transport::client::Transport; +#[cfg(feature = "async-network-client")] +use gix_transport::client::async_io::{connect, Transport}; +#[cfg(feature = "blocking-network-client")] +use gix_transport::client::blocking_io::{connect, Transport}; use crate::{config::tree::Protocol, remote::Connection, Remote}; mod error { + use super::connect; use crate::{bstr::BString, config, remote}; /// The error returned by [connect()][crate::Remote::connect()]. @@ -24,7 +28,7 @@ mod error { #[error("Protocol {scheme:?} of url {url:?} is denied per configuration")] ProtocolDenied { url: BString, scheme: gix_url::Scheme }, #[error(transparent)] - Connect(#[from] gix_protocol::transport::client::connect::Error), + Connect(#[from] connect::Error), #[error("The {} url was missing - don't know where to establish a connection to", direction.as_str())] MissingUrl { direction: remote::Direction }, #[error("The given protocol version was invalid. Choose between 1 and 2")] @@ -86,9 +90,9 @@ impl<'repo> Remote<'repo> { let (url, version) = self.sanitized_url_and_version(direction)?; #[cfg(feature = "blocking-network-client")] let scheme_is_ssh = url.scheme == gix_url::Scheme::Ssh; - let transport = gix_protocol::transport::connect( + let transport = connect::connect( url, - gix_protocol::transport::client::connect::Options { + connect::Options { version, #[cfg(feature = "blocking-network-client")] ssh: scheme_is_ssh diff --git a/gix/src/remote/connection/access.rs b/gix/src/remote/connection/access.rs index c97fdde912e..b8779dfe1d7 100644 --- a/gix/src/remote/connection/access.rs +++ b/gix/src/remote/connection/access.rs @@ -2,11 +2,15 @@ use crate::{ remote::{connection::AuthenticateFn, Connection}, Remote, }; +#[cfg(feature = "async-network-client")] +use gix_transport::client::async_io::Transport; +#[cfg(feature = "blocking-network-client")] +use gix_transport::client::blocking_io::Transport; /// Builder impl<'a, T> Connection<'a, '_, T> where - T: gix_transport::client::Transport, + T: Transport, { /// Set a custom credentials callback to provide credentials if the remotes require authentication. /// @@ -43,7 +47,7 @@ where /// Mutation impl<'a, T> Connection<'a, '_, T> where - T: gix_transport::client::Transport, + T: Transport, { /// Like [`with_credentials()`](Self::with_credentials()), but without consuming the connection. pub fn set_credentials( @@ -64,7 +68,7 @@ where /// Access impl<'repo, T> Connection<'_, 'repo, T> where - T: gix_transport::client::Transport, + T: Transport, { /// A utility to return a function that will use this repository's configuration to obtain credentials, similar to /// what `git credential` is doing. diff --git a/gix/src/remote/connection/fetch/mod.rs b/gix/src/remote/connection/fetch/mod.rs index 2e0014b4bea..6efb6be4f73 100644 --- a/gix/src/remote/connection/fetch/mod.rs +++ b/gix/src/remote/connection/fetch/mod.rs @@ -1,4 +1,7 @@ -use gix_protocol::transport::client::Transport; +#[cfg(feature = "async-network-client")] +use gix_transport::client::async_io::Transport; +#[cfg(feature = "blocking-network-client")] +use gix_transport::client::blocking_io::Transport; use crate::{ bstr::BString, @@ -73,7 +76,7 @@ pub struct Outcome { /// The result of the initial mapping of references, the prerequisite for any fetch. pub ref_map: RefMap, /// The outcome of the handshake with the server. - pub handshake: gix_protocol::handshake::Outcome, + pub handshake: gix_protocol::Handshake, /// The status of the operation to indicate what happened. pub status: Status, } diff --git a/gix/src/remote/connection/fetch/receive_pack.rs b/gix/src/remote/connection/fetch/receive_pack.rs index 579afebdcf3..b7b4e467f67 100644 --- a/gix/src/remote/connection/fetch/receive_pack.rs +++ b/gix/src/remote/connection/fetch/receive_pack.rs @@ -1,10 +1,11 @@ use std::{ops::DerefMut, path::PathBuf, sync::atomic::AtomicBool}; use gix_odb::store::RefreshMode; -use gix_protocol::{ - fetch::{negotiate, Arguments}, - transport::client::Transport, -}; +use gix_protocol::fetch::{negotiate, Arguments}; +#[cfg(feature = "async-network-client")] +use gix_transport::client::async_io::Transport; +#[cfg(feature = "blocking-network-client")] +use gix_transport::client::blocking_io::Transport; use crate::{ config::{ diff --git a/gix/src/remote/connection/fetch/update_refs/mod.rs b/gix/src/remote/connection/fetch/update_refs/mod.rs index e33cfe2151a..64200f98805 100644 --- a/gix/src/remote/connection/fetch/update_refs/mod.rs +++ b/gix/src/remote/connection/fetch/update_refs/mod.rs @@ -76,6 +76,7 @@ pub(crate) fn update( let mut updates = Vec::new(); let mut edit_indices_to_validate = Vec::new(); + let mut checked_out_branches = worktree_branches(repo)?; let implicit_tag_refspec = fetch_tags .to_refspec() .filter(|_| matches!(fetch_tags, crate::remote::fetch::Tags::Included)); @@ -110,7 +111,6 @@ pub(crate) fn update( continue; } } - let mut checked_out_branches = worktree_branches(repo)?; let (mode, edit_index, type_change) = match local { Some(name) => { let (mode, reflog_message, name, previous_value) = match repo.try_find_reference(name)? { diff --git a/gix/src/remote/connection/mod.rs b/gix/src/remote/connection/mod.rs index fc3a37f5be1..c5322c519f0 100644 --- a/gix/src/remote/connection/mod.rs +++ b/gix/src/remote/connection/mod.rs @@ -1,3 +1,8 @@ +#[cfg(feature = "async-network-client")] +use gix_transport::client::async_io::Transport; +#[cfg(feature = "blocking-network-client")] +use gix_transport::client::blocking_io::Transport; + use crate::Remote; /// A function that performs a given credential action, trying to obtain credentials for an operation that needs it. @@ -9,13 +14,13 @@ pub type AuthenticateFn<'a> = Box /// much like a remote procedure call. pub struct Connection<'a, 'repo, T> where - T: gix_transport::client::Transport, + T: Transport, { pub(crate) remote: &'a Remote<'repo>, pub(crate) authenticate: Option>, pub(crate) transport_options: Option>, pub(crate) transport: gix_protocol::SendFlushOnDrop, - pub(crate) handshake: Option, + pub(crate) handshake: Option, pub(crate) trace: bool, } diff --git a/gix/src/remote/connection/ref_map.rs b/gix/src/remote/connection/ref_map.rs index 47b46b6cbdf..e149705dc6b 100644 --- a/gix/src/remote/connection/ref_map.rs +++ b/gix/src/remote/connection/ref_map.rs @@ -1,5 +1,8 @@ use gix_features::progress::Progress; -use gix_protocol::transport::client::Transport; +#[cfg(feature = "async-network-client")] +use gix_transport::client::async_io::Transport; +#[cfg(feature = "blocking-network-client")] +use gix_transport::client::blocking_io::Transport; use crate::{ bstr::BString, @@ -89,7 +92,7 @@ where mut self, progress: impl Progress, options: Options, - ) -> Result<(fetch::RefMap, gix_protocol::handshake::Outcome), Error> { + ) -> Result<(fetch::RefMap, gix_protocol::Handshake), Error> { let refmap = self.ref_map_by_ref(progress, options).await?; let handshake = self .handshake @@ -140,29 +143,30 @@ where if let Some(config) = self.transport_options.as_ref() { self.transport.inner.configure(&**config)?; } - let mut handshake = gix_protocol::fetch::handshake( + let mut handshake = gix_protocol::handshake( &mut self.transport.inner, + gix_transport::Service::UploadPack, authenticate, handshake_parameters, &mut progress, ) .await?; - let refmap = gix_protocol::fetch::RefMap::new( - progress, - &self.remote.fetch_specs, - gix_protocol::fetch::Context { - handshake: &mut handshake, - transport: &mut self.transport.inner, - user_agent: self.remote.repo.config.user_agent_tuple(), - trace_packetlines: self.trace, - }, - gix_protocol::fetch::refmap::init::Options { + + let context = fetch::refmap::init::Context { + fetch_refspecs: self.remote.fetch_specs.clone(), + extra_refspecs, + }; + let ref_map = handshake + .fetch_or_extract_refmap( + progress, + &mut self.transport.inner, + self.remote.repo.config.user_agent_tuple(), + self.trace, prefix_from_spec_as_filter_on_remote, - extra_refspecs, - }, - ) - .await?; + context, + ) + .await?; self.handshake = Some(handshake); - Ok(refmap) + Ok(ref_map) } } diff --git a/gix/src/repository/blame.rs b/gix/src/repository/blame.rs index 84c0902f7de..18064ce5288 100644 --- a/gix/src/repository/blame.rs +++ b/gix/src/repository/blame.rs @@ -13,11 +13,30 @@ impl Repository { &self, file_path: &BStr, suspect: impl Into, - options: gix_blame::Options, + options: blame_file::Options, ) -> Result { - let cache: Option = self.commit_graph_if_enabled()?; + let cache = self.commit_graph_if_enabled()?; let mut resource_cache = self.diff_resource_cache_for_tree_diff()?; + let blame_file::Options { + diff_algorithm, + ranges, + since, + rewrites, + } = options; + let diff_algorithm = match diff_algorithm { + Some(diff_algorithm) => diff_algorithm, + None => self.diff_algorithm()?, + }; + + let options = gix_blame::Options { + diff_algorithm, + ranges, + since, + rewrites, + debug_track_path: false, + }; + let outcome = gix_blame::file( &self.objects, suspect.into(), diff --git a/gix/src/repository/config/mod.rs b/gix/src/repository/config/mod.rs index 93e91b7ae8a..15d109530ed 100644 --- a/gix/src/repository/config/mod.rs +++ b/gix/src/repository/config/mod.rs @@ -61,7 +61,8 @@ impl crate::Repository { #[cfg(feature = "blocking-network-client")] pub fn ssh_connect_options( &self, - ) -> Result { + ) -> Result + { use crate::config::{ cache::util::ApplyLeniency, tree::{gitoxide, Core, Ssh}, @@ -77,7 +78,7 @@ impl crate::Repository { config.string_filter(gitoxide::Ssh::COMMAND_WITHOUT_SHELL_FALLBACK, &mut trusted) }) .map(|cmd| gix_path::from_bstr(cmd).into_owned().into()); - let opts = gix_protocol::transport::client::ssh::connect::Options { + let opts = gix_protocol::transport::client::blocking_io::ssh::connect::Options { disallow_shell: fallback_active, command: ssh_command, kind: config diff --git a/gix/src/repository/config/transport.rs b/gix/src/repository/config/transport.rs index c7bb4592af3..3d1c7f70c65 100644 --- a/gix/src/repository/config/transport.rs +++ b/gix/src/repository/config/transport.rs @@ -48,9 +48,9 @@ impl crate::Repository { sync::{Arc, Mutex}, }; - use gix_transport::client::{ - http, - http::options::{ProxyAuthMethod, SslVersion, SslVersionRangeInclusive}, + use gix_transport::client::blocking_io::http::{ + self, + options::{ProxyAuthMethod, SslVersion, SslVersionRangeInclusive}, }; use crate::{ @@ -430,7 +430,8 @@ impl crate::Repository { .transpose() .with_leniency(lenient) .map_err(config::transport::http::Error::from)?; - let backend = gix_protocol::transport::client::http::curl::Options { schannel_check_revoke }; + let backend = + gix_protocol::transport::client::blocking_io::http::curl::Options { schannel_check_revoke }; opts.backend = Some(Arc::new(Mutex::new(backend)) as Arc>); } diff --git a/gix/src/repository/identity.rs b/gix/src/repository/identity.rs index 7837e9f2d65..ea9f3a506c4 100644 --- a/gix/src/repository/identity.rs +++ b/gix/src/repository/identity.rs @@ -140,7 +140,7 @@ impl Personas { .and_then(|date| gix_date::parse(date, Some(SystemTime::now())).ok()) }) .or_else(|| Some(gix_date::Time::now_local_or_utc())) - .map(|time| time.format(gix_date::time::Format::Raw)) + .map(|time| time.format_or_unix(gix_date::time::Format::Raw)) }; let fallback = ( diff --git a/gix/src/repository/location.rs b/gix/src/repository/location.rs index 6f38b3c958d..db490687e05 100644 --- a/gix/src/repository/location.rs +++ b/gix/src/repository/location.rs @@ -58,6 +58,36 @@ impl crate::Repository { self.work_tree.as_deref() } + /// Forcefully set the given `workdir` to be the worktree of this repository, *in memory*, + /// no matter if it had one or not, or unset it with `None`. + /// Return the previous working directory if one existed. + /// + /// Fail if the `workdir`, if not `None`, isn't accessible or isn't a directory. + /// No change is performed on error. + /// + /// ### About Worktrees + /// + /// * When setting a main worktree to a linked worktree directory, this repository instance + /// will still claim that it is the [main worktree](crate::Worktree::is_main()) as that depends + /// on the `git_dir`, not the worktree dir. + /// * When setting a linked worktree to a main worktree directory, this repository instance + /// will still claim that it is *not* a [main worktree](crate::Worktree::is_main()) as that depends + /// on the `git_dir`, not the worktree dir. + #[doc(alias = "git2")] + pub fn set_workdir(&mut self, workdir: impl Into>) -> Result, std::io::Error> { + let workdir = workdir.into(); + Ok(match workdir { + None => self.work_tree.take(), + Some(new_workdir) => { + _ = std::fs::read_dir(&new_workdir)?; + + let old = self.work_tree.take(); + self.work_tree = Some(new_workdir); + old + } + }) + } + /// Return the work tree containing all checked out files, if there is one. pub fn workdir(&self) -> Option<&std::path::Path> { self.work_tree.as_deref() diff --git a/gix/src/repository/mod.rs b/gix/src/repository/mod.rs index ae1b55b64d0..e6c2ab66ac0 100644 --- a/gix/src/repository/mod.rs +++ b/gix/src/repository/mod.rs @@ -19,6 +19,7 @@ pub enum Kind { #[cfg(any(feature = "attributes", feature = "excludes"))] pub mod attributes; +/// #[cfg(feature = "blame")] mod blame; mod cache; @@ -96,6 +97,19 @@ mod new_commit_as { /// #[cfg(feature = "blame")] pub mod blame_file { + /// Options to be passed to [Repository::blame_file()](crate::Repository::blame_file()). + #[derive(Default, Debug, Clone)] + pub struct Options { + /// The algorithm to use for diffing. If `None`, `diff.algorithm` will be used. + pub diff_algorithm: Option, + /// The ranges to blame in the file. + pub ranges: gix_blame::BlameRanges, + /// Don't consider commits before the given date. + pub since: Option, + /// Determine if rename tracking should be performed, and how. + pub rewrites: Option, + } + /// The error returned by [Repository::blame_file()](crate::Repository::blame_file()). #[derive(Debug, thiserror::Error)] #[allow(missing_docs)] @@ -103,6 +117,8 @@ pub mod blame_file { #[error(transparent)] CommitGraphIfEnabled(#[from] super::commit_graph_if_enabled::Error), #[error(transparent)] + DiffAlgorithm(#[from] crate::config::diff::algorithm::Error), + #[error(transparent)] DiffResourceCache(#[from] super::diff_resource_cache::Error), #[error(transparent)] Blame(#[from] gix_blame::Error), diff --git a/gix/src/repository/worktree.rs b/gix/src/repository/worktree.rs index e3475d2cc45..5dc76603494 100644 --- a/gix/src/repository/worktree.rs +++ b/gix/src/repository/worktree.rs @@ -1,3 +1,4 @@ +use crate::bstr::BStr; use crate::{worktree, Worktree}; /// Interact with individual worktrees and their information. @@ -20,16 +21,22 @@ impl crate::Repository { for entry in iter { let entry = entry?; let worktree_git_dir = entry.path(); - if worktree_git_dir.join("gitdir").is_file() { - res.push(worktree::Proxy { - parent: self, - git_dir: worktree_git_dir, - }); - } + res.extend(worktree::Proxy::new_if_gitdir_file_exists(self, worktree_git_dir)); } res.sort_by(|a, b| a.git_dir.cmp(&b.git_dir)); Ok(res) } + + /// Return the worktree that [is identified](Worktree::id) by the given `id`, if it exists at + /// `.git/worktrees/` and its `gitdir` file exists. + /// Return `None` otherwise. + pub fn worktree_proxy_by_id<'a>(&self, id: impl Into<&'a BStr>) -> Option> { + worktree::Proxy::new_if_gitdir_file_exists( + self, + self.common_dir().join("worktrees").join(gix_path::from_bstr(id.into())), + ) + } + /// Return the repository owning the main worktree, typically from a linked worktree. /// /// Note that it might be the one that is currently open if this repository doesn't point to a linked worktree. @@ -41,7 +48,7 @@ impl crate::Repository { /// Return the currently set worktree if there is one, acting as platform providing a validated worktree base path. /// - /// Note that there would be `None` if this repository is `bare` and the parent [`Repository`][crate::Repository] was instantiated without + /// Note that there would be `None` if this repository is `bare` and the parent [`Repository`](crate::Repository) was instantiated without /// registered worktree in the current working dir, even if no `.git` file or directory exists. /// It's merely based on configuration, see [Worktree::dot_git_exists()] for a way to perform more validation. pub fn worktree(&self) -> Option> { @@ -50,7 +57,7 @@ impl crate::Repository { /// Return true if this repository is bare, and has no main work tree. /// - /// This is not to be confused with the [`worktree()`][crate::Repository::worktree()] worktree, which may exists if this instance + /// This is not to be confused with the [`worktree()`](crate::Repository::worktree()) method, which may exist if this instance /// was opened in a worktree that was created separately. pub fn is_bare(&self) -> bool { self.config.is_bare && self.workdir().is_none() diff --git a/gix/src/revision/spec/parse/error.rs b/gix/src/revision/spec/parse/error.rs index eb361238995..7e043386110 100644 --- a/gix/src/revision/spec/parse/error.rs +++ b/gix/src/revision/spec/parse/error.rs @@ -44,7 +44,7 @@ impl std::fmt::Display for CandidateInfo { "commit {} {title:?}", gix_date::parse_header(date) .unwrap_or_default() - .format(gix_date::time::format::SHORT) + .format_or_unix(gix_date::time::format::SHORT) ) } } diff --git a/gix/src/worktree/proxy.rs b/gix/src/worktree/proxy.rs index e4b6672774c..b6f1b423e00 100644 --- a/gix/src/worktree/proxy.rs +++ b/gix/src/worktree/proxy.rs @@ -31,6 +31,15 @@ impl<'repo> Proxy<'repo> { git_dir: git_dir.into(), } } + + pub(crate) fn new_if_gitdir_file_exists(parent: &'repo Repository, git_dir: impl Into) -> Option { + let git_dir = git_dir.into(); + if git_dir.join("gitdir").is_file() { + Some(Proxy::new(parent, git_dir)) + } else { + None + } + } } impl Proxy<'_> { diff --git a/gix/tests/gix/clone.rs b/gix/tests/gix/clone.rs index 573a8e3ddfe..c843645b571 100644 --- a/gix/tests/gix/clone.rs +++ b/gix/tests/gix/clone.rs @@ -4,6 +4,10 @@ use crate::{remote, util::restricted}; mod blocking_io { use std::{borrow::Cow, path::Path, sync::atomic::AtomicBool}; + use crate::{ + remote, + util::{hex_to_id, restricted}, + }; use gix::{ bstr::BString, config::tree::{Clone, Core, Init, Key}, @@ -14,11 +18,7 @@ mod blocking_io { }; use gix_object::bstr::ByteSlice; use gix_ref::TargetRef; - - use crate::{ - remote, - util::{hex_to_id, restricted}, - }; + use gix_refspec::parse::Operation; #[test] fn fetch_shallow_no_checkout_then_unshallow() -> crate::Result { @@ -83,6 +83,40 @@ mod blocking_io { Ok(()) } + #[test] + fn shallow_clone_uses_single_branch_refspec() -> crate::Result { + let tmp = gix_testtools::tempfile::TempDir::new()?; + let (repo, _out) = gix::prepare_clone_bare(remote::repo("base").path(), tmp.path())? + .with_shallow(Shallow::DepthAtRemote(1.try_into()?)) + .fetch_only(gix::progress::Discard, &std::sync::atomic::AtomicBool::default())?; + + assert!(repo.is_shallow(), "repository should be shallow"); + + // Verify that only a single-branch refspec was configured + let remote = repo.find_remote("origin")?; + let refspecs: Vec<_> = remote + .refspecs(Direction::Fetch) + .iter() + .map(|spec| spec.to_ref().to_bstring()) + .collect(); + + assert_eq!(refspecs.len(), 1, "shallow clone should have only one fetch refspec"); + + // The refspec should be for a single branch (main), not a wildcard + let refspec_str = refspecs[0].to_str().expect("valid utf8"); + assert_eq!( + refspec_str, + if cfg!(windows) { + "+refs/heads/master:refs/remotes/origin/master" + } else { + "+refs/heads/main:refs/remotes/origin/main" + }, + "shallow clone refspec should not use wildcard and should be the main branch: {refspec_str}" + ); + + Ok(()) + } + #[test] fn from_shallow_prohibited_with_option() -> crate::Result { let tmp = gix_testtools::tempfile::TempDir::new()?; @@ -203,7 +237,16 @@ mod blocking_io { fn from_non_shallow_by_deepen_exclude_then_deepen_to_unshallow() -> crate::Result { let tmp = gix_testtools::tempfile::TempDir::new()?; let excluded_leaf_refs = ["g", "h", "j"]; + let (repo, _change) = gix::prepare_clone_bare(remote::repo("base").path(), tmp.path())? + .with_fetch_options(gix::remote::ref_map::Options { + extra_refspecs: vec![gix::refspec::parse( + "refs/heads/*:refs/remotes/origin/*".into(), + Operation::Fetch, + )? + .into()], + ..Default::default() + }) .with_shallow(Shallow::Exclude { remote_refs: excluded_leaf_refs .into_iter() diff --git a/gix/tests/gix/config/tree.rs b/gix/tests/gix/config/tree.rs index 22190235f94..e757ecd0fcb 100644 --- a/gix/tests/gix/config/tree.rs +++ b/gix/tests/gix/config/tree.rs @@ -122,7 +122,7 @@ mod ssh { #[cfg(feature = "blocking-network-client")] fn variant() -> crate::Result { use gix::config::tree::Ssh; - use gix_protocol::transport::client::ssh::ProgramKind; + use gix_protocol::transport::client::blocking_io::ssh::ProgramKind; use crate::config::tree::bcow; for (actual, expected) in [ @@ -931,7 +931,7 @@ mod http { #[test] fn follow_redirects() -> crate::Result { - use gix_transport::client::http::options::FollowRedirects; + use gix_transport::client::blocking_io::http::options::FollowRedirects; assert_eq!( Http::FOLLOW_REDIRECTS.try_into_follow_redirects(bcow("initial"), || unreachable!("no call"))?, FollowRedirects::Initial @@ -989,7 +989,7 @@ mod http { #[test] fn http_version() -> crate::Result { - use gix_transport::client::http::options::HttpVersion; + use gix_transport::client::blocking_io::http::options::HttpVersion; for (actual, expected) in [("HTTP/1.1", HttpVersion::V1_1), ("HTTP/2", HttpVersion::V2)] { assert_eq!(Http::VERSION.try_into_http_version(bcow(actual))?, expected); @@ -1009,7 +1009,7 @@ mod http { #[test] fn ssl_version() -> crate::Result { - use gix_transport::client::http::options::SslVersion::*; + use gix_transport::client::blocking_io::http::options::SslVersion::*; for (actual, expected) in [ ("default", Default), @@ -1039,7 +1039,7 @@ mod http { #[test] fn proxy_auth_method() -> crate::Result { - use gix_transport::client::http::options::ProxyAuthMethod::*; + use gix_transport::client::blocking_io::http::options::ProxyAuthMethod::*; for (actual, expected) in [ ("anyauth", AnyAuth), ("basic", Basic), diff --git a/gix/tests/gix/repository/blame.rs b/gix/tests/gix/repository/blame.rs index eb3f0d1ac0c..aa5106f0d32 100644 --- a/gix/tests/gix/repository/blame.rs +++ b/gix/tests/gix/repository/blame.rs @@ -17,8 +17,8 @@ fn simple() -> crate::Result { fn with_options() -> crate::Result { let repo = crate::named_repo("make_blame_repo.sh")?; - let options = gix::blame::Options { - range: gix::blame::BlameRanges::from_range(1..=2), + let options = gix::repository::blame_file::Options { + ranges: gix::blame::BlameRanges::from_one_based_inclusive_range(1..=2)?, ..Default::default() }; diff --git a/gix/tests/gix/repository/config/mod.rs b/gix/tests/gix/repository/config/mod.rs index 35cbb3d1476..fff507d5155 100644 --- a/gix/tests/gix/repository/config/mod.rs +++ b/gix/tests/gix/repository/config/mod.rs @@ -27,7 +27,10 @@ mod ssh_options { let repo = repo("ssh-all-options"); let opts = repo.ssh_connect_options()?; assert_eq!(opts.command.as_deref(), Some(OsStr::new("ssh -VVV"))); - assert_eq!(opts.kind, Some(gix::protocol::transport::client::ssh::ProgramKind::Ssh)); + assert_eq!( + opts.kind, + Some(gix::protocol::transport::client::blocking_io::ssh::ProgramKind::Ssh) + ); assert!(!opts.disallow_shell, "we can use the shell by default"); Ok(()) } @@ -39,7 +42,7 @@ mod ssh_options { assert_eq!(opts.command.as_deref(), Some(OsStr::new("ssh --fallback"))); assert_eq!( opts.kind, - Some(gix::protocol::transport::client::ssh::ProgramKind::Putty) + Some(gix::protocol::transport::client::blocking_io::ssh::ProgramKind::Putty) ); assert!( opts.disallow_shell, diff --git a/gix/tests/gix/repository/config/transport_options.rs b/gix/tests/gix/repository/config/transport_options.rs index b411a19b776..faf9ed82d0c 100644 --- a/gix/tests/gix/repository/config/transport_options.rs +++ b/gix/tests/gix/repository/config/transport_options.rs @@ -3,7 +3,7 @@ feature = "blocking-http-transport-curl" ))] mod http { - use gix_transport::client::http::options::{ + use gix_transport::client::blocking_io::http::options::{ FollowRedirects, HttpVersion, ProxyAuthMethod, SslVersion, SslVersionRangeInclusive, }; @@ -13,12 +13,12 @@ mod http { repo: &gix::Repository, remote_name: Option<&str>, url: &str, - ) -> gix_transport::client::http::Options { + ) -> gix_transport::client::blocking_io::http::Options { let opts = repo .transport_options(url, remote_name.map(Into::into)) .expect("valid configuration") .expect("configuration available for http"); - opts.downcast_ref::() + opts.downcast_ref::() .expect("http options have been created") .to_owned() } @@ -26,7 +26,7 @@ mod http { #[test] fn remote_overrides() { let repo = repo("http-remote-override"); - let gix_transport::client::http::Options { + let gix_transport::client::blocking_io::http::Options { proxy, proxy_auth_method, follow_redirects, @@ -41,7 +41,7 @@ mod http { #[test] fn simple_configuration() { let repo = repo("http-config"); - let gix_transport::client::http::Options { + let gix_transport::client::blocking_io::http::Options { extra_headers, follow_redirects, low_speed_limit_bytes_per_second, @@ -91,7 +91,7 @@ mod http { .as_ref() .map(|b| b.lock().expect("not poisoned")) .expect("backend is set for curl due to specific options"); - match backend.downcast_ref::() { + match backend.downcast_ref::() { Some(opts) => { assert_eq!(opts.schannel_check_revoke, Some(true)); } diff --git a/gix/tests/gix/repository/open.rs b/gix/tests/gix/repository/open.rs index 48eea8ad821..6541a584836 100644 --- a/gix/tests/gix/repository/open.rs +++ b/gix/tests/gix/repository/open.rs @@ -154,7 +154,7 @@ fn non_bare_non_git_repo_without_worktree() -> crate::Result { #[test] fn none_bare_repo_without_index() -> crate::Result { - let repo = named_subrepo_opts( + let mut repo = named_subrepo_opts( "make_basic_repo.sh", "non-bare-repo-without-index", gix::open::Options::isolated(), @@ -175,6 +175,28 @@ fn none_bare_repo_without_index() -> crate::Result { .is_ok(), "this is a minimal path" ); + + let old = repo.set_workdir(None).expect("should never fail"); + assert_eq!( + old.as_ref().and_then(|wd| wd.file_name()?.to_str()), + Some("non-bare-repo-without-index") + ); + assert!(repo.workdir().is_none(), "the workdir was unset"); + assert!(repo.worktree().is_none(), "the worktree was unset"); + assert!( + !repo.is_bare(), + "this is based on `core.bare`, not on the lack of worktree" + ); + + assert_eq!( + repo.set_workdir(old.clone()).expect("does not fail as it exists"), + None, + "nothing was set before" + ); + assert_eq!(repo.workdir(), old.as_deref()); + + let worktree = repo.worktree().expect("should be present after setting"); + assert!(worktree.is_main(), "it's still the main worktree"); Ok(()) } diff --git a/gix/tests/gix/repository/worktree.rs b/gix/tests/gix/repository/worktree.rs index ad233eff627..773d875f2f1 100644 --- a/gix/tests/gix/repository/worktree.rs +++ b/gix/tests/gix/repository/worktree.rs @@ -254,6 +254,43 @@ fn from_nonbare_parent_repo() { run_assertions(repo, false /* bare */); } +#[test] +fn from_nonbare_parent_repo_set_workdir() -> gix_testtools::Result { + if gix_testtools::should_skip_as_git_version_is_smaller_than(2, 31, 0) { + return Ok(()); + } + + let dir = gix_testtools::scripted_fixture_read_only("make_worktree_repo.sh").unwrap(); + let mut repo = gix::open(dir.join("repo")).unwrap(); + + assert!(repo.worktree().is_some_and(|wt| wt.is_main()), "we have main worktree"); + + let worktrees = repo.worktrees()?; + assert_eq!(worktrees.len(), 6); + + let linked_wt_dir = worktrees.first().unwrap().base().expect("this linked worktree exists"); + repo.set_workdir(linked_wt_dir).expect("works as the dir exists"); + + assert!( + repo.worktree().is_some_and(|wt| wt.is_main()), + "it's still the main worktree as that depends on the git_dir" + ); + + let mut wt_repo = repo.worktrees()?.first().unwrap().clone().into_repo()?; + assert!( + wt_repo.worktree().is_some_and(|wt| !wt.is_main()), + "linked worktrees are never main" + ); + + wt_repo.set_workdir(Some(repo.workdir().unwrap().to_owned()))?; + assert!( + wt_repo.worktree().is_some_and(|wt| !wt.is_main()), + "it's still the linked worktree as that depends on the git_dir" + ); + + Ok(()) +} + fn run_assertions(main_repo: gix::Repository, should_be_bare: bool) { assert_eq!(main_repo.is_bare(), should_be_bare); let mut baseline = Baseline::collect( @@ -313,8 +350,14 @@ fn run_assertions(main_repo: gix::Repository, should_be_bare: bool) { "in our case prunable repos have no worktree base" ); + assert_eq!( + main_repo.worktree_proxy_by_id(actual.id()).expect("exists").git_dir(), + actual.git_dir(), + "we can basically get the same proxy by its ID explicitly" + ); + let repo = if base.is_dir() { - let repo = actual.into_repo().unwrap(); + let repo = actual.clone().into_repo().unwrap(); assert_eq!( &gix::open(base).unwrap(), &repo, @@ -329,7 +372,7 @@ fn run_assertions(main_repo: gix::Repository, should_be_bare: bool) { ), "missing bases are detected" ); - actual.into_repo_with_possibly_inaccessible_worktree().unwrap() + actual.clone().into_repo_with_possibly_inaccessible_worktree().unwrap() }; let worktree = repo.worktree().expect("linked worktrees have at least a base path"); assert!(!worktree.is_main()); @@ -341,5 +384,19 @@ fn run_assertions(main_repo: gix::Repository, should_be_bare: bool) { main_repo, "main repo from worktree repo is the actual main repo" ); + + let proxy_by_id = repo + .worktree_proxy_by_id(actual.id()) + .expect("can get the proxy from a linked repo as well"); + assert_ne!( + proxy_by_id.git_dir(), + actual.git_dir(), + "The git directories might not look the same…" + ); + assert_eq!( + gix_path::realpath(proxy_by_id.git_dir()).ok(), + gix_path::realpath(actual.git_dir()).ok(), + "…but they are the same effectively" + ); } } diff --git a/justfile b/justfile index 81ab31ac8d9..eeb7a76229e 100755 --- a/justfile +++ b/justfile @@ -45,10 +45,10 @@ check: cargo check --workspace cargo check --no-default-features --features small cargo check -p gix-packetline --all-features 2>/dev/null + cargo check -p gix-transport --all-features 2>/dev/null # assure compile error occurs ! cargo check --features lean-async 2>/dev/null ! cargo check -p gitoxide-core --all-features 2>/dev/null - ! cargo check -p gix-transport --all-features 2>/dev/null ! cargo check -p gix-protocol --all-features 2>/dev/null # warning happens if nothing found, no exit code :/ cargo --color=never tree -p gix --no-default-features -e normal -i imara-diff \ diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index 5bc1aacdf70..1ff9821d904 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -658,7 +658,7 @@ pub fn main() -> Result<()> { } Subcommands::ConfigTree => show_progress(), Subcommands::Credential(cmd) => core::repository::credential( - repository(Mode::StrictWithGitInstallConfig)?, + repository(Mode::StrictWithGitInstallConfig).ok(), match cmd { credential::Subcommands::Fill => gix::credentials::program::main::Action::Get, credential::Subcommands::Approve => gix::credentials::program::main::Action::Store, @@ -1636,7 +1636,7 @@ pub fn main() -> Result<()> { &file, gix::blame::Options { diff_algorithm, - range: gix::blame::BlameRanges::from_ranges(ranges), + ranges: gix::blame::BlameRanges::from_one_based_inclusive_ranges(ranges)?, since, rewrites: Some(gix::diff::Rewrites::default()), debug_track_path: false, diff --git a/tests/it/src/args.rs b/tests/it/src/args.rs index 78f1873d64b..cad0be3bb04 100644 --- a/tests/it/src/args.rs +++ b/tests/it/src/args.rs @@ -123,6 +123,34 @@ pub enum Subcommands { #[clap(value_parser = AsPathSpec)] patterns: Vec, }, + /// Take a slider file generated with the help of [diff-slider-tools] and turn it into a series + /// of baseline diffs to be used in [slider-rs]. + /// + /// See [make-diff-for-sliders-repo] for details. + /// + /// [diff-slider-tools]: https://github.com/mhagger/diff-slider-tools + /// [slider-rs]: gix-diff/tests/diff/blob/slider.rs + /// [make-diff-for-sliders-repo]: gix-diff/tests/fixtures/make_diff_for_sliders_repo.sh + CreateDiffCases { + /// Don't really copy anything. + #[clap(long, short = 'n')] + dry_run: bool, + /// The `.sliders` file that contains a list of sliders. + #[clap(long)] + sliders_file: PathBuf, + /// The git root to extract the diff-related parts from. + #[clap(long)] + worktree_dir: PathBuf, + /// The directory into which to copy the files. + #[clap(long)] + destination_dir: PathBuf, + /// The number of sliders to generate test cases for. + #[clap(long, default_value_t = 10)] + count: usize, + /// The directory to place assets in. + #[clap(long)] + asset_dir: Option, + }, /// Check for executable bits that disagree with shebangs. /// /// This checks committed and staged files, but not anything unstaged, to find shell scripts diff --git a/tests/it/src/commands/blame_copy_royal.rs b/tests/it/src/commands/blame_copy_royal.rs index 6eaa2e51e09..b1714573386 100644 --- a/tests/it/src/commands/blame_copy_royal.rs +++ b/tests/it/src/commands/blame_copy_royal.rs @@ -37,7 +37,7 @@ pub(super) mod function { let options = gix::blame::Options { diff_algorithm, - range: gix::blame::BlameRanges::default(), + ranges: gix::blame::BlameRanges::default(), since: None, rewrites: Some(gix::diff::Rewrites::default()), debug_track_path: true, diff --git a/tests/it/src/commands/create_diff_cases.rs b/tests/it/src/commands/create_diff_cases.rs new file mode 100644 index 00000000000..e5b4d6079c1 --- /dev/null +++ b/tests/it/src/commands/create_diff_cases.rs @@ -0,0 +1,109 @@ +pub(super) mod function { + use anyhow::Context; + use std::{ + collections::HashSet, + path::{Path, PathBuf}, + }; + + use gix::{ + bstr::{BString, ByteSlice}, + objs::FindExt, + }; + + pub fn create_diff_cases( + dry_run: bool, + sliders_file: PathBuf, + worktree_dir: &Path, + destination_dir: PathBuf, + count: usize, + asset_dir: Option, + ) -> anyhow::Result<()> { + let prefix = if dry_run { "WOULD" } else { "Will" }; + let sliders = std::fs::read_to_string(&sliders_file)?; + + eprintln!( + "Read '{}' which has {} lines", + sliders_file.display(), + sliders.lines().count() + ); + + let sliders: HashSet<_> = sliders + .lines() + .take(count) + .map(|line| { + let parts: Vec<_> = line.split_ascii_whitespace().collect(); + + match parts[..] { + [before, after, ..] => (before, after), + _ => unreachable!(), + } + }) + .collect(); + + let repo = gix::open_opts(worktree_dir, gix::open::Options::isolated())?; + + let asset_dir = asset_dir.unwrap_or("assets".into()); + let assets = destination_dir.join(asset_dir.to_os_str()?); + + eprintln!("{prefix} create directory '{assets}'", assets = assets.display()); + if !dry_run { + std::fs::create_dir_all(&assets)?; + } + + let mut buf = Vec::new(); + let script_name = "make_diff_for_sliders_repo.sh"; + + let mut blocks: Vec = vec![format!( + r#"#!/usr/bin/env bash +set -eu -o pipefail + +ROOT="$(cd "$(dirname "${{BASH_SOURCE[0]}}")" && pwd)" + +mkdir -p {asset_dir} +"# + )]; + + for (before, after) in sliders.iter().copied() { + let revspec = repo.rev_parse(before)?; + let old_blob_id = revspec + .single() + .context(format!("rev-spec '{before}' must resolve to a single object"))?; + + let revspec = repo.rev_parse(after)?; + let new_blob_id = revspec + .single() + .context(format!("rev-spec '{after}' must resolve to a single object"))?; + + let dst_old_blob = assets.join(format!("{old_blob_id}.blob")); + let dst_new_blob = assets.join(format!("{new_blob_id}.blob")); + if !dry_run { + let old_blob = repo.objects.find_blob(&old_blob_id, &mut buf)?.data; + std::fs::write(dst_old_blob, old_blob)?; + + let new_blob = repo.objects.find_blob(&new_blob_id, &mut buf)?.data; + std::fs::write(dst_new_blob, new_blob)?; + } + + blocks.push(format!( + r#"git -c diff.algorithm=myers diff --no-index "$ROOT/{asset_dir}/{old_blob_id}.blob" "$ROOT/{asset_dir}/{new_blob_id}.blob" > {old_blob_id}-{new_blob_id}.myers.baseline || true +git -c diff.algorithm=histogram diff --no-index "$ROOT/{asset_dir}/{old_blob_id}.blob" "$ROOT/{asset_dir}/{new_blob_id}.blob" > {old_blob_id}-{new_blob_id}.histogram.baseline || true +cp "$ROOT/{asset_dir}/{old_blob_id}.blob" assets/ +cp "$ROOT/{asset_dir}/{new_blob_id}.blob" assets/ +"# + )); + } + + let script_file = destination_dir.join(script_name); + eprintln!( + "{prefix} write script file at '{script_file}'", + script_file = script_file.display() + ); + + if !dry_run { + let script = blocks.join("\n"); + std::fs::write(script_file, script)?; + } + + Ok(()) + } +} diff --git a/tests/it/src/commands/mod.rs b/tests/it/src/commands/mod.rs index 6bec01671df..ed3abfd0cfc 100644 --- a/tests/it/src/commands/mod.rs +++ b/tests/it/src/commands/mod.rs @@ -7,6 +7,9 @@ pub use copy_royal::function::copy_royal; pub mod git_to_sh; pub use git_to_sh::function::git_to_sh; +pub mod create_diff_cases; +pub use create_diff_cases::function::create_diff_cases; + pub mod check_mode; pub use check_mode::function::check_mode; diff --git a/tests/it/src/main.rs b/tests/it/src/main.rs index df22a7e8c6f..c0b3ff22741 100644 --- a/tests/it/src/main.rs +++ b/tests/it/src/main.rs @@ -46,6 +46,14 @@ fn main() -> anyhow::Result<()> { destination_dir, patterns, } => commands::copy_royal(dry_run, &worktree_root, destination_dir, patterns), + Subcommands::CreateDiffCases { + dry_run, + sliders_file, + worktree_dir, + destination_dir, + count, + asset_dir, + } => commands::create_diff_cases(dry_run, sliders_file, &worktree_dir, destination_dir, count, asset_dir), Subcommands::CheckMode {} => commands::check_mode(), Subcommands::Env {} => commands::env(), } diff --git a/tests/journey/ein.sh b/tests/journey/ein.sh index 89bffe1eed5..c971f8ac9e6 100644 --- a/tests/journey/ein.sh +++ b/tests/journey/ein.sh @@ -1,5 +1,10 @@ # Must be sourced into the main journey test + +function remove-thread-id { + sed -E 's/ \([0-9]+\)//' +} + if test "$kind" = "max" || test "$kind" = "max-pure"; then title "Porcelain ${kind}" ( @@ -8,21 +13,24 @@ title "Porcelain ${kind}" (with "the --quiet option set" it "fails as expected" && { WITH_SNAPSHOT="$snapshot/expected-failure" \ - expect_run_sh 101 "$exe -q panic" + SNAPSHOT_FILTER=remove-thread-id \ + expect_run_sh 0 "$exe -q panic" } ) (with "NO --quiet option set" it "fails as expected" && { WITH_SNAPSHOT="$snapshot/expected-failure-in-thread" \ - expect_run_sh 101 "$exe panic" + SNAPSHOT_FILTER=remove-thread-id \ + expect_run_sh 0 "$exe panic" } ) (not_on_ci # due to different TTY settings, the output differs, it's OK for now (with "progress option set" it "fails as expected" && { WITH_SNAPSHOT="$snapshot/expected-failure-in-thread-with-progress" \ - expect_run_sh 101 "$exe --progress panic" + SNAPSHOT_FILTER=remove-thread-id \ + expect_run_sh 0 "$exe --progress panic" } ) )