Skip to content

Commit

Permalink
fix!: pass crucial context to help spawning filter processes by addin…
Browse files Browse the repository at this point in the history
…g `context` to `Pipeline::new()`. (#1129)

Otherwise, they might now know which repository to apply to,
leading to errors.
  • Loading branch information
Byron committed Nov 25, 2023
1 parent 9d7e28d commit 969ff0f
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 8 deletions.
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
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
1 change: 1 addition & 0 deletions gix-filter/tests/pipeline/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ fn pipeline(
let (drivers, encodings_with_roundtrip_check, crlf_roundtrip_check, eol_config) = init();
let pipe = gix_filter::Pipeline::new(
cache.attributes_collection(),
Default::default(),
gix_filter::pipeline::Options {
drivers,
eol_config,
Expand Down

0 comments on commit 969ff0f

Please sign in to comment.