build-sys: A lot more manpage followups

- Remove duplicated logic between xtask and makefile
  for converting markdown; it needs to be in xtask
  as we handle the version substitution there and
  some other tweaks
- Really just make the developer entrypoint `just update-generated`
  in general
- Fix the rendering of booleans
- Remove unnecessary emoji from prints

Signed-off-by: Colin Walters <walters@verbum.org>

Author: cgwalters

Makefile

@@ -15,22 +15,9 @@ all: bin manpages
 bin:
 	cargo build --release --features "$(CARGO_FEATURES)"
 
-# Generate man pages from markdown sources
-MAN5_SOURCES := $(wildcard docs/src/man/*.5.md)
-MAN8_SOURCES := $(wildcard docs/src/man/*.8.md)
-TARGETMAN := target/man
-MAN5_TARGETS := $(patsubst docs/src/man/%.5.md,$(TARGETMAN)/%.5,$(MAN5_SOURCES))
-MAN8_TARGETS := $(patsubst docs/src/man/%.8.md,$(TARGETMAN)/%.8,$(MAN8_SOURCES))
-
-$(TARGETMAN)/%.5: docs/src/man/%.5.md
-	@mkdir -p $(TARGETMAN)
-	go-md2man -in $< -out $@
-
-$(TARGETMAN)/%.8: docs/src/man/%.8.md
-	@mkdir -p $(TARGETMAN)
-	go-md2man -in $< -out $@
-
-manpages: $(MAN5_TARGETS) $(MAN8_TARGETS)
+.PHONY: manpages
+manpages:
+	cargo run --package xtask -- manpages
 
 STORAGE_RELATIVE_PATH ?= $(shell realpath -m -s --relative-to="$(prefix)/lib/bootc/storage" /sysroot/ostree/bootc/storage)
 install:

crates/lib/src/cli_json.rs

@@ -13,6 +13,7 @@ pub struct CliOption {
     pub help: String,
     pub possible_values: Vec<String>,
     pub required: bool,
+    pub is_boolean: bool,
 }
 
 /// Representation of a CLI command for JSON export
@@ -73,16 +74,27 @@ pub fn command_to_json(cmd: &Command) -> CliCommand {
 
             let help = arg.get_help().map(|h| h.to_string()).unwrap_or_default();
 
+            // For boolean flags, don't show a value name
+            // Boolean flags use SetTrue or SetFalse actions and don't take values
+            let is_boolean = matches!(
+                arg.get_action(),
+                clap::ArgAction::SetTrue | clap::ArgAction::SetFalse
+            );
+            let value_name = if is_boolean {
+                None
+            } else {
+                arg.get_value_names()
+                    .and_then(|names| names.first())
+                    .map(|s| s.to_string())
+            };
+
             options.push(CliOption {
                 long: arg
                     .get_long()
                     .map(String::from)
                     .unwrap_or_else(|| id.to_string()),
                 short: arg.get_short().map(|c| c.to_string()),
-                value_name: arg
-                    .get_value_names()
-                    .and_then(|names| names.first())
-                    .map(|s| s.to_string()),
+                value_name,
                 default: arg
                     .get_default_values()
                     .first()
@@ -91,6 +103,7 @@ pub fn command_to_json(cmd: &Command) -> CliCommand {
                 help,
                 possible_values,
                 required: arg.is_required_set(),
+                is_boolean,
             });
         }
     }

crates/xtask/src/man.rs

@@ -26,6 +26,8 @@ pub struct CliOption {
     pub possible_values: Vec<String>,
     /// Whether the option is required
     pub required: bool,
+    /// Whether this is a boolean flag
+    pub is_boolean: bool,
 }
 
 /// Represents a CLI command from the JSON dump
@@ -132,16 +134,17 @@ fn format_options_as_markdown(options: &[CliOption], positionals: &[CliPositiona
         // Add long flag
         flag_line.push_str(&format!("**--{}**", opt.long));
 
-        // Add value name if option takes argument
+        // Add value name if option takes argument (but not for boolean flags)
+        // Boolean flags are detected by having no value_name (set to None in cli_json.rs)
         if let Some(value_name) = &opt.value_name {
             flag_line.push_str(&format!("=*{}*", value_name));
         }
 
         result.push_str(&format!("{}\n\n", flag_line));
         result.push_str(&format!("    {}\n\n", opt.help));
 
-        // Add possible values for enums
-        if !opt.possible_values.is_empty() {
+        // Add possible values for enums (but not for boolean flags)
+        if !opt.possible_values.is_empty() && !opt.is_boolean {
             result.push_str("    Possible values:\n");
             for value in &opt.possible_values {
                 result.push_str(&format!("    - {}\n", value));
@@ -191,10 +194,12 @@ pub fn update_markdown_with_subcommands(
         before, begin_marker, generated_subcommands, end_marker, after
     );
 
-    fs::write(markdown_path, new_content)
-        .with_context(|| format!("Writing to {}", markdown_path))?;
-
-    println!("Updated subcommands in {}", markdown_path);
+    // Only write if content has changed to avoid updating mtime unnecessarily
+    if new_content != content {
+        fs::write(markdown_path, new_content)
+            .with_context(|| format!("Writing to {}", markdown_path))?;
+        println!("Updated subcommands in {}", markdown_path);
+    }
     Ok(())
 }
 
@@ -238,10 +243,12 @@ pub fn update_markdown_with_options(
         format!("{}\n\n{}\n{}{}", before, begin_marker, end_marker, after)
     };
 
-    fs::write(markdown_path, new_content)
-        .with_context(|| format!("Writing to {}", markdown_path))?;
-
-    println!("Updated {}", markdown_path);
+    // Only write if content has changed to avoid updating mtime unnecessarily
+    if new_content != content {
+        fs::write(markdown_path, new_content)
+            .with_context(|| format!("Writing to {}", markdown_path))?;
+        println!("Updated {}", markdown_path);
+    }
     Ok(())
 }
 
@@ -374,21 +381,6 @@ pub fn sync_all_man_pages(sh: &Shell) -> Result<()> {
     Ok(())
 }
 
-/// Test the sync workflow
-pub fn test_sync_workflow(sh: &Shell) -> Result<()> {
-    println!("🧪 Testing man page sync workflow...");
-
-    // Create a backup of current files
-    let test_dir = "target/test-sync";
-    sh.create_dir(test_dir)?;
-
-    // Run sync
-    sync_all_man_pages(sh)?;
-
-    println!("✅ Sync workflow test completed successfully");
-    Ok(())
-}
-
 /// Generate man pages from hand-written markdown sources
 pub fn generate_man_pages(sh: &Shell) -> Result<()> {
     let man_src_dir = Utf8Path::new("docs/src/man");
@@ -432,18 +424,31 @@ pub fn generate_man_pages(sh: &Shell) -> Result<()> {
         let content = fs::read_to_string(&path)?;
         let content_with_version = content.replace("<!-- VERSION PLACEHOLDER -->", &version);
 
-        // Create temporary file with version-replaced content
-        let temp_path = format!("{}.tmp", path.display());
-        fs::write(&temp_path, content_with_version)?;
+        // Check if we need to regenerate by comparing input and output modification times
+        let should_regenerate = if let (Ok(input_meta), Ok(output_meta)) =
+            (fs::metadata(&path), fs::metadata(&output_file))
+        {
+            input_meta.modified().unwrap_or(std::time::UNIX_EPOCH)
+                > output_meta.modified().unwrap_or(std::time::UNIX_EPOCH)
+        } else {
+            // If output doesn't exist or we can't get metadata, regenerate
+            true
+        };
+
+        if should_regenerate {
+            // Create temporary file with version-replaced content
+            let temp_path = format!("{}.tmp", path.display());
+            fs::write(&temp_path, content_with_version)?;
 
-        cmd!(sh, "go-md2man -in {temp_path} -out {output_file}")
-            .run()
-            .with_context(|| format!("Converting {} to man page", path.display()))?;
+            cmd!(sh, "go-md2man -in {temp_path} -out {output_file}")
+                .run()
+                .with_context(|| format!("Converting {} to man page", path.display()))?;
 
-        // Clean up temporary file
-        fs::remove_file(&temp_path)?;
+            // Clean up temporary file
+            fs::remove_file(&temp_path)?;
 
-        println!("Generated {}", output_file);
+            println!("Generated {}", output_file);
+        }
     }
 
     // Apply post-processing fixes for apostrophe handling
@@ -495,9 +500,9 @@ pub fn update_manpages(sh: &Shell) -> Result<()> {
     // Check each command and create man page if missing
     for command_parts in commands_to_check {
         let filename = if command_parts.len() == 1 {
-            format!("bootc-{}.md", command_parts[0])
+            format!("bootc-{}.8.md", command_parts[0])
         } else {
-            format!("bootc-{}.md", command_parts.join("-"))
+            format!("bootc-{}.8.md", command_parts.join("-"))
         };
 
         let filepath = format!("docs/src/man/{}", filename);
@@ -511,14 +516,31 @@ pub fn update_manpages(sh: &Shell) -> Result<()> {
                 let command_name_full = format!("bootc {}", command_parts.join(" "));
                 let command_description = cmd.about.as_deref().unwrap_or("TODO: Add description");
 
+                // Generate SYNOPSIS line with proper arguments
+                let mut synopsis = format!("**{}** \\[*OPTIONS...*\\]", command_name_full);
+
+                // Add positional arguments
+                for positional in &cmd.positionals {
+                    if positional.required {
+                        synopsis.push_str(&format!(" <*{}*>", positional.name.to_uppercase()));
+                    } else {
+                        synopsis.push_str(&format!(" \\[*{}*\\]", positional.name.to_uppercase()));
+                    }
+                }
+
+                // Add subcommand if this command has subcommands
+                if !cmd.subcommands.is_empty() {
+                    synopsis.push_str(" <*SUBCOMMAND*>");
+                }
+
                 let template = format!(
                     r#"# NAME
 
 {} - {}
 
 # SYNOPSIS
 
-**{}** [*OPTIONS*]
+{}
 
 # DESCRIPTION
 
@@ -549,19 +571,19 @@ TODO: Add practical examples showing how to use this command.
                 std::fs::write(&filepath, template)
                     .with_context(|| format!("Writing template to {}", filepath))?;
 
-                println!("✓ Created man page template: {}", filepath);
+                println!("Created man page template: {}", filepath);
                 created_count += 1;
             }
         }
     }
 
     if created_count > 0 {
-        println!("✓ Created {} new man page templates", created_count);
+        println!("Created {} new man page templates", created_count);
     } else {
-        println!("✓ All commands already have man pages");
+        println!("All commands already have man pages");
     }
 
-    println!("🔄 Syncing OPTIONS sections...");
+    println!("Syncing OPTIONS sections...");
     sync_all_man_pages(sh)?;
 
     println!("Man pages updated.");
@@ -585,6 +607,13 @@ fn apply_man_page_fixes(sh: &Shell, dir: &Utf8Path) -> Result<()> {
             .and_then(|s| s.to_str())
             .map_or(false, |e| e.chars().all(|c| c.is_numeric()))
         {
+            // Check if the file already has the fix applied
+            let content = fs::read_to_string(&path)?;
+            if content.starts_with(".ds Aq \\(aq\n") {
+                // Already fixed, skip
+                continue;
+            }
+
             // Apply the same sed fixes as before
             let groffsub = r"1i .ds Aq \\(aq";
             let dropif = r"/\.g \.ds Aq/d";

crates/xtask/src/xtask.rs

@@ -36,9 +36,6 @@ fn main() {
 #[allow(clippy::type_complexity)]
 const TASKS: &[(&str, fn(&Shell) -> Result<()>)] = &[
     ("manpages", man::generate_man_pages),
-    ("sync-manpages", man::sync_all_man_pages),
-    ("test-sync-manpages", man::test_sync_workflow),
-    ("update-json-schemas", update_json_schemas),
     ("update-generated", update_generated),
     ("package", package),
     ("package-srpm", package_srpm),
@@ -47,17 +44,21 @@ const TASKS: &[(&str, fn(&Shell) -> Result<()>)] = &[
 ];
 
 fn try_main() -> Result<()> {
-    // Ensure our working directory is the toplevel
+    // Ensure our working directory is the toplevel (if we're in a git repo)
     {
-        let toplevel_path = Command::new("git")
+        if let Ok(toplevel_path) = Command::new("git")
             .args(["rev-parse", "--show-toplevel"])
             .output()
-            .context("Invoking git rev-parse")?;
-        if !toplevel_path.status.success() {
-            anyhow::bail!("Failed to invoke git rev-parse");
+        {
+            if toplevel_path.status.success() {
+                let path = String::from_utf8(toplevel_path.stdout)?;
+                std::env::set_current_dir(path.trim()).context("Changing to toplevel")?;
+            }
+        }
+        // Otherwise verify we're in the toplevel
+        if !Utf8Path::new("ADOPTERS.md").try_exists()? {
+            anyhow::bail!("Not in toplevel")
         }
-        let path = String::from_utf8(toplevel_path.stdout)?;
-        std::env::set_current_dir(path.trim()).context("Changing to toplevel")?;
     }
 
     let task = std::env::args().nth(1);
@@ -393,11 +394,20 @@ fn update_json_schemas(sh: &Shell) -> Result<()> {
     Ok(())
 }
 
-/// Update generated files using the new sync approach
+/// Update all generated files
+/// This is the main command developers should use to update generated content.
+/// It handles:
+/// - Creating new man page templates for new commands
+/// - Syncing CLI options to existing man pages
+/// - Updating JSON schema files
 #[context("Updating generated files")]
 fn update_generated(sh: &Shell) -> Result<()> {
+    // Update man pages (create new templates + sync options)
     man::update_manpages(sh)?;
+
+    // Update JSON schemas
     update_json_schemas(sh)?;
+
     Ok(())
 }
 

docs/src/man/bootc-container-lint.8.md

@@ -5,7 +5,7 @@ checks as part of a container build
 
 # SYNOPSIS
 
-**bootc container lint** [*OPTIONS...*]
+**bootc container lint** \[*OPTIONS...*\]
 
 # DESCRIPTION
 
@@ -24,34 +24,22 @@ part of a build process; it will error if any problems are detected.
 
     Default: /
 
-**--fatal-warnings**=*FATAL_WARNINGS*
+**--fatal-warnings**
 
     Make warnings fatal
 
-    Possible values:
-    - true
-    - false
-
-**--list**=*LIST*
+**--list**
 
     Instead of executing the lints, just print all available lints. At the current time, this will output in YAML format because it's reasonably human friendly. However, there is no commitment to maintaining this exact format; do not parse it via code or scripts
 
-    Possible values:
-    - true
-    - false
-
 **--skip**=*SKIP*
 
     Skip checking the targeted lints, by name. Use `--list` to discover the set of available lints
 
-**--no-truncate**=*NO_TRUNCATE*
+**--no-truncate**
 
     Don't truncate the output. By default, only a limited number of entries are shown for each lint, followed by a count of remaining entries
 
-    Possible values:
-    - true
-    - false
-
 <!-- END GENERATED OPTIONS -->
 
 # VERSION