Skip to content

Commit

Permalink
Merge branch 'improve-filters'
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Nov 26, 2023
2 parents f528ae8 + 8434aab commit f09ea13
Show file tree
Hide file tree
Showing 24 changed files with 402 additions and 28 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion gix-archive/tests/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,6 @@ mod from_tree {
}

fn noop_pipeline() -> gix_filter::Pipeline {
gix_filter::Pipeline::new(&Default::default(), Default::default())
gix_filter::Pipeline::new(&Default::default(), Default::default(), Default::default())
}
}
83 changes: 82 additions & 1 deletion gix-command/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@
#![deny(rust_2018_idioms, missing_docs)]
#![forbid(unsafe_code)]

use bstr::BString;
use std::ffi::OsString;
use std::path::PathBuf;

/// A structure to keep settings to use when invoking a command via [`spawn()`][Prepare::spawn()], after creating it with [`prepare()`].
pub struct Prepare {
/// The command to invoke (either with or without shell depending on `use_shell`.
pub command: OsString,
/// Additional information to be passed to the spawned command.
pub context: Option<Context>,
/// The way standard input is configured.
pub stdin: std::process::Stdio,
/// The way standard output is configured.
Expand Down Expand Up @@ -35,6 +39,37 @@ pub struct Prepare {
pub allow_manual_arg_splitting: bool,
}

/// Additional information that is relevant to spawned processes, which typically receive
/// a wealth of contextual information when spawned from `git`.
///
/// See [the git source code](https://github.com/git/git/blob/cfb8a6e9a93adbe81efca66e6110c9b4d2e57169/git.c#L191)
/// for details.
#[derive(Debug, Default, Clone)]
pub struct Context {
/// The `.git` directory that contains the repository.
///
/// If set, it will be used to set the the `GIT_DIR` environment variable.
pub git_dir: Option<PathBuf>,
/// Set the `GIT_WORK_TREE` environment variable with the given path.
pub worktree_dir: Option<PathBuf>,
/// If `true`, set `GIT_NO_REPLACE_OBJECTS` to `1`, which turns off object replacements, or `0` otherwise.
/// If `None`, the variable won't be set.
pub no_replace_objects: Option<bool>,
/// Set the `GIT_NAMESPACE` variable with the given value, effectively namespacing all
/// operations on references.
pub ref_namespace: Option<BString>,
/// If `true`, set `GIT_LITERAL_PATHSPECS` to `1`, which makes globs literal and prefixes as well, or `0` otherwise.
/// If `None`, the variable won't be set.
pub literal_pathspecs: Option<bool>,
/// If `true`, set `GIT_GLOB_PATHSPECS` to `1`, which lets wildcards not match the `/` character, and equals the `:(glob)` prefix.
/// If `false`, set `GIT_NOGLOB_PATHSPECS` to `1` which lets globs match only themselves.
/// If `None`, the variable won't be set.
pub glob_pathspecs: Option<bool>,
/// If `true`, set `GIT_ICASE_PATHSPECS` to `1`, to let patterns match case-insensitively, or `0` otherwise.
/// If `None`, the variable won't be set.
pub icase_pathspecs: Option<bool>,
}

mod prepare {
use std::{
ffi::OsString,
Expand All @@ -43,7 +78,7 @@ mod prepare {

use bstr::ByteSlice;

use crate::Prepare;
use crate::{Context, Prepare};

/// Builder
impl Prepare {
Expand All @@ -67,6 +102,15 @@ mod prepare {
self
}

/// Set additional `ctx` to be used when spawning the process.
///
/// Note that this is a must for most kind of commands that `git` usually spawns,
/// as at least they need to know the correct `git` repository to function.
pub fn with_context(mut self, ctx: Context) -> Self {
self.context = Some(ctx);
self
}

/// Use a shell, but try to split arguments by hand if this be safely done without a shell.
///
/// If that's not the case, use a shell instead.
Expand Down Expand Up @@ -164,6 +208,36 @@ mod prepare {
.stderr(prep.stderr)
.envs(prep.env)
.args(prep.args);
if let Some(ctx) = prep.context {
if let Some(git_dir) = ctx.git_dir {
cmd.env("GIT_DIR", &git_dir);
}
if let Some(worktree_dir) = ctx.worktree_dir {
cmd.env("GIT_WORK_TREE", worktree_dir);
}
if let Some(value) = ctx.no_replace_objects {
cmd.env("GIT_NO_REPLACE_OBJECTS", usize::from(value).to_string());
}
if let Some(namespace) = ctx.ref_namespace {
cmd.env("GIT_NAMESPACE", gix_path::from_bstring(namespace));
}
if let Some(value) = ctx.literal_pathspecs {
cmd.env("GIT_LITERAL_PATHSPECS", usize::from(value).to_string());
}
if let Some(value) = ctx.glob_pathspecs {
cmd.env(
if value {
"GIT_GLOB_PATHSPECS"
} else {
"GIT_NOGLOB_PATHSPECS"
},
"1",
);
}
if let Some(value) = ctx.icase_pathspecs {
cmd.env("GIT_ICASE_PATHSPECS", usize::from(value).to_string());
}
}
cmd
}
}
Expand All @@ -176,9 +250,16 @@ mod prepare {
/// - `stdin` is null to prevent blocking unexpectedly on consumption of stdin
/// - `stdout` is captured for consumption by the caller
/// - `stderr` is inherited to allow the command to provide context to the user
///
/// ### Warning
///
/// When using this method, be sure that the invoked program doesn't rely on the current working dir and/or
/// environment variables to know its context. If so, call instead [`Prepare::with_context()`] to provide
/// additional information.
pub fn prepare(cmd: impl Into<OsString>) -> Prepare {
Prepare {
command: cmd.into(),
context: None,
stdin: std::process::Stdio::null(),
stdout: std::process::Stdio::piped(),
stderr: std::process::Stdio::inherit(),
Expand Down
106 changes: 106 additions & 0 deletions gix-command/tests/command.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,111 @@
use gix_testtools::Result;

mod context {
use gix_command::Context;

fn winfix(expected: impl Into<String>) -> String {
// Unclear why it's not debug-printing the env on windows.
if cfg!(windows) {
"\"\"".into()
} else {
expected.into()
}
}

#[test]
fn git_dir_sets_git_dir_env_and_cwd() {
let ctx = Context {
git_dir: Some(".".into()),
..Default::default()
};
let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx));
assert_eq!(format!("{cmd:?}"), winfix(r#"GIT_DIR="." """#));
}

#[test]
fn worktree_dir_sets_env_only() {
let ctx = Context {
worktree_dir: Some(".".into()),
..Default::default()
};
let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx));
assert_eq!(format!("{cmd:?}"), winfix(r#"GIT_WORK_TREE="." """#));
}

#[test]
fn no_replace_objects_sets_env_only() {
for value in [false, true] {
let expected = usize::from(value);
let ctx = Context {
no_replace_objects: Some(value),
..Default::default()
};
let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx));
assert_eq!(
format!("{cmd:?}"),
winfix(format!(r#"GIT_NO_REPLACE_OBJECTS="{expected}" """#))
);
}
}

#[test]
fn ref_namespace_sets_env_only() {
let ctx = Context {
ref_namespace: Some("namespace".into()),
..Default::default()
};
let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx));
assert_eq!(format!("{cmd:?}"), winfix(r#"GIT_NAMESPACE="namespace" """#));
}

#[test]
fn literal_pathspecs_sets_env_only() {
for value in [false, true] {
let expected = usize::from(value);
let ctx = Context {
literal_pathspecs: Some(value),
..Default::default()
};
let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx));
assert_eq!(
format!("{cmd:?}"),
winfix(format!(r#"GIT_LITERAL_PATHSPECS="{expected}" """#))
);
}
}

#[test]
fn glob_pathspecs_sets_env_only() {
for (value, expected) in [
(false, "GIT_NOGLOB_PATHSPECS=\"1\""),
(true, "GIT_GLOB_PATHSPECS=\"1\""),
] {
let ctx = Context {
glob_pathspecs: Some(value),
..Default::default()
};
let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx));
assert_eq!(format!("{cmd:?}"), winfix(format!(r#"{expected} """#)));
}
}

#[test]
fn icase_pathspecs_sets_env_only() {
for value in [false, true] {
let expected = usize::from(value);
let ctx = Context {
icase_pathspecs: Some(value),
..Default::default()
};
let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx));
assert_eq!(
format!("{cmd:?}"),
winfix(format!(r#"GIT_ICASE_PATHSPECS="{expected}" """#))
);
}
}
}

mod prepare {
#[cfg(windows)]
const SH: &str = "sh";
Expand Down
1 change: 1 addition & 0 deletions gix-filter/src/driver/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ pub struct Context<'a, 'b> {
pub blob: Option<gix_hash::ObjectId>,
}

/// Apply operations to filter programs.
impl State {
/// Apply `operation` of `driver` to the bytes read from `src` and return a reader to immediately consume the output
/// produced by the filter. `rela_path` is the repo-relative path of the entry to handle.
Expand Down
1 change: 1 addition & 0 deletions gix-filter/src/driver/delayed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ pub mod fetch {
}
}

/// Operations related to delayed filtering.
impl State {
/// Return a list of delayed paths for `process` that can then be obtained with [`fetch_delayed()`][Self::fetch_delayed()].
///
Expand Down
12 changes: 9 additions & 3 deletions gix-filter/src/driver/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub enum Error {
},
}

/// Lifecycle
impl State {
/// Obtain a process as defined in `driver` suitable for a given `operation. `rela_path` may be used to substitute the current
/// file for use in the invoked `SingleFile` process.
Expand All @@ -40,7 +41,7 @@ impl State {
let client = match self.running.remove(process) {
Some(c) => c,
None => {
let (child, cmd) = spawn_driver(process.clone())?;
let (child, cmd) = spawn_driver(process.clone(), &self.context)?;
process::Client::handshake(child, "git-filter", &[2], &["clean", "smudge", "delay"]).map_err(
|err| Error::ProcessHandshake {
source: err,
Expand Down Expand Up @@ -79,20 +80,25 @@ impl State {
None => return Ok(None),
};

let (child, command) = spawn_driver(cmd)?;
let (child, command) = spawn_driver(cmd, &self.context)?;
Ok(Some(Process::SingleFile { child, command }))
}
}
}
}

fn spawn_driver(cmd: BString) -> Result<(std::process::Child, std::process::Command), Error> {
fn spawn_driver(
cmd: BString,
context: &gix_command::Context,
) -> Result<(std::process::Child, std::process::Command), Error> {
let mut cmd: std::process::Command = gix_command::prepare(gix_path::from_bstr(cmd).into_owned())
.with_shell()
.with_context(context.clone())
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::inherit())
.into();
gix_trace::debug!(cmd = ?cmd, "launching filter driver");
let child = match cmd.spawn() {
Ok(child) => child,
Err(err) => {
Expand Down
15 changes: 15 additions & 0 deletions gix-filter/src/driver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,27 @@ pub struct State {
/// Note that these processes are expected to shut-down once their stdin/stdout are dropped, so nothing else
/// needs to be done to clean them up after drop.
running: HashMap<BString, process::Client>,

/// The context to pass to spawned filter programs.
pub context: gix_command::Context,
}

/// Initialization
impl State {
/// Create a new instance using `context` to inform launched processes about their environment.
pub fn new(context: gix_command::Context) -> Self {
Self {
running: Default::default(),
context,
}
}
}

impl Clone for State {
fn clone(&self) -> Self {
State {
running: Default::default(),
context: self.context.clone(),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions gix-filter/src/driver/process/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,8 @@ impl Client {
self.send_command_and_meta(command, meta)?;
while let Some(data) = self.out.read_line() {
let line = data??;
if let Some(line) = line.as_bstr() {
inspect_line(line);
if let Some(line) = line.as_text() {
inspect_line(line.as_bstr());
}
}
self.out.reset_with(&[gix_packetline::PacketLineRef::Flush]);
Expand Down
14 changes: 9 additions & 5 deletions gix-filter/src/pipeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,22 @@ const ATTRS: [&str; 6] = ["crlf", "ident", "filter", "eol", "text", "working-tre

/// Lifecycle
impl Pipeline {
/// Create a new pipeline with configured `drivers` (which should be considered safe to invoke) as well as a way to initialize
/// our attributes with `collection`.
/// Create a new pipeline with configured `drivers` (which should be considered safe to invoke) with `context` as well as
/// a way to initialize our attributes with `collection`.
/// `eol_config` serves as fallback to understand how to convert line endings if no line-ending attributes are present.
/// `crlf_roundtrip_check` corresponds to the git-configuration of `core.safecrlf`.
/// `object_hash` is relevant for the `ident` filter.
pub fn new(collection: &gix_attributes::search::MetadataCollection, options: Options) -> Self {
pub fn new(
collection: &gix_attributes::search::MetadataCollection,
context: gix_command::Context,
options: Options,
) -> Self {
let mut attrs = gix_attributes::search::Outcome::default();
attrs.initialize_with_selection(collection, ATTRS);
Pipeline {
attrs,
context: Context::default(),
processes: driver::State::default(),
processes: driver::State::new(context),
options,
bufs: Default::default(),
}
Expand All @@ -80,7 +84,7 @@ impl Pipeline {
impl Default for Pipeline {
fn default() -> Self {
let collection = Default::default();
Pipeline::new(&collection, Default::default())
Pipeline::new(&collection, Default::default(), Default::default())
}
}

Expand Down
Loading

0 comments on commit f09ea13

Please sign in to comment.