Skip to content

Commit c12eca3

Browse files
committed
ci: Hard require that generated code is updated in CI
`just validate` now verifies that generated code is updated. We got burned badly by someone editing the generated code, and not noticing until the release PR which reverted the change. While I was working on this, I wanted to preserve the property that e.g. ostree and other heavier dependencies aren't required on the host system - they're only part of the container build. So using `podman build --output`, we can build generated docs etc as part of the container build, and copy them out to the host in the Justfile. Assisted-by: OpenCode (Claude Sonnet 4.6) Signed-off-by: Colin Walters <walters@verbum.org>
1 parent 333ae30 commit c12eca3

10 files changed

Lines changed: 356 additions & 92 deletions

File tree

.dockerignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
# Toplevel build bits
99
!Makefile
1010
!Cargo.*
11+
# cargo alias config (defines `cargo xtask` shorthand used in Dockerfile stages)
12+
!.cargo/config.toml
1113
# License and doc files needed for RPM
1214
!LICENSE-*
1315
!README.md

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,6 @@ bootc.tar.zst
55

66
# Added by cargo
77
/target
8+
9+
# Git worktrees
10+
.worktrees/

Dockerfile

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,24 @@ RUN --network=none --mount=type=tmpfs,target=/run --mount=type=tmpfs,target=/tmp
185185
FROM buildroot as validate
186186
RUN --network=none --mount=type=tmpfs,target=/run --mount=type=tmpfs,target=/tmp --mount=type=cache,target=/src/target --mount=type=cache,target=/var/roothome make validate
187187

