Skip to content

Commit f194cfc

Browse files
ByronByron
committed
feat: use real pathspecs where it was supported before.
1 parent 77da014 commit f194cfc

File tree

9 files changed

+102
-91
lines changed

9 files changed

+102
-91
lines changed

Diff for: ‎gitoxide-core/src/query/engine/command.rs

+11-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::collections::HashMap;
22

3-
use anyhow::Context;
3+
use anyhow::{bail, Context};
44
use gix::{bstr::ByteSlice, prelude::ObjectIdExt, Progress};
55
use rusqlite::{params, OptionalExtension};
66

@@ -18,10 +18,16 @@ impl query::Engine {
1818
) -> anyhow::Result<()> {
1919
match cmd {
2020
Command::TracePath { mut spec } => {
21-
if let Some(prefix) = self.repo.prefix() {
22-
spec.apply_prefix(&prefix?);
23-
};
24-
let relpath = spec.items().next().expect("spec has at least one item");
21+
let is_excluded = spec.is_excluded();
22+
let relpath = spec
23+
.normalize(
24+
self.repo.prefix().transpose()?.unwrap_or_default().as_ref(),
25+
self.repo.work_dir().unwrap_or_else(|| self.repo.git_dir()),
26+
)?
27+
.path();
28+
if relpath.is_empty() || is_excluded {
29+
bail!("Invalid pathspec {spec} - path must not be empty, not be excluded, and wildcards are taken literally")
30+
}
2531
let file_id: usize = self
2632
.con
2733
.query_row(

Diff for: ‎gitoxide-core/src/query/engine/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
pub enum Command {
22
TracePath {
33
/// The repo-relative path to the file to trace
4-
spec: gix::path::Spec,
4+
spec: gix::pathspec::Pattern,
55
},
66
}
77

Diff for: ‎gitoxide-core/src/repository/attributes/query.rs

+23-20
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub(crate) mod function {
1818

1919
pub fn query(
2020
repo: gix::Repository,
21-
pathspecs: impl Iterator<Item = gix::path::Spec>,
21+
pathspecs: impl Iterator<Item = gix::pathspec::Pattern>,
2222
mut out: impl io::Write,
2323
mut err: impl io::Write,
2424
Options { format, statistics }: Options,
@@ -28,28 +28,31 @@ pub(crate) mod function {
2828
}
2929

3030
let mut cache = attributes_cache(&repo)?;
31-
let prefix = repo.prefix().expect("worktree - we have an index by now")?;
3231
let mut matches = cache.attribute_matches();
32+
// TODO(pathspec): The search is just used as a shortcut to normalization, but one day should be used for an actual search.
33+
let search = gix::pathspec::Search::from_specs(
34+
pathspecs,
35+
repo.prefix().transpose()?.as_deref(),
36+
repo.work_dir().unwrap_or_else(|| repo.git_dir()),
37+
)?;
3338

34-
for mut spec in pathspecs {
35-
for path in spec.apply_prefix(&prefix).items() {
36-
let is_dir = gix::path::from_bstr(path).metadata().ok().map(|m| m.is_dir());
37-
let entry = cache.at_entry(path, is_dir, |oid, buf| repo.objects.find_blob(oid, buf))?;
39+
for spec in search.into_patterns() {
40+
let is_dir = gix::path::from_bstr(spec.path()).metadata().ok().map(|m| m.is_dir());
41+
let entry = cache.at_entry(spec.path(), is_dir, |oid, buf| repo.objects.find_blob(oid, buf))?;
3842

39-
if !entry.matching_attributes(&mut matches) {
40-
continue;
41-
}
42-
for m in matches.iter() {
43-
writeln!(
44-
out,
45-
"{}:{}:{}\t{}\t{}",
46-
m.location.source.map(Path::to_string_lossy).unwrap_or_default(),
47-
m.location.sequence_number,
48-
m.pattern,
49-
path,
50-
m.assignment
51-
)?;
52-
}
43+
if !entry.matching_attributes(&mut matches) {
44+
continue;
45+
}
46+
for m in matches.iter() {
47+
writeln!(
48+
out,
49+
"{}:{}:{}\t{}\t{}",
50+
m.location.source.map(Path::to_string_lossy).unwrap_or_default(),
51+
m.location.sequence_number,
52+
m.pattern,
53+
spec.path(),
54+
m.assignment
55+
)?;
5356
}
5457
}
5558

Diff for: ‎gitoxide-core/src/repository/attributes/validate_baseline.rs

+17-22
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ pub(crate) mod function {
1818
};
1919

2020
use anyhow::{anyhow, bail};
21+
use gix::bstr::BString;
2122
use gix::{attrs::Assignment, odb::FindExt, Progress};
2223

2324
use crate::{
@@ -27,7 +28,7 @@ pub(crate) mod function {
2728

2829
pub fn validate_baseline(
2930
repo: gix::Repository,
30-
pathspecs: Option<impl Iterator<Item = gix::path::Spec> + Send + 'static>,
31+
paths: Option<impl Iterator<Item = BString> + Send + 'static>,
3132
mut progress: impl Progress + 'static,
3233
mut out: impl io::Write,
3334
mut err: impl io::Write,
@@ -51,26 +52,24 @@ pub(crate) mod function {
5152
ignore = false;
5253
}
5354
let mut num_entries = None;
54-
let pathspecs = pathspecs.map_or_else(
55+
let paths = paths.map_or_else(
5556
{
5657
let repo = repo.clone();
5758
let num_entries = &mut num_entries;
5859
move || -> anyhow::Result<_> {
5960
let index = repo.index_or_load_from_head()?.into_owned();
6061
let (entries, path_backing) = index.into_parts().0.into_entries();
6162
*num_entries = Some(entries.len());
62-
let iter = Box::new(entries.into_iter().map(move |e| {
63-
gix::path::Spec::from_bytes(e.path_in(&path_backing)).expect("each entry path is a valid spec")
64-
}));
65-
Ok(iter as Box<dyn Iterator<Item = gix::path::Spec> + Send + 'static>)
63+
let iter = Box::new(entries.into_iter().map(move |e| e.path_in(&path_backing).to_owned()));
64+
Ok(iter as Box<dyn Iterator<Item = BString> + Send + 'static>)
6665
}
6766
},
68-
|i| anyhow::Result::Ok(Box::new(i)),
67+
|paths| anyhow::Result::Ok(Box::new(paths)),
6968
)?;
7069

7170
let (tx_base, rx_base) = std::sync::mpsc::channel::<(String, Baseline)>();
7271
let feed_attrs = {
73-
let (tx, rx) = std::sync::mpsc::sync_channel::<gix::path::Spec>(1);
72+
let (tx, rx) = std::sync::mpsc::sync_channel::<BString>(100);
7473
std::thread::spawn({
7574
let path = repo.path().to_owned();
7675
let tx_base = tx_base.clone();
@@ -89,12 +88,10 @@ pub(crate) mod function {
8988
move || -> anyhow::Result<()> {
9089
progress.init(num_entries, gix::progress::count("paths"));
9190
let start = std::time::Instant::now();
92-
for spec in rx {
91+
for path in rx {
9392
progress.inc();
94-
for path in spec.items() {
95-
stdin.write_all(path.as_ref())?;
96-
stdin.write_all(b"\n")?;
97-
}
93+
stdin.write_all(&path)?;
94+
stdin.write_all(b"\n")?;
9895
}
9996
progress.show_throughput(start);
10097
Ok(())
@@ -123,7 +120,7 @@ pub(crate) mod function {
123120
})
124121
.transpose()?;
125122
let feed_excludes = ignore.then(|| {
126-
let (tx, rx) = std::sync::mpsc::sync_channel::<gix::path::Spec>(1);
123+
let (tx, rx) = std::sync::mpsc::sync_channel::<BString>(100);
127124
std::thread::spawn({
128125
let path = work_dir.expect("present if we are here");
129126
let tx_base = tx_base.clone();
@@ -142,12 +139,10 @@ pub(crate) mod function {
142139
move || -> anyhow::Result<()> {
143140
progress.init(num_entries, gix::progress::count("paths"));
144141
let start = std::time::Instant::now();
145-
for spec in rx {
142+
for path in rx {
146143
progress.inc();
147-
for path in spec.items() {
148-
stdin.write_all(path.as_ref())?;
149-
stdin.write_all(b"\n")?;
150-
}
144+
stdin.write_all(path.as_ref())?;
145+
stdin.write_all(b"\n")?;
151146
}
152147
progress.show_throughput(start);
153148
Ok(())
@@ -175,12 +170,12 @@ pub(crate) mod function {
175170
drop(tx_base);
176171

177172
std::thread::spawn(move || {
178-
for spec in pathspecs {
179-
if feed_attrs.send(spec.clone()).is_err() {
173+
for path in paths {
174+
if feed_attrs.send(path.clone()).is_err() {
180175
break;
181176
}
182177
if let Some(ch) = feed_excludes.as_ref() {
183-
if ch.send(spec).is_err() {
178+
if ch.send(path).is_err() {
184179
break;
185180
}
186181
}

Diff for: ‎gitoxide-core/src/repository/exclude.rs

+24-22
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ pub mod query {
2020

2121
pub fn query(
2222
repo: gix::Repository,
23-
pathspecs: impl Iterator<Item = gix::path::Spec>,
23+
pathspecs: impl Iterator<Item = gix::pathspec::Pattern>,
2424
mut out: impl io::Write,
2525
mut err: impl io::Write,
2626
query::Options {
@@ -41,28 +41,30 @@ pub fn query(
4141
Default::default(),
4242
)?;
4343

44-
let prefix = repo.prefix().expect("worktree - we have an index by now")?;
44+
// TODO(pathspec): actually use the search to find items. This looks like `gix` capabilities to put it all together.
45+
let search = gix::pathspec::Search::from_specs(
46+
pathspecs,
47+
repo.prefix().transpose()?.as_deref(),
48+
repo.work_dir().unwrap_or_else(|| repo.git_dir()),
49+
)?;
4550

46-
for mut spec in pathspecs {
47-
for path in spec.apply_prefix(&prefix).items() {
48-
// TODO: what about paths that end in /? Pathspec might handle it, it's definitely something git considers
49-
// even if the directory doesn't exist. Seems to work as long as these are kept in the spec.
50-
let is_dir = gix::path::from_bstr(path).metadata().ok().map(|m| m.is_dir());
51-
let entry = cache.at_entry(path, is_dir, |oid, buf| repo.objects.find_blob(oid, buf))?;
52-
let match_ = entry
53-
.matching_exclude_pattern()
54-
.and_then(|m| (show_ignore_patterns || !m.pattern.is_negative()).then_some(m));
55-
match match_ {
56-
Some(m) => writeln!(
57-
out,
58-
"{}:{}:{}\t{}",
59-
m.source.map(std::path::Path::to_string_lossy).unwrap_or_default(),
60-
m.sequence_number,
61-
m.pattern,
62-
path
63-
)?,
64-
None => writeln!(out, "::\t{path}")?,
65-
}
51+
for spec in search.into_patterns() {
52+
let path = spec.path();
53+
let is_dir = gix::path::from_bstr(path).metadata().ok().map(|m| m.is_dir());
54+
let entry = cache.at_entry(path, is_dir, |oid, buf| repo.objects.find_blob(oid, buf))?;
55+
let match_ = entry
56+
.matching_exclude_pattern()
57+
.and_then(|m| (show_ignore_patterns || !m.pattern.is_negative()).then_some(m));
58+
match match_ {
59+
Some(m) => writeln!(
60+
out,
61+
"{}:{}:{}\t{}",
62+
m.source.map(std::path::Path::to_string_lossy).unwrap_or_default(),
63+
m.sequence_number,
64+
m.pattern,
65+
path
66+
)?,
67+
None => writeln!(out, "::\t{path}")?,
6668
}
6769
}
6870

Diff for: ‎src/plumbing/main.rs

+13-16
Original file line numberDiff line numberDiff line change
@@ -1030,12 +1030,14 @@ pub fn main() -> Result<()> {
10301030
core::repository::attributes::query(
10311031
repository(Mode::Strict)?,
10321032
if pathspecs.is_empty() {
1033-
Box::new(
1034-
stdin_or_bail()?
1035-
.byte_lines()
1036-
.filter_map(Result::ok)
1037-
.filter_map(|line| gix::path::Spec::from_bytes(line.as_bstr())),
1038-
) as Box<dyn Iterator<Item = gix::path::Spec>>
1033+
Box::new(stdin_or_bail()?.byte_lines().filter_map(Result::ok).filter_map(|line| {
1034+
gix::pathspec::parse(
1035+
line.as_bstr(),
1036+
// TODO(pathspec): use `repo` actual global defaults when available (see following as well)
1037+
Default::default(),
1038+
)
1039+
.ok()
1040+
})) as Box<dyn Iterator<Item = gix::pathspec::Pattern>>
10391041
} else {
10401042
Box::new(pathspecs.into_iter())
10411043
},
@@ -1053,15 +1055,11 @@ pub fn main() -> Result<()> {
10531055
progress_keep_open,
10541056
None,
10551057
move |progress, out, err| {
1056-
use gix::bstr::ByteSlice;
10571058
core::repository::attributes::validate_baseline(
10581059
repository(Mode::StrictWithGitInstallConfig)?,
1059-
stdin_or_bail().ok().map(|stdin| {
1060-
stdin
1061-
.byte_lines()
1062-
.filter_map(Result::ok)
1063-
.filter_map(|line| gix::path::Spec::from_bytes(line.as_bstr()))
1064-
}),
1060+
stdin_or_bail()
1061+
.ok()
1062+
.map(|stdin| stdin.byte_lines().filter_map(Result::ok).map(gix::bstr::BString::from)),
10651063
progress,
10661064
out,
10671065
err,
@@ -1088,16 +1086,15 @@ pub fn main() -> Result<()> {
10881086
progress_keep_open,
10891087
None,
10901088
move |_progress, out, err| {
1091-
use gix::bstr::ByteSlice;
10921089
core::repository::exclude::query(
10931090
repository(Mode::Strict)?,
10941091
if pathspecs.is_empty() {
10951092
Box::new(
10961093
stdin_or_bail()?
10971094
.byte_lines()
10981095
.filter_map(Result::ok)
1099-
.filter_map(|line| gix::path::Spec::from_bytes(line.as_bstr())),
1100-
) as Box<dyn Iterator<Item = gix::path::Spec>>
1096+
.filter_map(|line| gix::pathspec::parse(&line, Default::default()).ok()),
1097+
) as Box<dyn Iterator<Item = gix::pathspec::Pattern>>
11011098
} else {
11021099
Box::new(pathspecs.into_iter())
11031100
},

Diff for: ‎src/plumbing/options/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,7 @@ pub mod attributes {
610610
statistics: bool,
611611
/// The git path specifications to list attributes for, or unset to read from stdin one per line.
612612
#[clap(value_parser = AsPathSpec)]
613-
pathspecs: Vec<gix::path::Spec>,
613+
pathspecs: Vec<gix::pathspec::Pattern>,
614614
},
615615
}
616616
}
@@ -639,7 +639,7 @@ pub mod exclude {
639639
patterns: Vec<OsString>,
640640
/// The git path specifications to check for exclusion, or unset to read from stdin one per line.
641641
#[clap(value_parser = AsPathSpec)]
642-
pathspecs: Vec<gix::path::Spec>,
642+
pathspecs: Vec<gix::pathspec::Pattern>,
643643
},
644644
}
645645
}

Diff for: ‎src/porcelain/options.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ pub mod tools {
125125
TracePath {
126126
/// The path to trace through history.
127127
#[clap(value_parser = AsPathSpec)]
128-
path: gix::path::Spec,
128+
path: gix::pathspec::Pattern,
129129
},
130130
}
131131
}

Diff for: ‎src/shared.rs

+10-2
Original file line numberDiff line numberDiff line change
@@ -349,11 +349,19 @@ mod clap {
349349
pub struct AsPathSpec;
350350

351351
impl TypedValueParser for AsPathSpec {
352-
type Value = gix::path::Spec;
352+
type Value = gix::pathspec::Pattern;
353353

354354
fn parse_ref(&self, cmd: &Command, arg: Option<&Arg>, value: &OsStr) -> Result<Self::Value, Error> {
355355
OsStringValueParser::new()
356-
.try_map(|arg| gix::path::Spec::try_from(arg.as_os_str()))
356+
.try_map(|arg| {
357+
let arg: &std::path::Path = arg.as_os_str().as_ref();
358+
gix::pathspec::parse(
359+
gix::path::into_bstr(arg).as_ref(),
360+
// TODO(pathspec): it *should* be possible to obtain these defaults from the environment and then act correctly.
361+
// gix should
362+
Default::default(),
363+
)
364+
})
357365
.parse_ref(cmd, arg, value)
358366
}
359367
}

0 commit comments

Comments
 (0)