Skip to content

ci: share the build setup through a composite action - #579

Closed
somethingwithproof wants to merge 2 commits into
Cacti:developfrom
somethingwithproof:ci/dry-workflows
Closed

ci: share the build setup through a composite action#579
somethingwithproof wants to merge 2 commits into
Cacti:developfrom
somethingwithproof:ci/dry-workflows

Conversation

@somethingwithproof

Copy link
Copy Markdown
Member

Every job that builds Spine carried its own copy of the same apt-get install list and the same ./bootstrap && ./configure pair: four in ci.yml, one in fuzz.yml. Adding a dependency meant finding all five. The setup moves into .github/actions/build-spine, parameterised by extra packages and configure flags.

This is not a line-count win. The workflows lose 42 lines and the action adds 60. What changes is that there is now one place to edit.

mysql-server comes out of the package set. configure.ac looks for mysql.h and libmysqlclient.*, both of which libmysqlclient-dev provides, and no job in the repository starts a server or connects to one. It was installed in four jobs and used by none.

The pins had split. actions/checkout was on 3d3c42e (v7.0.1) in five places and 11bd719 (v4.2.2, October 2024) in three; upload-artifact was on v7.0.1 and v4.6.2. Dependabot moved everything to v7 in #560, and #563, #570 and #577 then merged with older SHAs copied in. Only one of the eleven pins carried a version comment, so nothing about the diff made the drift visible. All pins are now uniform and commented.

The second commit is separate because it changes behaviour rather than structure. The sanitizer job built the whole tree under ASan and UBSan and then ran ./spine --version and ./spine --help, which reaches almost none of the poller. It now runs make check instrumented, and keeps the smoke test after it.

Verified with actionlint (clean before and after) and by executing the action's argument construction for all four call shapes:

distcheck:   ./configure
build:       ./configure [--enable-warnings]
coverage:    ./configure [CFLAGS=-g -O0 --coverage] [LDFLAGS=--coverage]
sanitizers:  ./configure [CFLAGS=-fsanitize=address,undefined -fno-omit-frame-pointer -g -O1] [LDFLAGS=-fsanitize=address,undefined]

Two things need the CI run rather than local checking: that the reduced package set still builds on the runner, and what make check finds under the sanitizers. If the sanitizer job comes back red, that is the point of the change.

Every build job repeated the same apt-get install and bootstrap/configure
pair, so a new dependency meant editing five places. The setup now lives in
.github/actions/build-spine.

mysql-server is dropped from the package set: configure only looks for
mysql.h and libmysqlclient, both from libmysqlclient-dev, and no job starts
or connects to a server.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
The sanitizer build only ran --version and --help, which exercises almost
none of the poller. make check is what drives the code, so it is what the
sanitizers should watch.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof

Copy link
Copy Markdown
Member Author

Consolidated into #597, which carries this branch's commits unchanged.

Every pair of these ten conflicted on tests/unit/test_linked.c because each appended to the same registration array, so whichever merged first would have forced a rebase on the other nine. #597 is one review and one approval for the same 20 commits, still one logical change each.

Nothing here is dropped. Reopen this if you would rather review it separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant