Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions gitoxide-core/src/pack/receive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ where
shallow_file: "no shallow file required as we reject it to keep it simple".into(),
shallow: &Default::default(),
tags: Default::default(),
filter: None,
reject_shallow_remote: true,
},
)
Expand Down
3 changes: 3 additions & 0 deletions gitoxide-core/src/repository/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub struct Options {
pub no_tags: bool,
pub shallow: gix::remote::fetch::Shallow,
pub ref_name: Option<gix::refs::PartialName>,
pub filter: Option<gix::remote::fetch::ObjectFilter>,
}

pub const PROGRESS_RANGE: std::ops::RangeInclusive<u8> = 1..=3;
Expand Down Expand Up @@ -34,6 +35,7 @@ pub(crate) mod function {
no_tags,
ref_name,
shallow,
filter,
}: Options,
) -> anyhow::Result<()>
where
Expand Down Expand Up @@ -76,6 +78,7 @@ pub(crate) mod function {
prepare = prepare.configure_remote(|r| Ok(r.with_fetch_tags(gix::remote::fetch::Tags::None)));
}
let (mut checkout, fetch_outcome) = prepare
.with_filter(filter)
.with_shallow(shallow)
.with_ref_name(ref_name.as_ref())?
.fetch_then_checkout(&mut progress, &gix::interrupt::IS_INTERRUPTED)?;
Expand Down
21 changes: 17 additions & 4 deletions gitoxide-core/src/repository/fetch.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use gix::bstr::BString;
use gix::{bstr::BString, hash::ObjectId};

use crate::OutputFormat;

Expand Down Expand Up @@ -29,7 +29,7 @@ pub(crate) mod function {
std_shapes::shapes::{Arrow, Element, ShapeKind},
};

use super::Options;
use super::{ObjectId, Options};
use crate::OutputFormat;

