Skip to content

fix(core): move newer bail! sites out of expression position (partial #2835, complements #2843) #3086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
phil-opp wants to merge 1 commit into from
+46 −34
Closed

fix(core): move newer bail! sites out of expression position (partial #2835, complements #2843) #3086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 12 additions & 10 deletions libraries/core/src/build/git.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl GitManager {
"the build directory is still in use by the following \
dataflows, please stop them before rebuilding: {}",
using.iter().join(", ")
)
);
}

let reuse = if self.clone_dir_ready(session_id, &clone_dir) {
Expand Down Expand Up @@ -293,7 +293,7 @@ impl GitFolder {
.log_message(LogLevel::Error, format!("{err:?}"))
.await;
cleanup_failed_clone(logger, &target_dir).await;
bail!(err)
bail!(err);
}
}
}
Expand Down Expand Up @@ -345,7 +345,7 @@ impl GitFolder {
Ok(()) => target_dir,
Err(err) => {
cleanup_failed_clone(logger, &target_dir).await;
bail!(err)
bail!(err);
}
}
}
Expand Down Expand Up @@ -379,7 +379,7 @@ impl GitFolder {
Ok(()) => target_dir,
Err(err) => {
cleanup_failed_clone(logger, &target_dir).await;
bail!(err)
bail!(err);
}
}
}
Expand Down Expand Up @@ -432,12 +432,14 @@ impl GitFolder {
// in-progress set only covers one GitManager and the CLI
// builds with its own, so deleting here is how #2711
// wipes out a live build. Fail loudly, leave the dir.
Err(err) => bail!(
"couldn't verify clone dir {} is on commit {commit_hash}: {err:?}; \
leaving it in place in case another build is writing it, \
please retry",
dir.display()
),
Err(err) => {
bail!(
"couldn't verify clone dir {} is on commit {commit_hash}: {err:?}; \
leaving it in place in case another build is writing it, \
please retry",
dir.display()
);
}
}
}

Expand Down
10 changes: 6 additions & 4 deletions libraries/core/src/descriptor/expand.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,10 +693,12 @@ fn rewrite_module_input(
}))
}
None if optional_inputs.contains(&port_name) => Ok(None),
None => bail!(
"module input reference `_mod/{}` not found in module node inputs",
port_name,
),
None => {
bail!(
"module input reference `_mod/{}` not found in module node inputs",
port_name,
);
}
}
} else if inner_node_ids.contains(&source_str) {
// Internal cross-reference: prefix with module_id
Expand Down
14 changes: 7 additions & 7 deletions libraries/core/src/descriptor/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ fn node_kind_mut(node: &mut Node) -> eyre::Result<NodeKindMut<'_>> {
"module node `{}` must be expanded before resolution — \
call expand_modules() first",
node.id
)
);
}
NodeKind::Standard(_) => {
let source = match (&node.git, &node.branch, &node.tag, &node.rev) {
Expand All @@ -369,7 +369,7 @@ fn node_kind_mut(node: &mut Node) -> eyre::Result<NodeKindMut<'_>> {
other @ (_, _, _) => {
eyre::bail!(
"only one of `branch`, `tag`, and `rev` are allowed (got {other:?})"
)
);
}
};
NodeSource::GitBranch {
Expand All @@ -378,7 +378,7 @@ fn node_kind_mut(node: &mut Node) -> eyre::Result<NodeKindMut<'_>> {
}
}
(None, _, _, _) => {
eyre::bail!("`git` source required when using branch, tag, or rev")
eyre::bail!("`git` source required when using branch, tag, or rev");
}
};

Expand Down Expand Up @@ -454,7 +454,7 @@ pub fn resolve_path(source: &str, working_dir: &Path) -> Result<PathBuf> {
} else if let Ok(abs_path) = which::which(&path) {
Ok(abs_path)
} else {
bail!("Could not find source path {}", path.display())
bail!("Could not find source path {}", path.display());
}
}

Expand Down Expand Up @@ -500,7 +500,7 @@ pub fn resolve_path_confined(
python_env_dir
.map(|env| format!(" or its managed environment `{}`", env.display()))
.unwrap_or_default(),
)
);
}

/// Canonicalize `candidate` and require it to stay under `root`.
Expand Down Expand Up @@ -605,7 +605,7 @@ impl NodeExt for Node {
eyre::bail!(
"node `{}` requires a `path`, `custom`, `operators`, `ros2`, or `module` field",
self.id
)
);
}
(None, None, None, Some(operator), None, None) => Ok(NodeKind::Operator(operator)),
(None, None, Some(custom), None, None, None) => Ok(NodeKind::Custom(custom)),
Expand All @@ -617,7 +617,7 @@ impl NodeExt for Node {
eyre::bail!(
"node `{}` has multiple exclusive fields set, only one of `path`, `custom`, `operators`, `operator`, `ros2`, and `module` is allowed",
self.id
)
);
}
}
}
Expand Down
34 changes: 21 additions & 13 deletions libraries/core/src/descriptor/validate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,9 @@ fn parse_byte_size(s: &str) -> eyre::Result<u64> {
"KB" | "K" => 1024,
"MB" | "M" => 1024 * 1024,
"GB" | "G" => 1024 * 1024 * 1024,
_ => bail!("unknown byte size unit: '{unit}', expected B, KB, MB, or GB"),
_ => {
bail!("unknown byte size unit: '{unit}', expected B, KB, MB, or GB");
}
};
// Use integer parse when possible to avoid float rounding
if let Ok(num) = num_str.parse::<u64>() {
Expand Down Expand Up @@ -472,9 +474,11 @@ fn parse_log_level(s: &str) -> eyre::Result<dora_message::common::LogLevelOrStdo
log::Level::Trace,
)),
"stdout" => Ok(dora_message::common::LogLevelOrStdout::Stdout),
_ => bail!(
"invalid min_log_level: '{s}', expected one of: error, warn, info, debug, trace, stdout"
),
_ => {
bail!(
"invalid min_log_level: '{s}', expected one of: error, warn, info, debug, trace, stdout"
);
}
}
}

Expand Down Expand Up @@ -565,7 +569,7 @@ assert dora.__version__=='{VERSION}', 'Python dora-rs should be {VERSION}, but
.wrap_err("Could not get exit status when checking python dora-rs")?;

if !status.success() {
bail!("Something went wrong with Python dora-rs. {reinstall_command}")
bail!("Something went wrong with Python dora-rs. {reinstall_command}");
}

Ok(())
Expand Down Expand Up @@ -757,19 +761,23 @@ fn validate_ros2_qos(
if let Some(d) = &qos.durability {
match d.as_str() {
"volatile" | "transient_local" => {}
_ => bail!(
"node `{node_id}`: invalid QoS durability `{d}`, \
expected \"volatile\" or \"transient_local\""
),
_ => {
bail!(
"node `{node_id}`: invalid QoS durability `{d}`, \
expected \"volatile\" or \"transient_local\""
);
}
}
}
if let Some(l) = &qos.liveliness {
match l.as_str() {
"automatic" | "manual_by_participant" | "manual_by_topic" => {}
_ => bail!(
"node `{node_id}`: invalid QoS liveliness `{l}`, \
expected \"automatic\", \"manual_by_participant\", or \"manual_by_topic\""
),
_ => {
bail!(
"node `{node_id}`: invalid QoS liveliness `{l}`, \
expected \"automatic\", \"manual_by_participant\", or \"manual_by_topic\""
);
}
}
}
if let Some(depth) = qos.keep_last
Expand Down