Skip to content

Commit 0f51cc6

Browse files
committed
fix: only add a backend input for specified inputs.
Fix check in the Docker backend that requires an input to exist for it to be included in the bind mounts. The underlying problem is that a backend input should only be added for a specified input as it may contain host or remote paths; for inputs that are defaulted, the evaluated expression will only contain guest paths and therefore does not need an input.
1 parent 035dd4d commit 0f51cc6

File tree

3 files changed

+49
-43
lines changed

3 files changed

+49
-43
lines changed

wdl-engine/src/backend/docker.rs

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -135,20 +135,15 @@ impl TaskManagerRequest for DockerTaskRequest {
135135
let mut inputs = Vec::with_capacity(self.inner.inputs().len() + 2);
136136
for input in self.inner.inputs().iter() {
137137
let guest_path = input.guest_path().expect("input should have guest path");
138-
let location = input
139-
.location()
140-
.unwrap_or_else(|| input.path().as_local().expect("input should be local"));
141-
142-
if location.exists() {
143-
inputs.push(
144-
Input::builder()
145-
.path(guest_path)
146-
.contents(Contents::Path(location.into()))
147-
.ty(input.kind())
148-
.read_only(true)
149-
.build(),
150-
);
151-
}
138+
let local_path = input.local_path().expect("input should be localized");
139+
inputs.push(
140+
Input::builder()
141+
.path(guest_path)
142+
.contents(Contents::Path(local_path.into()))
143+
.ty(input.kind())
144+
.read_only(true)
145+
.build(),
146+
);
152147
}
153148

154149
// Add an input for the work directory

wdl-engine/src/eval.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,8 @@ impl Input {
562562
}
563563

564564
/// Gets the path to the input.
565+
///
566+
/// The path of the input may be local or remote.
565567
pub fn path(&self) -> &EvaluationPath {
566568
&self.path
567569
}
@@ -571,14 +573,16 @@ impl Input {
571573
self.guest_path.as_deref()
572574
}
573575

574-
/// Gets the location of the input if it has been downloaded.
576+
/// Gets the local path of the input.
575577
///
576-
/// Returns `None` if the input has not been downloaded or is not remote.
577-
pub fn location(&self) -> Option<&Path> {
578-
self.location.as_deref()
578+
/// Returns `None` if the input is remote and has not been localized.
579+
pub fn local_path(&self) -> Option<&Path> {
580+
self.location.as_deref().or_else(|| self.path.as_local())
579581
}
580582

581583
/// Sets the location of the input.
584+
///
585+
/// This is used during localization to set a local path for remote inputs.
582586
pub fn set_location(&mut self, location: Location<'static>) {
583587
self.location = Some(location);
584588
}

wdl-engine/src/eval/v1/task.rs

Lines changed: 32 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -997,8 +997,23 @@ impl TaskEvaluator {
997997
let decl_ty = decl.ty();
998998
let ty = crate::convert_ast_type_v1(state.document, &decl_ty)?;
999999

1000-
let (value, span) = match inputs.get(name.text()) {
1001-
Some(input) => (input.clone(), name.span()),
1000+
let value = match inputs.get(name.text()) {
1001+
Some(input) => {
1002+
let mut value = input.coerce(&ty).map_err(|e| {
1003+
runtime_type_mismatch(e, &ty, name.span(), &input.ty(), name.span())
1004+
})?;
1005+
1006+
// Add the input to the inputs collection
1007+
// Only performed for inputs given to the task as they may contain host paths
1008+
if state.guest_roots.is_none() {
1009+
self.add_input(&name, &ty, &mut value, state).await?;
1010+
} else {
1011+
self.add_guest_path_input(&name, &ty, &mut value, state)
1012+
.await?;
1013+
}
1014+
1015+
value
1016+
}
10021017
None => match decl.expr() {
10031018
Some(expr) => {
10041019
debug!(
@@ -1015,28 +1030,20 @@ impl TaskEvaluator {
10151030
ROOT_SCOPE_INDEX,
10161031
));
10171032
let value = evaluator.evaluate_expr(&expr).await?;
1018-
(value, expr.span())
1033+
1034+
// Note: a backend input is *not* added to the state because any literal or
1035+
// reference to another input will already be a guest path
1036+
value.coerce(&ty).map_err(|e| {
1037+
runtime_type_mismatch(e, &ty, name.span(), &value.ty(), expr.span())
1038+
})?
10191039
}
10201040
_ => {
10211041
assert!(ty.is_optional(), "type should be optional");
1022-
(Value::new_none(ty.clone()), name.span())
1042+
Value::new_none(ty.clone())
10231043
}
10241044
},
10251045
};
10261046

1027-
let mut value = value
1028-
.coerce(&ty)
1029-
.map_err(|e| runtime_type_mismatch(e, &ty, name.span(), &value.ty(), span))?;
1030-
1031-
// If the backend does not run tasks in a container, add the input without guest
1032-
// path
1033-
if state.guest_roots.is_none() {
1034-
self.add_input(&name, &ty, &mut value, state).await?;
1035-
} else {
1036-
self.add_guest_path_input(&name, &ty, &mut value, state)
1037-
.await?;
1038-
}
1039-
10401047
// Insert the name into the scope
10411048
state.scopes[ROOT_SCOPE_INDEX.0].insert(name.text(), value.clone());
10421049

@@ -1524,7 +1531,7 @@ impl TaskEvaluator {
15241531

15251532
// Download any necessary files
15261533
for (idx, input) in state.inputs.as_slice_mut().iter_mut().enumerate() {
1527-
if input.location().is_some() {
1534+
if input.local_path().is_some() {
15281535
continue;
15291536
}
15301537

@@ -1559,7 +1566,7 @@ impl TaskEvaluator {
15591566

15601567
if enabled!(Level::DEBUG) {
15611568
for input in state.inputs.as_slice() {
1562-
match (input.location(), input.guest_path()) {
1569+
match (input.local_path(), input.guest_path()) {
15631570
(None, None) => {}
15641571
(None, Some(guest_path)) => {
15651572
debug!(
@@ -1570,25 +1577,25 @@ impl TaskEvaluator {
15701577
path = input.path().display(),
15711578
);
15721579
}
1573-
(Some(location), None) => {
1580+
(Some(local_path), None) => {
15741581
debug!(
15751582
task_id,
15761583
task_name = state.task.name(),
15771584
document = state.document.uri().as_str(),
1578-
"task input `{path}` downloaded to `{location}`",
1585+
"task input `{path}` downloaded to `{local_path}`",
15791586
path = input.path().display(),
1580-
location = location.display()
1587+
local_path = local_path.display()
15811588
);
15821589
}
1583-
(Some(location), Some(guest_path)) => {
1590+
(Some(local_path), Some(guest_path)) => {
15841591
debug!(
15851592
task_id,
15861593
task_name = state.task.name(),
15871594
document = state.document.uri().as_str(),
1588-
"task input `{path}` downloaded to `{location}` and mapped to \
1595+
"task input `{path}` downloaded to `{local_path}` and mapped to \
15891596
`{guest_path}`",
15901597
path = input.path().display(),
1591-
location = location.display(),
1598+
local_path = local_path.display(),
15921599
);
15931600
}
15941601
}

0 commit comments

Comments
 (0)