188+
FROM validate as validate-post-build
189+
RUN --network=none --mount=type=tmpfs,target=/run --mount=type=tmpfs,target=/tmp --mount=type=cache,target=/src/target --mount=type=cache,target=/var/roothome \
190+
cargo xtask update-generated from-code --check
191+
192+
# Stage for updating generated docs files (man pages, JSON schemas) on the host.
193+
# Usage: podman build --target update-generated-from-code --output type=local,dest=. .
194+
# This runs `from-code` inside the container (where ostree is available) and
195+
# exports only the generated files back to the host working directory.
196+
FROM buildroot as update-generated-from-code
197+
RUN --network=none --mount=type=tmpfs,target=/run --mount=type=tmpfs,target=/tmp --mount=type=cache,target=/src/target --mount=type=cache,target=/var/roothome \
198+
cargo xtask update-generated from-code
199+
# Export only the generated docs — not the entire container filesystem.
200+
# Glob patterns here automatically pick up any new schemas added to JSON_SCHEMAS
201+
# in crates/xtask/src/xtask.rs without requiring Dockerfile changes.
202+
FROM scratch as update-generated-from-code-output
203+
COPY --from=update-generated-from-code /src/docs/src/man/ /docs/src/man/
204+
COPY --from=update-generated-from-code /src/docs/src/*.schema.json /docs/src/
205+
188206
# ----
189207
# Stages for the final image
190208
# ----

Justfile

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,10 +169,11 @@ test-upgrade *ARGS: build _build-upgrade-source-image
169169
"${composefs_args[@]}" \
170170
{{upgrade_source_img}} {{ARGS}} readonly
171171

172-
# Run cargo fmt and clippy checks in container
172+
# Run all validation checks: tmt plan staleness (local), then fmt/clippy/man/schema (container)
173173
[group('core')]
174174
validate:
175-
podman build {{base_buildargs}} --target validate .
175+
cargo xtask update-generated direct --check
176+
podman build {{base_buildargs}} --target validate-post-build .
176177

177178
# Test container export via Anaconda liveimg install in a QEMU VM
178179
[group('testing')]
@@ -291,9 +292,12 @@ pullspec-for-os TYPE NAME:
291292
# ============================================================================
292293

293294
# Update generated files (man pages, JSON schemas)
295+
# tmt plans are updated directly; man pages + JSON schemas are regenerated
296+
# inside a container (so ostree is available) and written back via --output.
294297
[group('maintenance')]
295298
update-generated:
296-
cargo run -p xtask update-generated
299+
cargo xtask update-generated direct
300+
podman build {{base_buildargs}} --target update-generated-from-code-output --output type=local,dest=. .
297301

298302
# Remove all locally-built test container images
299303
[group('maintenance')]

Makefile

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,9 +129,6 @@ fix-rust:
129129
cargo clippy --fix --allow-dirty -- $(CLIPPY_CONFIG)
130130
.PHONY: fix-rust
131131

132-
update-generated:
133-
cargo xtask update-generated
134-
.PHONY: update-generated
135132

136133
vendor:
137134
cargo xtask $@

crates/xtask/src/man.rs

Lines changed: 171 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -174,20 +174,19 @@ fn format_options_as_markdown(options: &[CliOption], positionals: &[CliPositiona
174174
result
175175
}
176176

177-
/// Update markdown file with generated subcommands
178-
pub fn update_markdown_with_subcommands(
177+
/// Compute what `docs/src/man/<file>` should look like after regenerating its subcommands section.
178+
/// Returns `None` if the file has no subcommands marker (nothing to do).
179+
fn compute_markdown_with_subcommands(
179180
markdown_path: &Utf8Path,
181+
content: &str,
180182
subcommands: &[CliCommand],
181183
parent_path: &[&str],
182-
) -> Result<()> {
183-
let content =
184-
fs::read_to_string(markdown_path).with_context(|| format!("Reading {}", markdown_path))?;
185-
184+
) -> Result<Option<String>> {
186185
let begin_marker = "<!-- BEGIN GENERATED SUBCOMMANDS -->";
187186
let end_marker = "<!-- END GENERATED SUBCOMMANDS -->";
188187

189188
let Some((before, rest)) = content.split_once(begin_marker) else {
190-
return Ok(()); // Skip files without markers
189+
return Ok(None); // Skip files without markers
191190
};
192191

193192
let Some((_, after)) = rest.split_once(end_marker) else {
@@ -202,34 +201,25 @@ pub fn update_markdown_with_subcommands(
202201
// Trim trailing whitespace from before section and ensure exactly one blank line
203202
let before = before.trim_end();
204203

205-
let new_content = format!(
204+
Ok(Some(format!(
206205
"{}\n\n{}\n{}{}{}",
207206
before, begin_marker, generated_subcommands, end_marker, after
208-
);
209-
210-
// Only write if content has changed to avoid updating mtime unnecessarily
211-
if new_content != content {
212-
fs::write(markdown_path, new_content)
213-
.with_context(|| format!("Writing to {}", markdown_path))?;
214-
println!("Updated subcommands in {}", markdown_path);
215-
}
216-
Ok(())
207+
)))
217208
}
218209

219-
/// Update markdown file with generated options
220-
pub fn update_markdown_with_options(
210+
/// Compute what `docs/src/man/<file>` should look like after regenerating its options section.
211+
/// Returns `None` if the file has no options marker (nothing to do).
212+
fn compute_markdown_with_options(
221213
markdown_path: &Utf8Path,
214+
content: &str,
222215
options: &[CliOption],
223216
positionals: &[CliPositional],
224-
) -> Result<()> {
225-
let content =
226-
fs::read_to_string(markdown_path).with_context(|| format!("Reading {}", markdown_path))?;
227-
217+
) -> Result<Option<String>> {
228218
let begin_marker = "<!-- BEGIN GENERATED OPTIONS -->";
229219
let end_marker = "<!-- END GENERATED OPTIONS -->";
230220

231221
let Some((before, rest)) = content.split_once(begin_marker) else {
232-
return Ok(()); // Skip files without markers
222+
return Ok(None); // Skip files without markers
233223
};
234224

235225
let Some((_, after)) = rest.split_once(end_marker) else {
@@ -256,6 +246,48 @@ pub fn update_markdown_with_options(
256246
format!("{}\n\n{}\n{}{}", before, begin_marker, end_marker, after)
257247
};
258248

249+
Ok(Some(new_content))
250+
}
251+
252+
/// Update markdown file with generated subcommands
253+
pub fn update_markdown_with_subcommands(
254+
markdown_path: &Utf8Path,
255+
subcommands: &[CliCommand],
256+
parent_path: &[&str],
257+
) -> Result<()> {
258+
let content =
259+
fs::read_to_string(markdown_path).with_context(|| format!("Reading {}", markdown_path))?;
260+
261+
let Some(new_content) =
262+
compute_markdown_with_subcommands(markdown_path, &content, subcommands, parent_path)?
263+
else {
264+
return Ok(());
265+
};
266+
267+
// Only write if content has changed to avoid updating mtime unnecessarily
268+
if new_content != content {
269+
fs::write(markdown_path, new_content)
270+
.with_context(|| format!("Writing to {}", markdown_path))?;
271+
println!("Updated subcommands in {}", markdown_path);
272+
}
273+
Ok(())
274+
}
275+
276+
/// Update markdown file with generated options
277+
pub fn update_markdown_with_options(
278+
markdown_path: &Utf8Path,
279+
options: &[CliOption],
280+
positionals: &[CliPositional],
281+
) -> Result<()> {
282+
let content =
283+
fs::read_to_string(markdown_path).with_context(|| format!("Reading {}", markdown_path))?;
284+
285+
let Some(new_content) =
286+
compute_markdown_with_options(markdown_path, &content, options, positionals)?
287+
else {
288+
return Ok(());
289+
};
290+
259291
// Only write if content has changed to avoid updating mtime unnecessarily
260292
if new_content != content {
261293
fs::write(markdown_path, new_content)
@@ -611,6 +643,121 @@ TODO: Add practical examples showing how to use this command.
611643
Ok(())
612644
}
613645

646+
/// Check that all man page markdown files are up to date.
647+
/// Fails with an error if any file would change, similar to `cargo fmt --check`.
648+
#[context("Checking man pages")]
649+
pub fn check_manpages(sh: &Shell) -> Result<()> {
650+
let cli_structure = extract_cli_json(sh)?;
651+
652+
// First: check no man pages are missing
653+
fn collect_commands(cmd: &CliCommand, path: Vec<String>, acc: &mut Vec<Vec<String>>) {
654+
for sub in &cmd.subcommands {
655+
let mut sub_path = path.clone();
656+
sub_path.push(sub.name.clone());
657+
acc.push(sub_path.clone());
658+
collect_commands(sub, sub_path, acc);
659+
}
660+
}
661+
let mut commands_to_check = Vec::new();
662+
collect_commands(&cli_structure, Vec::new(), &mut commands_to_check);
663+
for command_parts in &commands_to_check {
664+
let filename = format!("bootc-{}.8.md", command_parts.join("-"));
665+
let filepath = Utf8Path::new("docs/src/man").join(&filename);
666+
if !filepath.exists() {
667+
anyhow::bail!(
668+
"{} is missing; run `cargo xtask update-generated` to create it",
669+
filepath
670+
);
671+
}
672+
}
673+
674+
let mappings = discover_man_page_mappings(&cli_structure)?;
675+
676+
for (filename, subcommand_path) in mappings {
677+
let markdown_path = Utf8Path::new("docs/src/man").join(&filename);
678+
if !markdown_path.exists() {
679+
continue;
680+
}
681+
682+
let target_cmd = if let Some(ref path) = subcommand_path {
683+
let path_refs: Vec<&str> = path.iter().map(|s| s.as_str()).collect();
684+
find_subcommand(&cli_structure, &path_refs)
685+
.ok_or_else(|| anyhow::anyhow!("Subcommand {:?} not found", path))?
686+
} else {
687+
&cli_structure
688+
};
689+
690+
let content = fs::read_to_string(&markdown_path)
691+
.with_context(|| format!("Reading {}", markdown_path))?;
692+
693+
if content.contains("<!-- BEGIN GENERATED OPTIONS -->") {
694+
check_markdown_options(
695+
&markdown_path,
696+
&content,
697+
&target_cmd.options,
698+
&target_cmd.positionals,
699+
)?;
700+
}
701+
if content.contains("<!-- BEGIN GENERATED SUBCOMMANDS -->") {
702+
let parent_path: Vec<&str> = if let Some(path) = &subcommand_path {
703+
path.iter().map(|s| s.as_str()).collect()
704+
} else {
705+
vec![]
706+
};
707+
check_markdown_subcommands(
708+
&markdown_path,
709+
&content,
710+
&target_cmd.subcommands,
711+
&parent_path,
712+
)?;
713+
}
714+
}
715+
716+
Ok(())
717+
}
718+
719+
/// Compare-only variant of `update_markdown_with_options`.
720+
fn check_markdown_options(
721+
markdown_path: &Utf8Path,
722+
content: &str,
723+
options: &[CliOption],
724+
positionals: &[CliPositional],
725+
) -> Result<()> {
726+
let Some(new_content) =
727+
compute_markdown_with_options(markdown_path, content, options, positionals)?
728+
else {
729+
return Ok(());
730+
};
731+
if new_content != content {
732+
anyhow::bail!(
733+
"{} is out of date; run `cargo xtask update-generated` to update it",
734+
markdown_path
735+
);
736+
}
737+
Ok(())
738+
}
739+
740+
/// Compare-only variant of `update_markdown_with_subcommands`.
741+
fn check_markdown_subcommands(
742+
markdown_path: &Utf8Path,
743+
content: &str,
744+
subcommands: &[CliCommand],
745+
parent_path: &[&str],
746+
) -> Result<()> {
747+
let Some(new_content) =
748+
compute_markdown_with_subcommands(markdown_path, content, subcommands, parent_path)?
749+
else {
750+
return Ok(());
751+
};
752+
if new_content != content {
753+
anyhow::bail!(
754+
"{} is out of date; run `cargo xtask update-generated` to update it",
755+
markdown_path
756+
);
757+
}
758+
Ok(())
759+
}
760+
614761
/// Apply post-processing fixes to generated man pages
615762
#[context("Fixing man pages")]
616763
fn apply_man_page_fixes(sh: &Shell, dir: &Utf8Path) -> Result<()> {

0 commit comments

Comments
 (0)