Skip to content

feat(workflows): step 7, add shared Makefile.common and drift check - #48

Open
seantronsen wants to merge 1 commit into
seantronsen/feat/runner-selectionfrom
seantronsen/feat/makefile-drift-avoidance
Open

seantronsen wants to merge 1 commit into
seantronsen/feat/runner-selectionfrom
seantronsen/feat/makefile-drift-avoidance

Conversation

@seantronsen

@seantronsen seantronsen commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Description

Important

This is a first draft at deduplicating Makefile recipes across the org's repos. I have notes on possible additions that I'll look at later. In the meantime, if you have questions or think something's missing, let me know.

Adds a shared Makefile.common for OpenCHAMI repos and a reusable workflow that catches drift from it.

  • Makefile.common: common targets (Go build/test/lint/vuln, Fabrica codegen, containers, GoReleaser packages, REUSE, deps, GNU-style install/uninstall, help, clean) and standard variable names. Repos vendor it at their root, set overrides before include Makefile.common, and keep repo-only targets in their own Makefile. all, deps, build, run, install, and uninstall are double-colon rules so repos can extend them. make build embeds build info into <module>/internal/version (Version, Tag, Branch, Commit, Date, GoVersion, GitState, BuildHost, BuildUser); GO_VERSION_PACKAGE or GO_LDFLAGS overrides it. Targets that don't apply skip (no go.mod, no Fabrica, no Containerfile); GoReleaser targets fail without a config. GNU make 3.81 compatible.
  • lint-makefile-common.yml: reusable workflow, no inputs. Resolves this repo's latest release to its commit SHA, fetches Makefile.common at that SHA, and fails with a diff if the caller's copy differs.
  • README: entries for both, plus adoption instructions and a GoReleaser ldflags snippet that sets the same internal/version variables, so release binaries and make build report the same build info.

Stacked on #47.

Downstream adoption is separate work and changes target/variable names in adopting repos (e.g. test-coverage → coverage, rpm-build → build-packages, install (dependencies) → deps, BINARY_NAME → PROJECT/BINARIES, main.version ldflags → internal/version).

Testbed: OpenCHAMI/github-actions-testbed#11.

Checklist

  • My code follows the style guidelines of this project
  • I have added/updated comments where needed
  • I have added tests that prove my fix is effective or my feature works
  • I have run make test (or equivalent) locally and all tests pass
  • I have updated the relevant documentation (CLI examples, man pages, README, other docs, etc.)
  • DCO Sign-off: All commits are signed off (git commit -s) with my real name and email
  • REUSE Compliance:
    • Each new/modified source file has SPDX copyright and license headers
    • Any non-commentable files include a <filename>.license sidecar
    • All referenced licenses are present in the LICENSES/ directory

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Dependency update
  • Build system/CI

@seantronsen
seantronsen changed the base branch from main to seantronsen/feat/runner-selection October 14, 2026 14:56
@seantronsen
seantronsen added this pull request to stack #40 October 14, 2026 14:56
@seantronsen seantronsen changed the title draft: makefile drift avoidance. feat: add Makefile.common and lint-makefile-common drift check Oct 14, 2026
@seantronsen seantronsen changed the title feat: add Makefile.common and lint-makefile-common drift check feat(workflows): step 7, add shared Makefile.common and drift check Oct 14, 2026
@seantronsen

seantronsen commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

See OpenCHAMI/github-actions-testbed#11 for test results.

@seantronsen seantronsen self-assigned this Oct 15, 2026
@seantronsen
seantronsen marked this pull request as ready for review October 15, 2026 18:13
@seantronsen
seantronsen requested review from alexlovelltroy and synackd and removed request for alexlovelltroy October 15, 2026 18:13
@seantronsen
seantronsen force-pushed the seantronsen/feat/makefile-drift-avoidance branch from e953668 to d453b0d Compare October 19, 2026 16:27
@seantronsen seantronsen added this to the v4.0.0 milestone Oct 20, 2026
@seantronsen
seantronsen force-pushed the seantronsen/feat/makefile-drift-avoidance branch from d453b0d to fc66a0e Compare November 2, 2026 21:08
@seantronsen
seantronsen force-pushed the seantronsen/feat/makefile-drift-avoidance branch from fc66a0e to e8ad694 Compare November 2, 2026 21:34
@seantronsen
seantronsen force-pushed the seantronsen/feat/makefile-drift-avoidance branch 2 times, most recently from 809a949 to c220fbf Compare November 3, 2026 16:32
@synackd

synackd commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

@seantronsen I'd like to split this PR before it merges. The work is good and I want to build on it, but in hindsight the approach should have started as an RFD. I've opened OpenCHAMI/roadmap#143 to work out the approach.

Why not merge it and hold off on adoption? Merging here means shipping in v4.0.0 (due Nov 23): #45's release notes announce Makefile.common, and the docs tell repos to adopt it. "Don't adopt yet" wouldn't be written down anywhere, and the design is unlikely to settle by then.

What needs discussion first:

  • No org-wide contract exists yet. install has four meanings across repos, and deps, build-packages, reuse-check, and lint-ci appear in 0 of 28 root Makefiles. reuse is in 10 and rpm-build in 8.
  • Double-colon targets can't be fully replaced. all and deps are add-only, and build needs the BINARIES := workaround. Any existing single-colon all:, build:, run:, install:, or uninstall: is a fatal error, which affects 23 of 28 root Makefiles.
  • Some callers would break quietly. make test PARALLEL=4 would ignore PARALLEL, and bare make would change meaning in repos whose CI relies on it.
  • The drift check compares against /releases/latest. One release that changes the file would fail PRs across every adopter, including repos pinned to an older version.

Proposed split:

  1. Keep the Renovate annotations, managers, and version pins in step 7. They don't depend on the Makefile work.
  2. Move Makefile.common, lint-makefile-common.yml, the Renovate manager for Makefile.common, and the docs into a draft PR, which becomes the RFD's reference implementation.
  3. Drop Makefile.common from release: v4.0.0 #45's release notes.

I tried this in a scratch clone. All 24 commits above this PR replayed with one conflict (README.md, in the docs split), and the final tree differed from the real tip only by the two removed files. Beyond that, it needs a cleanup of 10 doc references plus docs/makefile-common.md, and one self-referencing SHA pin bump. I didn't run CI on the result.

@synackd synackd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes per comment above.

Signed-off-by: Sean Tronsen <sean.tronsen@gmail.com>
@synackd
synackd force-pushed the seantronsen/feat/makefile-drift-avoidance branch from df9b46f to 5ce9505 Compare November 6, 2026 21:16
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.

3 participants