nprasad2077/doris

0 Fork 0

mirror of https://github.com/apache/doris.git synced 2026-11-04 07:31:17 +00:00

Files

T

Mingyu Chen (Rayner)andClaude Fable 5 7933a82bdd [opt](build) Enforce BE compile-time hygiene at configure time (#66901 )

### What problem does this PR solve?

Related PR: #66510 (the BE build-time series, #66615..#66826, that this
PR locks in)

Problem Summary:

The build-time series cut BE cold builds by ~49% and took
`olap_common.h` / `uid_util.h` off the everything-rebuilds line. Every
one of those wins can regress **silently**: a stray include, a lost
`extern template`, a header added to the PCH — the code still compiles,
only every build afterwards is slower, and nothing reports it. `#66052`
already showed the failure mode: the layering rules in
`check-header-deps.py` only worked because the author happened to
maintain them by hand; nothing in CI runs them.

This PR turns each known regression vector into a **fail-loud,
seconds-level gate that runs at configure time**, so a violation is a
build error on every path that builds the BE (CI, `build.sh`, the UT
build) with zero extra CI wiring — the same mounting pattern the FE side
already uses for its architecture gates (`exec-maven-plugin` running
`check-fe-core-metadata-funnel.sh` at `validate`).

**Checks** (pure text scans, ~1.3s combined, zero build dependency, zero
false positives on current master):

| Gate | Regression it stops |
|---|---|
| Header layering rules (30, +6 new) | a hub header (e.g. `exec_env.h`,
`function.h`, `pch.h`) re-growing an include edge into a subsystem that
was deliberately cut |
| Third-party ban table (new) |
`fmt`/`boost`/`concurrentqueue.h`/`<ranges>` returning to headers whose
bodies were moved out of line to contain template-instantiation cost
(`<ranges>` in a src header also breaks the `-fno-access-control` UT
build on libc++) |
| Two-axis budgets (new) | the edges nobody names: light hubs must stay
light (include-closure budget, zero slack), heavy payloads must not
spread (transitive-reach baseline +10%); `--closure` / `--reach`
subcommands rank the offending edges for the fix |
| pch whitelist (new) | any quoted include added to `pch/pch.h` — the
single most leveraged regression in the repo (touch one such header and
the whole backend plus the PCH rebuild) |
| `check-extern-template-pairing.py` (new) | the one **silent**
extern-template failure: a `.cpp` keeps the explicit instantiation but
the header loses the `extern` declaration, so every TU quietly
re-instantiates; also the loud direction (declaration with no
definition), reported at once instead of at link |
| `check-unity-skip-coverage.py` (new) | a test `#include`-ing a src
`.cpp` that is not opted out of unity batching — reported at configure
with the exact skip entry to add, instead of as duplicate symbols at the
end of the BE UT link |

`build-support/check-build-hygiene.sh` runs all three scripts;
`be/CMakeLists.txt` executes it at configure time. Escape hatches, by
design:

- `-DENABLE_BUILD_HYGIENE=OFF` (default ON; a missing `python3` is a
loud FATAL naming this switch, never a silent skip);
- every budget/whitelist/rule table lives in the scripts themselves, so
the legal way past a gate is a **one-line reviewed diff in the same PR**
— the gate is never a dead end. Every failure message carries the
mechanism (why this edge is expensive) and the concrete fix path.

**Found by the new gates on day one, fixed here:** `core/types.h` has
carried five `extern template struct fmt::formatter<Decimal*>;`
declarations with **no matching definition anywhere** since #53483 —
every member is defined in-class, so the linker never noticed; deleted.
Also made the FE funnel self-test's `sed -i` invocation portable to
BSD/macOS.

Blind spot, by design: instantiations expanded from macros (e.g.
`DECLARE_OPERATOR` in `operator.cpp`) are invisible to the pairing scan
on both sides — macro bodies are skipped; a comment at the site says so.

`be/README.md` (new) documents the gates, what to do when each fires,
and the `*_fwd.h` convention as the sanctioned way through a layering
barrier.

The same rules are encoded for AI-assisted development and review:
`be/AGENTS.md` (new — include discipline while writing BE code + review
checkpoints), `build-support/AGENTS.md` (new — maintenance discipline
for the gate scripts themselves), an include-src-cpp checkpoint in
`be/test/AGENTS.md`, and a Header Hygiene entry in the root `AGENTS.md`,
so coding agents and the review pipeline apply them automatically.

### Release note

None

### Check List (For Author)

- Test
    - [x] Manual test (add detailed scripts or steps below)
