Skip to content

Commit 101fa54

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 101fa54

9 files changed

Lines changed: 355 additions & 91 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

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

0 commit comments

Comments
 (0)