Skip to content

Commit 0288b50

Browse files
committed
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>
1 parent f6c400d commit 0288b50

13 files changed

+118
-240
lines changed

Makefile

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,9 @@ all: bin manpages
1515
bin:
1616
cargo build --release --features "$(CARGO_FEATURES)"
1717

18-
# Generate man pages from markdown sources
19-
MAN5_SOURCES := $(wildcard docs/src/man/*.5.md)
20-
MAN8_SOURCES := $(wildcard docs/src/man/*.8.md)
21-
TARGETMAN := target/man
22-
MAN5_TARGETS := $(patsubst docs/src/man/%.5.md,$(TARGETMAN)/%.5,$(MAN5_SOURCES))
23-
MAN8_TARGETS := $(patsubst docs/src/man/%.8.md,$(TARGETMAN)/%.8,$(MAN8_SOURCES))
24-
25-
$(TARGETMAN)/%.5: docs/src/man/%.5.md
26-
@mkdir -p $(TARGETMAN)
27-
go-md2man -in $< -out $@
28-
29-
$(TARGETMAN)/%.8: docs/src/man/%.8.md
30-
@mkdir -p $(TARGETMAN)
31-
go-md2man -in $< -out $@
32-
33-
manpages: $(MAN5_TARGETS) $(MAN8_TARGETS)
18+
.PHONY: manpages
19+
manpages:
20+
cargo xtask manpages
3421

3522
STORAGE_RELATIVE_PATH ?= $(shell realpath -m -s --relative-to="$(prefix)/lib/bootc/storage" /sysroot/ostree/bootc/storage)
3623
install:

crates/lib/src/cli_json.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ pub struct CliOption {
1313
pub help: String,
1414
pub possible_values: Vec<String>,
1515
pub required: bool,
16+
pub is_boolean: bool,
1617
}
1718

1819
/// Representation of a CLI command for JSON export
@@ -73,16 +74,27 @@ pub fn command_to_json(cmd: &Command) -> CliCommand {
7374

7475
let help = arg.get_help().map(|h| h.to_string()).unwrap_or_default();
7576

77+
// For boolean flags, don't show a value name
78+
// Boolean flags use SetTrue or SetFalse actions and don't take values
79+
let is_boolean = matches!(
80+
arg.get_action(),
81+
clap::ArgAction::SetTrue | clap::ArgAction::SetFalse
82+
);
83+
let value_name = if is_boolean {
84+
None
85+
} else {
86+
arg.get_value_names()
87+
.and_then(|names| names.first())
88+
.map(|s| s.to_string())
89+
};
90+
7691
options.push(CliOption {
7792
long: arg
7893
.get_long()
7994
.map(String::from)
8095
.unwrap_or_else(|| id.to_string()),
8196
short: arg.get_short().map(|c| c.to_string()),
82-
value_name: arg
83-
.get_value_names()
84-
.and_then(|names| names.first())
85-
.map(|s| s.to_string()),
97+
value_name,
8698
default: arg
8799
.get_default_values()
88100
.first()
@@ -91,6 +103,7 @@ pub fn command_to_json(cmd: &Command) -> CliCommand {
91103
help,
92104
possible_values,
93105
required: arg.is_required_set(),
106+
is_boolean,
94107
});
95108
}
96109
}

crates/xtask/src/man.rs

Lines changed: 53 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ pub struct CliOption {
2626
pub possible_values: Vec<String>,
2727
/// Whether the option is required
2828
pub required: bool,
29+
/// Whether this is a boolean flag
30+
pub is_boolean: bool,
2931
}
3032

3133
/// Represents a CLI command from the JSON dump
@@ -132,16 +134,17 @@ fn format_options_as_markdown(options: &[CliOption], positionals: &[CliPositiona
132134
// Add long flag
133135
flag_line.push_str(&format!("**--{}**", opt.long));
134136

135-
// Add value name if option takes argument
137+
// Add value name if option takes argument (but not for boolean flags)
138+
// Boolean flags are detected by having no value_name (set to None in cli_json.rs)
136139
if let Some(value_name) = &opt.value_name {
137140
flag_line.push_str(&format!("=*{}*", value_name));
138141
}
139142

140143
result.push_str(&format!("{}\n\n", flag_line));
141144
result.push_str(&format!(" {}\n\n", opt.help));
142145

143-
// Add possible values for enums
144-
if !opt.possible_values.is_empty() {
146+
// Add possible values for enums (but not for boolean flags)
147+
if !opt.possible_values.is_empty() && !opt.is_boolean {
145148
result.push_str(" Possible values:\n");
146149
for value in &opt.possible_values {
147150
result.push_str(&format!(" - {}\n", value));
@@ -191,10 +194,12 @@ pub fn update_markdown_with_subcommands(
191194
before, begin_marker, generated_subcommands, end_marker, after
192195
);
193196

194-
fs::write(markdown_path, new_content)
195-
.with_context(|| format!("Writing to {}", markdown_path))?;
196-
197-
println!("Updated subcommands in {}", markdown_path);
197+
// Only write if content has changed to avoid updating mtime unnecessarily
198+
if new_content != content {
199+
fs::write(markdown_path, new_content)
200+
.with_context(|| format!("Writing to {}", markdown_path))?;
201+
println!("Updated subcommands in {}", markdown_path);
202+
}
198203
Ok(())
199204
}
200205

@@ -238,10 +243,12 @@ pub fn update_markdown_with_options(
238243
format!("{}\n\n{}\n{}{}", before, begin_marker, end_marker, after)
239244
};
240245

241-
fs::write(markdown_path, new_content)
242-
.with_context(|| format!("Writing to {}", markdown_path))?;
243-
244-
println!("Updated {}", markdown_path);
246+
// Only write if content has changed to avoid updating mtime unnecessarily
247+
if new_content != content {
248+
fs::write(markdown_path, new_content)
249+
.with_context(|| format!("Writing to {}", markdown_path))?;
250+
println!("Updated {}", markdown_path);
251+
}
245252
Ok(())
246253
}
247254

@@ -374,21 +381,6 @@ pub fn sync_all_man_pages(sh: &Shell) -> Result<()> {
374381
Ok(())
375382
}
376383

377-
/// Test the sync workflow
378-
pub fn test_sync_workflow(sh: &Shell) -> Result<()> {
379-
println!("🧪 Testing man page sync workflow...");
380-
381-
// Create a backup of current files
382-
let test_dir = "target/test-sync";
383-
sh.create_dir(test_dir)?;
384-
385-
// Run sync
386-
sync_all_man_pages(sh)?;
387-
388-
println!("✅ Sync workflow test completed successfully");
389-
Ok(())
390-
}
391-
392384
/// Generate man pages from hand-written markdown sources
393385
pub fn generate_man_pages(sh: &Shell) -> Result<()> {
394386
let man_src_dir = Utf8Path::new("docs/src/man");
@@ -432,18 +424,31 @@ pub fn generate_man_pages(sh: &Shell) -> Result<()> {
432424
let content = fs::read_to_string(&path)?;
433425
let content_with_version = content.replace("<!-- VERSION PLACEHOLDER -->", &version);
434426

435-
// Create temporary file with version-replaced content
436-
let temp_path = format!("{}.tmp", path.display());
437-
fs::write(&temp_path, content_with_version)?;
427+
// Check if we need to regenerate by comparing input and output modification times
428+
let should_regenerate = if let (Ok(input_meta), Ok(output_meta)) =
429+
(fs::metadata(&path), fs::metadata(&output_file))
430+
{
431+
input_meta.modified().unwrap_or(std::time::UNIX_EPOCH)
432+
> output_meta.modified().unwrap_or(std::time::UNIX_EPOCH)
433+
} else {
434+
// If output doesn't exist or we can't get metadata, regenerate
435+
true
436+
};
437+
438+
if should_regenerate {
439+
// Create temporary file with version-replaced content
440+
let temp_path = format!("{}.tmp", path.display());
441+
fs::write(&temp_path, content_with_version)?;
438442

439-
cmd!(sh, "go-md2man -in {temp_path} -out {output_file}")
440-
.run()
441-
.with_context(|| format!("Converting {} to man page", path.display()))?;
443+
cmd!(sh, "go-md2man -in {temp_path} -out {output_file}")
444+
.run()
445+
.with_context(|| format!("Converting {} to man page", path.display()))?;
442446

443-
// Clean up temporary file
444-
fs::remove_file(&temp_path)?;
447+
// Clean up temporary file
448+
fs::remove_file(&temp_path)?;
445449

446-
println!("Generated {}", output_file);
450+
println!("Generated {}", output_file);
451+
}
447452
}
448453

449454
// Apply post-processing fixes for apostrophe handling
@@ -495,9 +500,9 @@ pub fn update_manpages(sh: &Shell) -> Result<()> {
495500
// Check each command and create man page if missing
496501
for command_parts in commands_to_check {
497502
let filename = if command_parts.len() == 1 {
498-
format!("bootc-{}.md", command_parts[0])
503+
format!("bootc-{}.8.md", command_parts[0])
499504
} else {
500-
format!("bootc-{}.md", command_parts.join("-"))
505+
format!("bootc-{}.8.md", command_parts.join("-"))
501506
};
502507

503508
let filepath = format!("docs/src/man/{}", filename);
@@ -549,19 +554,19 @@ TODO: Add practical examples showing how to use this command.
549554
std::fs::write(&filepath, template)
550555
.with_context(|| format!("Writing template to {}", filepath))?;
551556

552-
println!("Created man page template: {}", filepath);
557+
println!("Created man page template: {}", filepath);
553558
created_count += 1;
554559
}
555560
}
556561
}
557562

558563
if created_count > 0 {
559-
println!("Created {} new man page templates", created_count);
564+
println!("Created {} new man page templates", created_count);
560565
} else {
561-
println!("All commands already have man pages");
566+
println!("All commands already have man pages");
562567
}
563568

564-
println!("🔄 Syncing OPTIONS sections...");
569+
println!("Syncing OPTIONS sections...");
565570
sync_all_man_pages(sh)?;
566571

567572
println!("Man pages updated.");
@@ -585,6 +590,13 @@ fn apply_man_page_fixes(sh: &Shell, dir: &Utf8Path) -> Result<()> {
585590
.and_then(|s| s.to_str())
586591
.map_or(false, |e| e.chars().all(|c| c.is_numeric()))
587592
{
593+
// Check if the file already has the fix applied
594+
let content = fs::read_to_string(&path)?;
595+
if content.starts_with(".ds Aq \\(aq\n") {
596+
// Already fixed, skip
597+
continue;
598+
}
599+
588600
// Apply the same sed fixes as before
589601
let groffsub = r"1i .ds Aq \\(aq";
590602
let dropif = r"/\.g \.ds Aq/d";

crates/xtask/src/xtask.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@ fn main() {
3636
#[allow(clippy::type_complexity)]
3737
const TASKS: &[(&str, fn(&Shell) -> Result<()>)] = &[
3838
("manpages", man::generate_man_pages),
39-
("sync-manpages", man::sync_all_man_pages),
40-
("test-sync-manpages", man::test_sync_workflow),
41-
("update-json-schemas", update_json_schemas),
4239
("update-generated", update_generated),
4340
("package", package),
4441
("package-srpm", package_srpm),
@@ -393,11 +390,20 @@ fn update_json_schemas(sh: &Shell) -> Result<()> {
393390
Ok(())
394391
}
395392

396-
/// Update generated files using the new sync approach
393+
/// Update all generated files
394+
/// This is the main command developers should use to update generated content.
395+
/// It handles:
396+
/// - Creating new man page templates for new commands
397+
/// - Syncing CLI options to existing man pages
398+
/// - Updating JSON schema files
397399
#[context("Updating generated files")]
398400
fn update_generated(sh: &Shell) -> Result<()> {
401+
// Update man pages (create new templates + sync options)
399402
man::update_manpages(sh)?;
403+
404+
// Update JSON schemas
400405
update_json_schemas(sh)?;
406+
401407
Ok(())
402408
}
403409

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

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -24,34 +24,22 @@ part of a build process; it will error if any problems are detected.
2424

2525
Default: /
2626

27-
**--fatal-warnings**=*FATAL_WARNINGS*
27+
**--fatal-warnings**
2828

2929
Make warnings fatal
3030

31-
Possible values:
32-
- true
33-
- false
34-
35-
**--list**=*LIST*
31+
**--list**
3632

3733
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
3834

39-
Possible values:
40-
- true
41-
- false
42-
4335
**--skip**=*SKIP*
4436

4537
Skip checking the targeted lints, by name. Use `--list` to discover the set of available lints
4638

47-
**--no-truncate**=*NO_TRUNCATE*
39+
**--no-truncate**
4840

4941
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
5042

51-
Possible values:
52-
- true
53-
- false
54-
5543
<!-- END GENERATED OPTIONS -->
5644

5745
# VERSION

docs/src/man/bootc-edit.8.md

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,10 @@ Only changes to the `spec` section are honored.
2626

2727
Use filename to edit system specification
2828

29-
**--quiet**=*QUIET*
29+
**--quiet**
3030

3131
Don't display progress
3232

33-
Possible values:
34-
- true
35-
- false
36-
3733
<!-- END GENERATED OPTIONS -->
3834

3935
# VERSION

0 commit comments

Comments
 (0)