docs: update overlay instructions with lessons from xen removal#16479
docs: update overlay instructions with lessons from xen removal#16479christopherco wants to merge 1 commit intotomls/base/mainfrom
Conversation
Add spec-remove-section to overlay types table and mark it as a non-exhaustive quick reference pointing to upstream docs at azure-linux-dev-tools as the canonical source. Add new rows to the choosing-right-type table: - build.without for bcond toggles - spec-remove-section for removing subpackages - file-search-replace for source config files Add pitfalls learned from practical overlay work: - spec-search-replace cannot match section tags - file-search-replace targets source files, not specs - -n packages need unexpanded macros in package field - Blank lines from empty replacements break continuations - Replacement string escaping differs from regex Add builder environment note to copilot-instructions.md documenting that %fedora is set on builders and its implications for feature gating.
There was a problem hiding this comment.
Pull request overview
This PR updates the repository’s overlay documentation to incorporate practical guidance learned during Xen removal work, and adds a note to the Copilot instructions about build-time macro behavior on Koji builders.
Changes:
- Extend overlay documentation with additional overlay-type guidance and new “pitfalls” notes.
- Add
build.without/file-search-replacedecision guidance to reduce use of fragile regex overlays. - Document
%fedoramacro implications in the Koji builder environment in Copilot instructions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/instructions/comp-toml.instructions.md |
Expands overlay type tables and adds practical pitfalls/decision guidance for choosing overlays. |
.github/copilot-instructions.md |
Adds a builder-environment note describing %fedora being defined on Koji builders and how that affects spec conditionals. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| | `spec-append-lines` | Insert at end of section body | `section`, `lines` | | ||
| | `spec-search-replace` | Regex replace in spec (**last resort**) | `regex`, `replacement` | | ||
| | `spec-remove-section` | Remove an entire section (header + body); **fails if section doesn't exist** | `section`, optionally `package` | | ||
| | `patch-add` | Copy a patch file into sources and register it in the spec (`PatchN` or `%patchlist`) | `source`, optionally `file` | | ||
| | `patch-remove` | Remove patch files and their spec references matching a glob | `file` | |
There was a problem hiding this comment.
spec-remove-section is documented here as an available overlay type, but the repo’s checked-in schema (external/schemas/azldev.schema.json) does not include spec-remove-section in the ComponentOverlay.type enum. As-is, users following this doc will likely hit config validation errors; either update the schema/tooling in this repo to include it or clearly note that this overlay type is only available in newer upstream azldev versions (and how to verify availability).
| | Remove a patch | `patch-remove` with glob (e.g., `file = "CVE-*.patch"`) | regex to delete PatchN line + `file-remove` | | ||
| | Target a sub-package's `%files` | `spec-append-lines` with `section = "%files"`, `package = "devel"` | regex scoped to a section | | ||
| | Disable a build feature with bcond toggle | `build.without = ["feature"]` | regex to modify conditional | | ||
| | Remove an entire subpackage | `spec-remove-section` (×3: `%package`, `%description`, `%files`) | commenting out or `%if 0` wrapping | |
There was a problem hiding this comment.
This guidance recommends spec-remove-section for removing subpackages, but spec-remove-section is not present in the repo’s current azldev.schema.json overlay type enum. Until the schema/tooling is updated, consider recommending an overlay approach that is actually supported here, or add an explicit note that this requires a newer azldev/schema than what’s currently vendored in this repo.
| | Remove an entire subpackage | `spec-remove-section` (×3: `%package`, `%description`, `%files`) | commenting out or `%if 0` wrapping | | |
| | Remove an entire subpackage | commenting out or `%if 0` wrapping of `%package`, `%description`, and `%files` | `spec-remove-section` can do this in newer `azldev` schema/tooling, but it is not in the vendored schema in this repo | |
| - **Do NOT use inline array syntax for overlays.** Write each overlay as a separate `[[components.<name>.overlays]]` block (array-of-tables), not as `overlays = [{ ... }, { ... }]`. The inline form is valid TOML but harder to read and review. Some older components in the repo use the inline style — don't copy it. | ||
| - **No `$schema` in TOML.** `$` is invalid at the start of a bare TOML key. | ||
| - **Scope regex overlays with `section` and `package`.** When using `spec-search-replace`, always set `section` (e.g. `"%files"`, `"%install"`) and `package` (e.g. `"foo"` for a `%files foo` section) to limit where the regex matches if possible. The `package` value is the **short sub-package suffix** as it appears after the section tag in the spec (e.g. `%files foo` → `package = "foo"`, not `package = "mypkg-foo"`). Unscoped regex overlays risk matching unintended lines elsewhere in the spec, especially after upstream updates. If the overlay targets a specific sub-package's `%files` section, both fields should be set. | ||
| - **`spec-search-replace` cannot match section tags.** `%package`, `%description`, `%files`, and other section headers are structural elements parsed by the spec engine, not matchable text. To remove an entire subpackage, use `spec-remove-section` with `section` and `package` fields — one overlay per section type (`%package`, `%description`, `%files`). | ||
| - **`file-search-replace` targets source files, not the spec.** It operates on files alongside the spec (Source0, Source1, etc. and loose files like build configs) — NOT on the `.spec` file itself. Use `spec-search-replace` for spec modifications. | ||
| - **Scope regex overlays with `section` and `package`.** When using `spec-search-replace`, always set `section` (e.g. `"%files"`, `"%install"`) and `package` (e.g. `"foo"` for a `%files foo` section) to limit where the regex matches if possible. The `package` value is the **short sub-package suffix** as it appears after the section tag in the spec (e.g. `%files foo` → `package = "foo"`, not `package = "mypkg-foo"`). For specs using `-n` naming (e.g. `%package -n %{name}+foo`), use the **unexpanded macro form**: `package = "%{name}+foo"`. Unscoped regex overlays risk matching unintended lines elsewhere in the spec, especially after upstream updates. If the overlay targets a specific sub-package's `%files` section, both fields should be set. |
There was a problem hiding this comment.
This pitfall section directs readers to use spec-remove-section, but that overlay type is not present in the repo’s current external/schemas/azldev.schema.json ComponentOverlay.type enum. Either align the documentation with the schema, or add a version/availability note so readers don’t assume it’s supported in this repo’s azldev version.
|
|
||
| **TOML include hierarchy**: `azldev.toml` → `distro/distro.toml` + `base/project.toml` → `base/comps/components.toml` → `**/*.comp.toml` (stitched into single namespace). | ||
|
|
||
| **Builder environment**: Our Koji builders run Fedora, so the `%fedora` macro IS defined during builds. Upstream Fedora specs that gate features on `%{defined fedora}` or `0%{?fedora}` will have those features **enabled** unless explicitly overridden via overlays or `build.defines`. This is the most common reason packages produce unwanted subpackages (e.g., Xen, Flatpak-specific features). When investigating unwanted build output, always check for `%fedora`-gated conditionals in the spec first. |
There was a problem hiding this comment.
Not quite true that it's defined because we run Fedora in the builders. (Will follow up on this offline.)
There was a problem hiding this comment.
ACK - discussed offline. Will amend this instruction to be more precise about only the fedora macro being set in the build environment, without stating that we run Fedora in the builder machines, because that's not true.
Add spec-remove-section to overlay types table and mark it as a non-exhaustive quick reference pointing to upstream docs at azure-linux-dev-tools as the canonical source.
Add new rows to the choosing-right-type table:
Add pitfalls learned from practical overlay work:
Add builder environment note to copilot-instructions.md documenting that %fedora is set on builders and its implications for feature gating.