pub fn fetch<P>(
Expand Down Expand Up @@ -57,15 +57,28 @@ pub(crate) mod function {
}

let mut remote = crate::repository::remote::by_name_or_url(&repo, remote.as_deref())?;
if !ref_specs.is_empty() {
remote.replace_refspecs(ref_specs.iter(), gix::remote::Direction::Fetch)?;
let mut wants = Vec::new();
let mut fetch_refspecs = Vec::new();
for spec in ref_specs {
if spec.len() == repo.object_hash().len_in_hex() {
if let Ok(oid) = ObjectId::from_hex(spec.as_ref()) {
wants.push(oid);
Comment on lines +63 to +65
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Disambiguate hex-only fetch args before treating them as OIDs

The new argument split treats any token whose length matches the object hash and parses as hex as an object ID, so that token is never passed through refspec handling. This breaks valid hex-only ref names (for example a branch/tag named with 40 hex chars), because fetch will issue a raw want instead of a refspec and can fail on servers that don't allow arbitrary SHA wants. Please avoid auto-converting ambiguous tokens unless the user explicitly asks for object-ID mode or after confirming they are not valid remote refs.

Useful? React with 👍 / 👎.

continue;
}
}
fetch_refspecs.push(spec);
}
Comment on lines +62 to +70
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces an ambiguity: any fetch argument that is exactly the hash length and hex-decodable is treated as an object ID, even if the user intended it to be a refspec/refname (e.g., an all-hex branch name of that length). If that ambiguity is acceptable, it should be documented in the CLI help; otherwise consider adding an explicit syntax/flag for object-id wants (or only treating it as an OID when prefixed, e.g. oid:<hex>).

Copilot uses AI. Check for mistakes.

if !fetch_refspecs.is_empty() {
remote.replace_refspecs(fetch_refspecs.iter(), gix::remote::Direction::Fetch)?;
remote = remote.with_fetch_tags(gix::remote::fetch::Tags::None);
}
let res: gix::remote::fetch::Outcome = remote
.connect(gix::remote::Direction::Fetch)?
.prepare_fetch(&mut progress, Default::default())?
.with_dry_run(dry_run)
.with_shallow(shallow)
.with_additional_wants(wants)
.receive(&mut progress, &gix::interrupt::IS_INTERRUPTED)?;

if handshake_info {
Expand Down
10 changes: 10 additions & 0 deletions gix-protocol/src/fetch/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub async fn fetch<P, T, E>(
shallow_file,
shallow,
tags,
filter,
reject_shallow_remote,
}: Options<'_>,
) -> Result<Option<Outcome>, Error>
Expand All @@ -74,6 +75,15 @@ where
crate::fetch::Response::check_required_features(protocol_version, &fetch_features)?;
let sideband_all = fetch_features.iter().any(|(n, _)| *n == "sideband-all");
let mut arguments = Arguments::new(protocol_version, fetch_features, trace_packetlines);
if let Some(filter) = filter {
if !arguments.can_use_filter() {
return Err(Error::MissingServerFeature {
feature: "filter",
description: "Partial clone filters require server support configured on the remote server",
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error description doesn’t clearly state the concrete failure observed (“remote didn’t advertise filter capability”). Consider rewording to explicitly mention that the server does not advertise (or permit) the filter capability, and that partial clone filtering can’t be used against that remote unless enabled server-side. This will better match the PR intent of providing a “clear error”.

Suggested change
description: "Partial clone filters require server support configured on the remote server",
description: "Remote does not advertise the `filter` capability; partial clone filtering cannot be used with this remote unless it is enabled server-side",

Copilot uses AI. Check for mistakes.
});
}
arguments.filter(filter);
}
if matches!(tags, Tags::Included) {
if !arguments.can_use_include_tag() {
return Err(Error::MissingServerFeature {
Expand Down
2 changes: 2 additions & 0 deletions gix-protocol/src/fetch/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ pub struct Options<'a> {
pub shallow: &'a Shallow,
/// Describe how to handle tags when fetching.
pub tags: Tags,
/// If set, request that the remote filters the object set according to `filter`.
pub filter: Option<&'a str>,
/// If `true`, if we fetch from a remote that only offers shallow clones, the operation will fail with an error
/// instead of writing the shallow boundary to the shallow file.
pub reject_shallow_remote: bool,
Expand Down
6 changes: 6 additions & 0 deletions gix/src/clone/access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ impl PrepareFetch {
self
}

/// Ask the remote to omit objects based on `filter`.
pub fn with_filter(mut self, filter: impl Into<Option<crate::remote::fetch::ObjectFilter>>) -> Self {
self.filter = filter.into();
self
}

/// Apply the given configuration `values` right before readying the actual fetch from the remote.
/// The configuration is marked with [source API](gix_config::Source::Api), and will not be written back, it's
/// retained only in memory.
Expand Down
17 changes: 16 additions & 1 deletion gix/src/clone/fetch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ pub enum Error {
RemoteConfiguration(#[source] Box<dyn std::error::Error + Send + Sync>),
#[error("Custom configuration of connection to use when cloning failed")]
RemoteConnection(#[source] Box<dyn std::error::Error + Send + Sync>),
#[error("Remote name {remote_name:?} is not valid UTF-8")]
RemoteNameNotUtf8 {
source: crate::bstr::Utf8Error,
remote_name: crate::bstr::BString,
},
#[error(transparent)]
ConfigSectionHeader(#[from] gix_config::parse::section::header::Error),
#[error(transparent)]
RemoteName(#[from] crate::config::remote::symbolic_name::Error),
#[error(transparent)]
Expand Down Expand Up @@ -203,7 +210,12 @@ impl PrepareFetch {
clone_fetch_tags = remote::fetch::Tags::All.into();
}

let config = util::write_remote_to_local_config_file(&mut remote, remote_name.clone())?;
let filter_spec_to_save = self
.filter
.as_ref()
.map(remote::fetch::ObjectFilter::to_argument_string);
let config =
util::write_remote_to_local_config_file(&mut remote, remote_name.clone(), filter_spec_to_save.as_deref())?;

// Now we are free to apply remote configuration we don't want to be written to disk.
if let Some(fetch_tags) = clone_fetch_tags {
Expand Down Expand Up @@ -283,6 +295,7 @@ impl PrepareFetch {
b
};
let outcome = pending_pack
.with_filter(self.filter.clone())
.with_write_packed_refs_only(true)
.with_reflog_message(RefLogMessage::Override {
message: reflog_message.clone(),
Expand Down Expand Up @@ -326,3 +339,5 @@ impl PrepareFetch {
}

mod util;

pub use util::write_remote_to_local_config_file;
29 changes: 28 additions & 1 deletion gix/src/clone/fetch/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,40 @@ enum WriteMode {
Append,
}

/// Persist the provided remote into the repository's local config, optionally adding partial clone settings.
#[allow(clippy::result_large_err)]
pub fn write_remote_to_local_config_file(
remote: &mut crate::Remote<'_>,
remote_name: BString,
filter: Option<&str>,
) -> Result<gix_config::File<'static>, Error> {
use gix_config::parse::section::ValueName;

let mut config = gix_config::File::new(local_config_meta(remote.repo));
remote.save_as_to(remote_name, &mut config)?;
remote.save_as_to(remote_name.clone(), &mut config)?;

if let Some(filter_spec) = filter {
let subsection = remote_name.to_str().map_err(|err| Error::RemoteNameNotUtf8 {
remote_name: remote_name.clone(),
source: err,
})?;
let mut remote_section = config.section_mut_or_create_new("remote", Some(subsection.into()))?;

while remote_section.remove("partialclonefilter").is_some() {}
while remote_section.remove("promisor").is_some() {}
Comment on lines +38 to +39
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: Create gix-config remove all.


let partial_clone_filter = ValueName::try_from("partialclonefilter").expect("known to be valid");
remote_section.push(partial_clone_filter, Some(filter_spec.into()));

let promisor = ValueName::try_from("promisor").expect("known to be valid");
remote_section.push(promisor, Some("true".into()));

let mut extensions_section = config.section_mut_or_create_new("extensions", None)?;
while extensions_section.remove("partialClone").is_some() {}

let partial_clone = ValueName::try_from("partialClone").expect("known to be valid");
Comment on lines +48 to +50
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extensions.partialclone is written/removed as partialClone (camelCase) here, but the rest of the code/tests refer to extensions.partialclone (lowercase). If config key lookup/removal is case-sensitive (or if existing repos already have lowercase keys), this can cause duplicated entries and/or make the persisted setting not discoverable by code expecting extensions.partialclone. Use the canonical lowercase key name consistently for remove/push (i.e., partialclone).

Suggested change
while extensions_section.remove("partialClone").is_some() {}
let partial_clone = ValueName::try_from("partialClone").expect("known to be valid");
while extensions_section.remove("partialclone").is_some() {}
let partial_clone = ValueName::try_from("partialclone").expect("known to be valid");

Copilot uses AI. Check for mistakes.
extensions_section.push(partial_clone, Some(remote_name.as_ref()));
}

write_to_local_config(&config, WriteMode::Append)?;
Ok(config)
Expand Down
4 changes: 4 additions & 0 deletions gix/src/clone/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ pub struct PrepareFetch {
/// How to handle shallow clones
#[cfg_attr(not(feature = "blocking-network-client"), allow(dead_code))]
shallow: remote::fetch::Shallow,
/// Optional object filter to request from the remote.
#[cfg_attr(not(feature = "blocking-network-client"), allow(dead_code))]
filter: Option<remote::fetch::ObjectFilter>,
/// The name of the reference to fetch. If `None`, the reference pointed to by `HEAD` will be checked out.
#[cfg_attr(not(feature = "blocking-network-client"), allow(dead_code))]
ref_name: Option<gix_ref::PartialName>,
Expand Down Expand Up @@ -121,6 +124,7 @@ impl PrepareFetch {
#[cfg(any(feature = "async-network-client", feature = "blocking-network-client"))]
configure_connection: None,
shallow: remote::fetch::Shallow::NoChange,
filter: None,
ref_name: None,
})
}
Expand Down
23 changes: 23 additions & 0 deletions gix/src/remote/connection/fetch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::{
},
Progress,
};
use gix_hash::ObjectId;

mod error;
pub use error::Error;
Expand Down Expand Up @@ -153,6 +154,8 @@ where
reflog_message: None,
write_packed_refs: WritePackedRefs::Never,
shallow: Default::default(),
filter: None,
additional_wants: Vec::new(),
})
}
}
Expand Down Expand Up @@ -184,6 +187,8 @@ where
reflog_message: Option<RefLogMessage>,
write_packed_refs: WritePackedRefs,
shallow: remote::fetch::Shallow,
filter: Option<remote::fetch::ObjectFilter>,
additional_wants: Vec<ObjectId>,
}

/// Builder
Expand Down Expand Up @@ -225,4 +230,22 @@ where
self.shallow = shallow;
self
}

/// Ask the server to apply `filter` when sending objects.
pub fn with_filter(mut self, filter: Option<remote::fetch::ObjectFilter>) -> Self {
self.filter = filter;
self
}

/// Request that the server also sends the objects identified by the given object ids.
///
/// Objects already present locally will be ignored during negotiation.
pub fn with_additional_wants(mut self, wants: impl IntoIterator<Item = ObjectId>) -> Self {
for want in wants {
if !self.additional_wants.contains(&want) {
self.additional_wants.push(want);
}
}
self
}
Comment on lines +240 to +250
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The doc comment says objects already present locally are ignored during negotiation, but this builder method only deduplicates within additional_wants and does not check whether the object exists locally. Either adjust the documentation to reflect actual behavior (“deduplicated only”) or implement the “already present locally” check where the object store is available.

Copilot uses AI. Check for mistakes.
}
Loading
Loading