Skip to content

Commit

Permalink
panic when image does not exist, warnings/errors have been emitted an…
Browse files Browse the repository at this point in the history
…d running on CI or with `CROSS_NO_WARNINGS=1` (#661)
  • Loading branch information
Emilgardis authored Jan 31, 2024
2 parents 6aada9d + fd3b8b8 commit 0547637
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changes/661.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"description": "When cross has given a warning, it will now quit instead of continuing with `cargo` when run in CI or with `CROSS_NO_WARNINGS=1`",
"breaking": true,
"type": "changed"
}
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.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ walkdir = { version = "2.3.2", optional = true }
tempfile = "3.3.0"
owo-colors = { version = "3.5.0", features = ["supports-colors"] }
semver = "1.0.16"
is_ci = "1.1.1"

[target.'cfg(not(windows))'.dependencies]
nix = { version = "0.26.2", default-features = false, features = ["user"] }
Expand Down
7 changes: 6 additions & 1 deletion src/bin/cross.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub fn main() -> cross::Result<()> {
let mut msg_info = shell::MessageInfo::create(args.verbose, args.quiet, args.color.as_deref())?;
let status = match cross::run(args, target_list, &mut msg_info)? {
Some(status) => status,
None => {
None if !msg_info.should_fail() => {
// if we fallback to the host cargo, use the same invocation that was made to cross
let argv: Vec<String> = env::args().skip(1).collect();
msg_info.note("Falling back to `cargo` on the host.")?;
Expand All @@ -42,6 +42,11 @@ pub fn main() -> cross::Result<()> {
_ => cargo::run(&argv, &mut msg_info)?,
}
}
None => {
msg_info.error("Errors encountered before cross compilation, aborting.")?;
msg_info.note("Disable this with `CROSS_NO_WARNINGS=0`")?;
std::process::exit(1);
}
};
let code = status
.code()
Expand Down
7 changes: 5 additions & 2 deletions src/docker/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub(crate) fn run(
paths: DockerPaths,
args: &[String],
msg_info: &mut MessageInfo,
) -> Result<ExitStatus> {
) -> Result<Option<ExitStatus>> {
let engine = &options.engine;
let toolchain_dirs = paths.directories.toolchain_directories();
let package_dirs = paths.directories.package_directories();
Expand Down Expand Up @@ -147,6 +147,9 @@ pub(crate) fn run(
}

ChildContainer::create(engine.clone(), container_id)?;
if msg_info.should_fail() {
return Ok(None);
}
let status = docker
.arg(&image_name)
.add_build_command(toolchain_dirs, &cmd)
Expand All @@ -162,5 +165,5 @@ pub(crate) fn run(
ChildContainer::exit_static();
}

status
status.map(Some)
}
5 changes: 4 additions & 1 deletion src/docker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,16 @@ pub fn image_name(target: &str, sub: Option<&str>, repository: &str, tag: &str)
}
}

// TODO: The Option here in the result should be removed and Result::Error replaced with a enum to properly signal error