- `build-support/check-build-hygiene.sh` on master: all green, ~1.3s,
zero false positives.
- Self-tests `bash build-support/tests/run.sh` (7/7 PASS): red/green
injection per gate family — each seeded violation turns red with the
mechanism + fix in the message, restoring the file turns it green again.
- Real configure on macOS arm64: green path passes the gate section;
with an injected violation, configure dies at the gate in under a second
with the full message; `-DENABLE_BUILD_HYGIENE=OFF` bypasses; a missing
`python3` is a loud exit-2 FATAL naming the switch.
- This PR's own Linux CI exercises the configure mount on the second
platform.

- Behavior changed:
- [x] Yes. BE configure now fails fast (with mechanism + fix in the
message) on header-hygiene violations; emergency bypass via
`-DENABLE_BUILD_HYGIENE=OFF`. No runtime behavior change — the only
source-code changes are comments and the deletion of five never-defined
`extern template` declarations.

- Does this need documentation?
- [x] No. Developer-facing docs are included in-repo (`be/README.md`).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01EyZL631YTeuNXa9WLjvk7s

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

2026-08-19 10:07:58 +08:00

3.9 KiB

Raw Blame History

AGENTS.md — Doris BE

Scope: be/. This file is operational — what to do while developing or reviewing BE C++ code so the compile-time invariants hold. The gate mechanics and the *_fwd.h convention are documented in be/README.md (read it when a gate fires). Repository-wide rules (commit format, build/test commands, clang-format/clang-tidy) live in the root AGENTS.md; nothing here overrides them.

Machine-Checked Obligations (configure-time gates)

BE configure runs build-support/check-build-hygiene.sh (option ENABLE_BUILD_HYGIENE, default ON; scripts and self-tests live in build-support/ and build-support/tests/). It is pure text and takes about a second — run it directly after any change touching BE headers, includes, template instantiations, unity skip lists, or test files that include src sources; do not wait for configure or CI to tell you.

Every failure message carries the mechanism and the fix path. The legal way past a gate is never to bypass it, but one of:

fix the edge as the message says (forward-declare + include in the .cpp, or route declarations through a *_fwd.h);
when the change is deliberate, edit the corresponding table (rule exception set / ANGLE_BANS / FORWARD_CLOSURE_BUDGETS / REVERSE_REACH_BASELINES / PCH_QUOTED_WHITELIST / ALLOW) in the same commit and justify it in the commit message — the table diff is the review signal.

Rules while writing BE code

Adding an #include to a widely-included (hub) header is a design decision, not a convenience. Everything a hub includes is reparsed by every TU behind it (~1000 TUs for exec_env.h, thread_context.h, runtime_state.h, function.h, dependency.h, column.h, ...). Prefer forward declarations; put the real include in the .cpp. If many files need the declarations, use a *_fwd.h (declarations and lightweight aliases only — never bodies or non-fwd project includes).
Never add a quoted include to pch/pch.h. Every header on the PCH rebuilds the whole backend (plus the PCH itself) when touched; it is the single most leveraged regression surface in the repo.
Keep extern template families paired. An explicit instantiation in a .cpp needs the matching extern template in the header, spelled with the same template arguments — a missing extern compiles and links fine and just silently re-instantiates in every TU. Instantiations expanded from macros (e.g. DECLARE_OPERATOR in operator.cpp) are invisible to the pairing gate on both sides: keep those in sync by hand.
A test that #includes a be/src .cpp needs that file opted out of unity batching via doris_skip_unity_inclusion in the owning be/src/.../CMakeLists.txt; otherwise the BE UT link fails with duplicate symbols an hour later. The gate error names the exact entry to add.
Do not re-add banned third-party includes (fmt/boost/ concurrentqueue.h/<ranges>) to the headers listed in ANGLE_BANS: their bodies were deliberately moved out of line, and <ranges> in a src header additionally breaks the -fno-access-control UT build on libc++.

Review checkpoints (AI review and self-review)

New includes in hub headers: could a forward declaration or *_fwd.h carry this instead? Does the PR pay a closure/reach budget bump — and if so, does the commit message justify it?
Any edit to a gate table (RULES exceptions, budgets, whitelist, ALLOW) must be deliberate, minimal, and explained in the same commit; an unexplained table edit is a red flag, not a fix.
Any change to pch/pch.h is near-always wrong; demand the reasoning.
New explicit instantiation lists or extern template blocks: both sides present, same spelling? New test #include of a src .cpp: skip entry present?
git grep for a deleted/renamed header in skip lists and gate tables: stale entries fail configure loudly — fix them in the same PR.

3.9 KiB Raw Blame History

AGENTS.md — Doris BE

Machine-Checked Obligations (configure-time gates)

Rules while writing BE code

Review checkpoints (AI review and self-review)

3.9 KiB

Raw Blame History