instructions: document AZL component-modification workflow conventions

.github/copilot-instructions.md

@@ -71,6 +71,14 @@ When making a PR, the lock and rendered specs must be updated and self-consisten
 
 Run all commands from the repo root (where `azldev.toml` lives). If the terminal's cwd has drifted, use `azldev -C /path/to/repo <command>`. Use `azldev --help` and `azldev <command> --help` for current syntax — the tool is under active development.
 
+**Installing / updating `azldev`:** the canonical install/update command is
+
+```bash
+go install github.com/microsoft/azure-linux-dev-tools/cmd/azldev@main
+```
+
+`@main` pins to the latest published source — fine for an actively-tracked dev tool. The binary lands at `$(go env GOPATH)/bin/azldev`. Verify with `azldev --version`. Re-run the same command to update; new features (e.g. project-config validation rules) land continuously, so running stale `azldev` against a fresh repo state can produce false-positive CI failures.
+
 **Agent-friendly flags:** Use `-q` (quiet) to reduce noise and `-O json` for machine-parseable output. These are global flags and work on all commands.
 
 | Task | Command |
@@ -102,3 +110,8 @@ Run all commands from the repo root (where `azldev.toml` lives). If the terminal
 3. **Schema validation**: The authoritative schema is `external/schemas/azldev.schema.json`. Do NOT add `$schema` keys to TOML files — `$` is invalid at the start of a bare TOML key.
 4. **Don't edit generated output**: Build artifacts in `base/out/` and `base/build/` are generated (configured by `output-dir`, `log-dir`, `work-dir` in `base/project.toml`) — never edit them directly. Note: `prep-sources -o <dir>` writes to a user-specified directory, separate from these project output dirs.
 5. **Mandatory testing**: Any change that affects a component's output (overlays, build config, spec edits, version bumps, new components) MUST be validated by building AND testing the resulting RPMs. A successful build alone is not sufficient — smoke-test in a mock chroot. Pure organizational changes (moving definitions between files, editing descriptions/comments) do not require rebuild. See `AGENTS.md` for the full testing protocol.
