Skip to content

Commit

Permalink
Warn on console= karg if --console could have been used instead
Browse files Browse the repository at this point in the history
If one or more console= kargs are specified to the install or iso/pxe
customize subcommands, and Console can successfully parse all of those
kargs, warn the user that they might benefit from specifying them using
the console mechanism instead.

For legibility, wrap the message at 80 characters, but avoid wrapping in
the middle of the example argument lists.  Use the textwrap crate to do
this.  textwrap is already a clap dependency without default features, and
we also avoid non-default features here, so this doesn't increase the
binary weight.
  • Loading branch information
bgilbert committed Sep 10, 2022
1 parent 4aaf6fe commit c8a24ea
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 1 deletion.
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 @@ -77,6 +77,7 @@ serde_json = "^1.0"
serde_with = ">= 1.9.4, < 2"
serde_yaml = ">= 0.8, < 0.10"
tempfile = ">= 3.1, < 4"
textwrap = { version = ">= 0.15.0, < 0.16.0", default-features = false }
thiserror = "1.0"
url = ">= 2.1, < 3.0"
uuid = { version = ">= 0.8, < 2.0", features = ["v4"] }
Expand Down
4 changes: 3 additions & 1 deletion docs/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Minor changes:
- Fix unlikely decompression error reading initrd
- Add release notes to documentation
- iso: Detect incomplete ISO files
- Warn if console kargs could have used `--console`/`--dest-console` instead

Internal changes:

Expand All @@ -30,7 +31,8 @@ Internal changes:
Packaging changes:

- Require Rust ≥ 1.58.0
- Add dependency on `zstd` crate and `libzstd` shared library
- Add dependencies on `textwrap` and `zstd` crates
- Add dependency on `libzstd` shared library
- Support `serde_yaml` 0.9
- Remove non-Linux dependencies from vendor archive
- Install example installer config file in `/usr/share/coreos-installer`
Expand Down
87 changes: 87 additions & 0 deletions src/cmdline/console.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,55 @@ impl Console {
pub fn karg(&self) -> String {
format!("{KARG_PREFIX}{}", self)
}

/// Write a warning message to stdout if kargs contains "console="
/// arguments we can parse and no "console=" arguments we can't. The
/// warning suggests that users use console_option instead of
/// karg_option to specify the desired console.
pub fn maybe_warn_on_kargs(kargs: &[String], karg_option: &str, console_option: &str) {
use textwrap::{fill, Options, WordSplitter};
if let Some(args) = Self::maybe_console_args_from_kargs(kargs) {
// automatically wrap the message, but use Unicode non-breaking
// spaces to avoid wrapping in the middle of the argument
// strings, and then replace the non-breaking spaces afterward
const NBSP: &str = "\u{a0}";
let msg = format!(
"Note: consider using \"{}\" instead of \"{}\" to configure both kernel and bootloader consoles.",
args.iter()
.map(|a| format!("{console_option}{NBSP}{a}"))
.collect::<Vec<String>>()
.join(NBSP),
args.iter()
.map(|a| format!("{karg_option}{NBSP}console={a}"))
.collect::<Vec<String>>()
.join(NBSP),
);
let wrapped = fill(
&msg,
Options::new(80)
.break_words(false)
.word_splitter(WordSplitter::NoHyphenation),
)
.replace(NBSP, " ");
eprintln!("\n{}\n", wrapped);
}
}

/// If kargs contains at least one console argument and all console
/// arguments are parseable as consoles, return a vector of verbatim
/// (unparsed) console arguments with the console= prefixes removed.
fn maybe_console_args_from_kargs(kargs: &[String]) -> Option<Vec<&str>> {
let (parseable, unparseable): (Vec<&str>, Vec<&str>) = kargs
.iter()
.filter(|a| a.starts_with(KARG_PREFIX))
.map(|a| &a[KARG_PREFIX.len()..])
.partition(|a| Console::from_str(a).is_ok());
if !parseable.is_empty() && unparseable.is_empty() {
Some(parseable)
} else {
None
}
}
}

impl FromStr for Console {
Expand Down Expand Up @@ -257,4 +306,42 @@ mod tests {
Console::from_str(input).unwrap_err();
}
}

#[test]
fn maybe_console_args_from_kargs() {
assert_eq!(
Console::maybe_console_args_from_kargs(&[
"foo".into(),
"console=ttyS0".into(),
"bar".into()
]),
Some(vec!["ttyS0"])
);
assert_eq!(
Console::maybe_console_args_from_kargs(&[
"foo".into(),
"console=ttyS0".into(),
"console=tty0".into(),
"console=tty0".into(),
"console=ttyAMA1,115200n8".into(),
"bar".into()
]),
Some(vec!["ttyS0", "tty0", "tty0", "ttyAMA1,115200n8"])
);
assert_eq!(
Console::maybe_console_args_from_kargs(&[
"foo".into(),
"console=ttyS0".into(),
"console=ttyS1z".into(),
"console=tty0".into(),
"bar".into()
]),
None
);
assert_eq!(
Console::maybe_console_args_from_kargs(&["foo".into(), "bar".into()]),
None
);
assert_eq!(Console::maybe_console_args_from_kargs(&[]), None);
}
}
1 change: 1 addition & 0 deletions src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,7 @@ fn write_disk(
if !config.append_karg.is_empty() || !config.delete_karg.is_empty() {
eprintln!("Modifying kernel arguments");

Console::maybe_warn_on_kargs(&config.append_karg, "--append-karg", "--console");
visit_bls_entry_options(mount.mountpoint(), |orig_options: &str| {
KargsEditor::new()
.append(config.append_karg.as_slice())
Expand Down
5 changes: 5 additions & 0 deletions src/live/customize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ impl LiveInitrd {
for arg in &common.dest_console {
conf.dest_console(arg)?;
}
Console::maybe_warn_on_kargs(
&common.dest_karg_append,
"--dest-karg-append",
"--dest-console",
);
for arg in &common.dest_karg_append {
conf.dest_karg_append(arg);
}
Expand Down

0 comments on commit c8a24ea

Please sign in to comment.