build: generate the coverage report without exuberant-ctags - #407
Merged
Merged
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
`make cov` extracted the ICU function lists with `ctags -x --c-kinds=fp`.
The `--c-kinds` flag is specific to exuberant-ctags, so the report could
not be generated with the ctags that most systems ship, and could not be
generated at all where no ctags is installed.
Replace the ctags call with `build/icu_c_functions.awk`. awk is in POSIX,
so nothing extra has to be installed. The extraction keys on how ICU
declares its C API rather than on parsing C: every public function is
introduced by a line holding U_EXPORT2, with the name either trailing on
that line or starting the next one. Keying on U_EXPORT2 rather than on
U_CAPI also covers the older U_STABLE, U_DRAFT and U_DEPRECATED
spellings.
The script also called `icu-config`, which ICU removed in favor of
pkg-config and which distributions no longer ship, so the script failed
on its first line on a current system. Fall back to pkg-config when
icu-config is absent, and report a usable error when neither is found.
Two smaller fixes the above surfaced:
* The detail section of the report was opened with `>>`, so a run
following one that died partway through appended to the leftovers.
Truncate it.
* Report an error when a header yields no functions, rather than
recording 0 / 0 coverage, so a future change to how ICU spells its
declarations is visible instead of silent.
The awk extraction agrees with ctags exactly. Running the old and the new
script against ICU 74 produces byte-identical output over all 40 files in
coverage/:
$ diff -r old/coverage new/coverage && echo IDENTICAL
IDENTICAL
The checked-in report changes because it was generated against a newer
ICU, whose ucol.h carries a C++-only block that ctags miscounted as three
C functions named `match`, `operator` and `Predicate`. Those were listed
as unimplemented ICU API. The awk extraction skips them, so ucol.h goes
from 8 / 54 to 8 / 51.
Verified with `cargo check --workspace` (clean) and by running
`build/showprogress.sh` with neither ctags nor icu-config installed.
filmil
force-pushed
the
fix-81-hermetic-coverage
branch
from
August 13, 2026 09:23
97ee3fa to
5ae976c
Compare
Member
Author
|
The google-cla remarks is incorrect. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #81
Root cause
make covrunsbuild/showprogress.sh, which extracted the ICU function lists with:all=$(ctags -x --c-kinds=fp $header_fullname | ...)--c-kindsis specific to exuberant-ctags. Other ctags implementations reject it, and where no ctags is installed the report cannot be generated at all.While confirming this, I found the script fails even earlier on a current system. Line 31 called
icu-config, which ICU removed in favor of pkg-config and which distributions no longer ship:So the script aborted on its first real command, before reaching the ctags call.
The change
build/icu_c_functions.awkreplaces the ctags call. awk is in POSIX, so nothing extra has to be installed.It keys on how ICU declares its C API rather than on parsing C. Every public function is introduced by a line holding
U_EXPORT2, with the name either starting the next line:or trailing on the same one:
Keying on
U_EXPORT2rather than onU_CAPIalso covers the olderU_STABLE,U_DRAFTandU_DEPRECATEDspellings, which take the same shape. Function pointer typedefs useU_CALLCONVinstead and are skipped. ICU's handful of C++ convenience wrappers, declaredinline <Type>with the name on the next line, are still reported, because ctags reported them.The other changes:
icu-configis absent. icu-config is still tried first, since a hand-built ICU installs it and it reports that installation, which is whatbuild/Dockerfile.maintsets up. Without this the script cannot run at all on a current system, so the ctags fix could not be verified.exuberant-ctagsfrombuild/Dockerfile.buildenv.gawkis already installed there.>>, so a run following one that died partway through appended to the leftovers. Easy to hit, since the script currently always dies partway through.0 / 0. The extraction keys on how ICU spells its declarations, so a future change to that should be loud rather than silent.grep -v U_DEFINEfilter. It existed because ctags reportedU_DEFINE_LOCAL_OPEN_POINTERmacro invocations as functions. The awk extraction does not pick them up (checked on ICU 74 and ICU 77). Keeping it would have been actively harmful: underset -o pipefail,grep -vexits 1 on empty input, which killed the script silently and made the new "no functions found" check unreachable.Verification
Ran on Ubuntu 24.04, ICU 74.2, with neither ctags nor icu-config installed.
The awk extraction agrees with ctags exactly. Running the old script and the new script against the same headers produces byte-identical output over all 40 files in
coverage/:Per header, against both ctags implementations (
exuberant-ctags 5.9,universal-ctags 5.9, which agree with each other on all 18):Also checked against 10 ICU 77 headers, to confirm the extraction is not overfitted to the ICU version installed here. It matches ctags on 362 symbols, with three differences, all of which are ctags misparses that the awk extraction correctly skips:
These are C++ class members in a
U_SHOW_CPLUSPLUS_APIblock, not ICU C API. This is whycoverage/report.mdchanges in this PR: the checked-in report was generated against a newer ICU and listsmatch,operatorandPredicateas unimplemented ICU functions.ucol.hgoes from8 / 54to8 / 51. That is the only content change to the checked-in report; no real function appears or disappears.Checked with mawk 1.3.4, the default
awkon Debian and Ubuntu. gawk and busybox awk were not installed here, so I did not run them; the script uses no extension beyond POSIX awk (match,substr,sub,RSTART,RLENGTH).Error paths:
Workspace check:
No crate sources changed, so there is no affected crate to run
cargo testagainst. Nothing under.github/workflows/is touched.Generated by Claude Code