+6. **Commit signing**: Every commit MUST be GPG-signed (`git commit -S`, or `commit.gpgsign = true` in your git config). Verify with `git log -1 --show-signature` — the output must include a `Good signature` line. Do **not** disable signing (`--no-gpg-sign`) even for fix-branch / amend / force-push workflows; if a commit fails due to a signing-tool issue, surface the error rather than working around it.
+7. **Public-content hygiene**: In committed content — overlay `description`s, `replace-reason` strings, `*.comp.toml` comments, `*.spec` files, `modify_source.sh` echo lines, commit messages, PR descriptions — describe motivations **technically and neutrally**. Do not name specific Microsoft-internal infrastructure (signing services, scanner brand names, internal pipeline names, internal CLI wrappers, internal Azure tenants, etc.). Use neutral phrasing like "automated package-signing pipeline", "FS-aware deep scanner", "automated malware scan". See [`comp-toml.instructions.md` — Public-content hygiene](instructions/comp-toml.instructions.md#public-content-hygiene) for examples.
+8. **Component-modification order of operations**: For any component change, the canonical sequence is `edit comp.toml → azldev comp update -p <name> → commit ALL working-tree changes (comp.toml + lock + anything else like source-side scripts) → azldev comp render -p <name> → amend (rendered spec)`. The **commit between `update` and `render` is mandatory** and must include the entire working tree, not just the lock and comp.toml: `%changelog` / `Release:` are derived from `git log` for the component, and `azldev` treats any uncommitted working-tree change as a synthetic "dirty" commit. Rendering with uncommitted changes therefore produces an extra `Release:` bump and a `Local changes (uncommitted)` changelog entry that diverges from the actual commit — the `Check Rendered Specs` CI check then fails. Commit first; render after; amend the rendered spec into the same single commit. See [`skill-update-component`](skills/skill-update-component/SKILL.md).
+9. **Single-commit PRs**: Each PR should land as a single commit. If you produce intermediate commits during development (e.g., separate the source override from a follow-up simplification), squash them into one GPG-signed commit before opening or marking the PR ready for review. Use `git reset --soft <merge-base>` followed by `git commit -S -F <message>` rather than `--squash` so the commit metadata stays clean. A multi-commit PR will fail the merge bar.
+10. **Reviewer-facing-only commit/PR descriptions**: Commit messages and pull-request descriptions MUST describe **only changes that are visible in the diff** to a reviewer. Do NOT include narrative about investigations, findings, side-observations, or background that did not produce any file changes — that context belongs in the working-session log, internal notes, or a follow-up issue. For example, a commit that strips one file from a tarball must not include a section about an unrelated `gzip: stdin: not in gzip format` warning the author noticed while investigating, because there's no diff for a reviewer to evaluate against it. If an investigation produced a follow-up issue or TODO, link to it from the commit message ("filed as #N for follow-up") — the link itself is the diff-visible artefact. Commit/PR descriptions are reviewer-facing artefacts that explain *what this change does*, not *what the author thought about while writing it*. See [`skill-update-component`](skills/skill-update-component/SKILL.md).

.github/instructions/comp-toml.instructions.md

@@ -148,6 +148,75 @@ Optional fields that apply to multiple types: `section` (target spec section), `
 - **Dedicated** (`<name>/<name>.comp.toml`): when overlays, build config, or local spec are needed
 - Rule of thumb: if it's more than `[components.<name>]`, give it a dedicated file
 - `components.toml` has `includes = ["**/*.comp.toml"]` — dedicated files are picked up automatically
+- **When moving a component out of inline `components.toml` into a dedicated `<name>.comp.toml` file, DELETE the inline entry.** Don't leave a "moved to X" pointer comment. Discovery is automatic via the `**/*.comp.toml` include glob.
+- **When removing a component outright, DELETE the inline entry.** No tombstone comments.
+
+## `replace-upstream` Source Override
+
+When a component must serve a **locally-modified** Source0 (or any other source) under the **same filename** as the upstream sources manifest declares, use a `[[components.<name>.source-files]]` block with `replace-upstream = true`. This swaps the same-named entry in the Fedora `sources` manifest **in place** during render — there is **no** separate `file-remove` overlay needed against the `sources` file, and no need to invent a new filename.
+
+```toml
+[[components.examplepkg.source-files]]
+filename = "examplepkg-1.2.3.tar.xz"
+hash = "<sha512 of LOCALLY-MODIFIED tarball>"
+hash-type = "SHA512"
+replace-upstream = true
+replace-reason = "Upstream tarball ships a vendored copy of a third-party library plus a 'tests/network' tree that trip an automated package-signing pipeline's deep scanner. Repacked under the same filename with those subtrees stripped via base/comps/examplepkg/modify_source.sh; the surviving bits are byte-identical to upstream so the build is unaffected."
+
+[components.examplepkg.source-files.origin]
+uri = "https://azltempstaginglookaside.blob.core.windows.net/repo/pkgs_modified/examplepkg/examplepkg-1.2.3.tar.xz/sha512/<same-sha512-as-hash-field>/examplepkg-1.2.3.tar.xz"
+```
+
+### Required field semantics
+
+| Field | Notes |
+|-------|-------|
+| `filename` | Must match the upstream Source0/N filename exactly. That's how the override is keyed. |
+| `hash` | SHA-512 of the **locally-modified** tarball. Lowercase hex. |
+| `hash-type` | `"SHA512"`. |
+| `replace-upstream` | `true`. Tells render to swap the same-named entry in the upstream manifest instead of appending a new one. |
+| `replace-reason` | Single TOML string (basic or literal, not triple-quoted/multi-line). Must self-contain the full WHY so no TOML comment is needed in addition. See "`replace-reason` style" below. |
+| `origin.uri` | Lookaside URL. The `$hash` path segment MUST be the **same SHA-512** as the `hash` field. |
+
+### Lookaside URL pattern
+
+```
+https://azltempstaginglookaside.blob.core.windows.net/repo/pkgs_modified/$pkg/$filename/$hashtype/$hash/$filename
+```
+
+- `$pkg` = component name (e.g., `examplepkg`)
+- `$filename` = upstream Source0 filename (appears twice)
+- `$hashtype` = `sha512` (lowercase in the URL even though the TOML field is `"SHA512"`)
+- `$hash` = same SHA-512 as `hash`. If they ever diverge, the upload at one URL won't be findable from the other.
+
+### `replace-reason` style
+
+- A **single-line TOML string** (basic `"..."` or literal `'...'`). No triple-quoted / multi-line strings.
+- Self-contained: must include *what* was changed and *why* clearly enough that no separate TOML comment is needed.
+- Use **neutral, public-safe wording** for the motivation. Phrases like "automated package-signing pipeline", "FS-aware deep scanner", "automated malware scan" are fine. Do **not** name specific internal Microsoft scanners, pipelines, CLIs, tenants, or wrappers. Describe the *shape* of the bad content (what subtrees / what scanner class flagged it), not the brand of the tool.
+- One short banner-style line at the very top of the `*.comp.toml` summarizing the override is fine. **Multi-paragraph header comments are not** — keep the explanation inside `replace-reason`.
+
+### What render emits
+
+Render emits an audit `WARN`-level log entry naming the override and the from/to SHA-512 pair. That is **expected and desired** — it makes overrides discoverable in render output.
+
+### How the modified tarball is produced
+
+Use a `modify_source.sh` script alongside the `*.comp.toml`. The script MUST be byte-deterministic (same input → same hash, across machines and re-runs), or the lookaside URL and the `hash` field will drift on every re-pack. The canonical pattern is documented in [`skill-modify-source`](../skills/skill-modify-source/SKILL.md).
+
+## Public-content hygiene
+
+In committed content — `*.comp.toml` files (overlay `description`, `replace-reason`, etc.), local `*.spec` files, `modify_source.sh` scripts, commit messages, PR descriptions — describe motivations **technically and neutrally**. Do not name specific Microsoft-internal infrastructure (signing services, scanner brand names, internal pipeline names, internal CLI wrappers, internal Azure tenants, etc.) by name.
+
+Use neutral phrasing instead:
+
+| Avoid | Prefer |
+|-------|--------|
+| Brand name of an internal signing service | "automated package-signing pipeline" |
+| Brand name of an internal malware scanner | "FS-aware deep scanner", "automated malware scan" |
+| Names of internal CI/CD pipelines or wrappers | "the build pipeline", or just `azldev` for tool references |
+
+The technical *what* (the shape of the content that's being changed, the class of scanner that flagged it, the nature of the false positive) belongs in the description. The internal *brand* does not.
 
 ## Build Configuration
 
@@ -165,6 +234,21 @@ with = ["feature_x"]
 without = ["plugin_rhsm"]
 ```
 
+### Build-flag overrides: don't duplicate distro-level disablement
+
+Several build flags are applied across many components via shared **disablement groups** rather than per-component `[components.<name>.build]` blocks. Before adding a `build.without` / `build.with` / `build.defines` override to a `*.comp.toml`, check whether the same flag is already applied at one of these layers — duplicating it in the per-component file is redundant and creates two sources of truth that can drift.
+
+Check these layers, in order:
+
+1. **`base/comps/component-mingw-disablement.toml`** — applies `build.without = ["mingw"]` to every component listed under `[component-groups.mingw-disabled]` via the group's `default-component-config.build` block. Azure Linux does not ship mingw cross-compilation toolchains; any component whose upstream spec has a `mingw` bcond should be added to that list, **not** carry its own `without = ["mingw"]`.
+2. **`base/comps/component-check-disablement.toml`** — applies `build.check = { skip = true, ... }` to components listed under `[component-groups.check-skip-initial-failures]` (initial-bringup `%check` failures). Don't duplicate the check skip in a per-component file.
+3. **Any other `component-*-disablement.toml`** under `base/comps/` — the pattern is `[component-groups.<group-name>.default-component-config.build]`.
+4. **`distro/azurelinux.distro.toml`** — `[distros.azurelinux.versions.'<ver>'.default-component-config]` blocks set distro-wide defaults that every component inherits unless explicitly overridden.
+
+If the desired build flag is already applied at any of those layers, **do not duplicate it in the per-component file**. If you're moving a component from an inline entry in `components.toml` into a dedicated `<name>.comp.toml` and the original inline entry had no `build` block, the dedicated file should also not have one — let the group / distro defaults apply.
+
+Conversely, only add a per-component `[components.<name>.build]` block when the flag genuinely diverges from what the disablement groups and distro defaults already provide (e.g., a `with` that isn't shared by other components, a `defines` specific to this package).
+
 ## Release Configuration
 
 By default (`release.calculation = "auto"`), `azldev` auto-calculates the `Release` tag during rendering. There are four modes:

.github/skills/skill-fix-overlay/SKILL.md

@@ -55,6 +55,33 @@ The regex doesn't match anything in the spec. Causes:
 
 The spec section (`%prep`, `%build`, `%install`, etc.) doesn't exist or has different casing. Check the actual section names in `<pre-dir>/<name>.spec`.
 
+### `spec-append-lines` / `spec-prepend-lines`: lines land on the wrong section
+
+Multi-section types like `%files`, `%description`, `%package` appear MANY times in a typical spec (one per subpackage). An unscoped `section = "%files"` append silently lands on the FIRST `%files` (often the unnamed main `%files`) — not on the section you visually wrote it "next to". Always scope by `package = "<subpackage>"` (or `package = ""` for the unnamed/main section if that's what you really want) when targeting `%files` / `%description` / `%package`.
+
+Example — the wrong way (lands on the unnamed main `%files`):
+```toml
+{ type = "spec-append-lines", section = "%files", lines = ["%if %{have_libblkio}"] }
+```
+The right way (lands at the end of `%files common`, which is the desired anchor):
+```toml
+{ type = "spec-append-lines", section = "%files", package = "common", lines = ["%if %{have_libblkio}"] }
+```
+
+### `spec-remove-subpackage` / `spec-remove-section`: orphan `%endif` / vanished macros
+
+`spec-remove-subpackage` (and `spec-remove-section` for `%description` / `%files`) walks forward from the matched directive to the *next* section directive and deletes everything in between. The engine does NOT understand `%if` / `%else` / `%endif` as structural elements. Two failure modes follow:
+
+1. **Greedy consumption of `%if` guards.** If an `%if %{have_FOO}` line sits between the removed section and the next subpackage's directive (a common Fedora pattern where the next subpackage is gated on a bcond), that `%if` is swallowed too. The matching `%endif` further down is left orphaned and `rpmspec -P` fails with `%endif with no %if`.
+
+   Fix: re-append the `%if %{have_FOO}` line via `spec-append-lines` against the now-trailing section. Use `package = "..."` to anchor the append at the right `%description` / `%files` block (see the previous failure mode).
+
+2. **`%define` / `%global` inside the removed body vanishes.** If a macro is `%define`d inside the removed `%package` body but referenced from `%install` or `%build`, the reference becomes undefined after the removal. Hoist an equivalent declaration to the top of the spec via `spec-search-replace` (anchor on an existing `%global ...` line you can find).
+
+3. **Stray buildroot files.** If `%install` writes files that the removed `%files <subpackage>` used to claim, RPM fails with "Installed (but unpackaged) file(s)". Either edit `%install` to gate the writes (`spec-search-replace`) or append `rm -rf` / `rm -f` lines to `%install` (`spec-append-lines`) that delete the orphaned files from `%{buildroot}`.
+
+See `microsoft/azurelinux#17212` for a worked example that combines all three.
+
 ### `file-*`: file not found
 
 The file doesn't exist in the upstream sources. Check `ls <pre-dir>/` for actual filenames. Globs (`**/*`) are supported for `file-search-replace`.