// Ok(None) means that the command failed, due to a warning or error, when `msg_info.should_fail() == true`
pub fn run(
options: DockerOptions,
paths: DockerPaths,
args: &[String],
subcommand: Option<crate::Subcommand>,
msg_info: &mut MessageInfo,
) -> Result<ExitStatus> {
) -> Result<Option<ExitStatus>> {
if cfg!(target_os = "windows") && options.in_docker() {
msg_info.fatal(
"running cross insider a container running windows is currently unsupported",
Expand Down
8 changes: 6 additions & 2 deletions src/docker/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ pub(crate) fn run(
args: &[String],
subcommand: Option<crate::Subcommand>,
msg_info: &mut MessageInfo,
) -> Result<ExitStatus> {
) -> Result<Option<ExitStatus>> {
let engine = &options.engine;
let target = &options.target;
let toolchain_dirs = paths.directories.toolchain_directories();
Expand Down Expand Up @@ -897,6 +897,10 @@ pub(crate) fn run(

let mut cmd = options.command_variant.safe_command();

if msg_info.should_fail() {
return Ok(None);
}

if !options.command_variant.is_shell() {
// `clean` doesn't handle symlinks: it will just unlink the target
// directory, so we should just substitute it our target directory
Expand Down Expand Up @@ -1010,5 +1014,5 @@ symlink_recurse \"${{prefix}}\"

ChildContainer::finish_static(is_tty, msg_info);

status
status.map(Some)
}
20 changes: 15 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -624,21 +624,31 @@ pub fn run(
false,
);

if msg_info.should_fail() {
return Ok(None);
}

install_interpreter_if_needed(
&args,
host_version_meta,
&target,
&options,
msg_info,
)?;
let status = docker::run(
let status = if let Some(status) = docker::run(
options,
paths,
&filtered_args,
args.subcommand.clone(),
msg_info,
)
.wrap_err("could not run container")?;
.wrap_err("could not run container")?
{
status
} else {
return Ok(None);
};

let needs_host = args.subcommand.map_or(false, |sc| sc.needs_host(is_remote));
if !status.success() {
warn_on_failure(&target, &toolchain, msg_info)?;
Expand Down Expand Up @@ -857,19 +867,19 @@ pub(crate) fn warn_host_version_mismatch(
);
if versions.is_lt() || (versions.is_eq() && dates.is_lt()) {
if cfg!(not(test)) {
msg_info.warn(format_args!("using older {rustc_warning}.\n > Update with `rustup update --force-non-host {toolchain}`"))?;
msg_info.info(format_args!("using older {rustc_warning}.\n > Update with `rustup update --force-non-host {toolchain}`"))?;
}
return Ok(VersionMatch::OlderTarget);
} else if versions.is_gt() || (versions.is_eq() && dates.is_gt()) {
if cfg!(not(test)) {
msg_info.warn(format_args!(
msg_info.info(format_args!(
"using newer {rustc_warning}.\n > Update with `rustup update`"
))?;
}
return Ok(VersionMatch::NewerTarget);
} else {
if cfg!(not(test)) {
msg_info.warn(format_args!("using {rustc_warning}."))?;
msg_info.info(format_args!("using {rustc_warning}."))?;
}
return Ok(VersionMatch::Different);
}
Expand Down
13 changes: 13 additions & 0 deletions src/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ pub struct MessageInfo {
pub stdout_needs_erase: bool,
pub stderr_needs_erase: bool,
pub cross_debug: bool,
pub has_warned: bool,
}

impl MessageInfo {
Expand All @@ -153,6 +154,7 @@ impl MessageInfo {
.as_deref()
.map(bool_from_envvar)
.unwrap_or_default(),
has_warned: false,
}
}

Expand Down Expand Up @@ -231,13 +233,15 @@ impl MessageInfo {
/// prints a red 'error' message.
#[track_caller]
pub fn error<T: fmt::Display>(&mut self, message: T) -> Result<()> {
self.has_warned = true;
self.stderr_check_erase()?;
status!(@stderr cross_prefix!("error"), Some(&message), red, self)
}

/// prints an amber 'warning' message.
#[track_caller]
pub fn warn<T: fmt::Display>(&mut self, message: T) -> Result<()> {
self.has_warned = true;
match self.verbosity {
Verbosity::Quiet => Ok(()),
_ => status!(@stderr
Expand Down Expand Up @@ -371,6 +375,15 @@ impl MessageInfo {

Ok(())
}

/// Returns true if we've previously warned or errored, and we're in CI or `CROSS_NO_WARNINGS` has been set.
///
/// This is used so that unexpected warnings and errors cause ci to fail.
pub fn should_fail(&self) -> bool {
// FIXME: store env var
env::var("CROSS_NO_WARNINGS").map_or_else(|_| is_ci::cached(), |env| bool_from_envvar(&env))
&& self.has_warned
}
}

impl Default for MessageInfo {
Expand Down

0 comments on commit 0547637

Please sign in to comment.