Skip to content

fix(sources): clamp interleaved commit timestamps to keep %changelog descending - #298

Draft
WithEnoughCoffee wants to merge 2 commits into
microsoft:mainfrom
WithEnoughCoffee:fix/synthistory-monotonic-changelog-dates
Draft

fix(sources): clamp interleaved commit timestamps to keep %changelog descending#298
WithEnoughCoffee wants to merge 2 commits into
microsoft:mainfrom
WithEnoughCoffee:fix/synthistory-monotonic-changelog-dates

Conversation

@WithEnoughCoffee

@WithEnoughCoffee WithEnoughCoffee commented Jul 27, 2026

Copy link
Copy Markdown

Summary

When azldev rebuilds a component's synthetic dist-git history, it interleaves synthetic downstream commits (from lock-file fingerprint changes) with upstream commits. A synthetic commit that references an older upstream pin but has a later wall-clock date can be placed before a newer-versioned upstream release commit with an earlier date, making the replayed history non-monotonic in time.

rpmautospec dates each %changelog entry from the git committer timestamp (pkg_history.py uses commit.commit_time), so a non-monotonic timeline renders a %changelog whose dates are not in descending order. rpmbuild then fails the build with:

error: %changelog not in descending chronological order

Fix

In replayInterleavedHistory, track a running lastWhen and clamp each replayed commit's author and committer timestamps forward to that maximum (clampWhenForward / laterTime). This guarantees the replayed history is monotonic non-decreasing, so the generated changelog is always in descending order.

  • Only activates in the inversion case; normal renders (synthetics newest) are unaffected.
  • Equal adjacent dates are accepted by rpmbuild, so ties introduced by clamping are fine.
  • The synthetic history is ephemeral (used only to feed rpmautospec), so clamped dates do not affect lock fingerprints.

Test

Adds TestCommitInterleavedHistory_ClampsInvertedTimestamps, which reproduces the inversion using an upstream commit with distinct author/committer dates (both earlier than the inverting synthetic) and asserts both timelines are monotonic and clamped. The committer assertion mirrors the date rpmautospec actually renders; verified the test fails if either the author- or committer-clamp is removed.

Validation

  • mage check all (mod, lint, editorconfig, static, licenses) — pass
  • go test ./internal/app/azldev/core/sources/ — pass (all 8 interleave tests)
  • End-to-end: rebuilt azldev and re-rendered a real component whose changelog was date-inverted; the rendered %changelog is now valid descending order and passes rpmbuild -bs (no descending-order error). Render is deterministic (byte-identical across runs).

Notes

This is the root-cause fix for a downstream libjcat build failure where a f43 0.2.6 pin (Apr 14) sat after a repo-wide lock-file infra commit (Apr 30) in the interleaved history.

Note: Python lint fix

Also adds a year to render_process.py's copyright header. This lint failure predates my change and is unrelated to it — Ruff's CPY001 requires a 4-digit year, so the year-less header fails CI (main is already red on the same rule).

…descending

Interleaving can place a later-dated synthetic commit before an earlier-dated
upstream release commit, making the replayed history non-monotonic. rpmautospec
dates %changelog entries from the committer timestamp, so this renders an
out-of-order changelog and rpmbuild fails with "%changelog not in descending
chronological order".

Clamp each replayed commit's author/committer time forward to the running max
so the timeline stays monotonic. Only activates in the inversion case.

Copilot-Session: 377514ba-3f63-4447-b0ba-939640c6ee62
Copilot AI review requested due to automatic review settings July 27, 2026 19:37

Copilot AI 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.

Pull request overview

This PR fixes a build-breaking edge case in azldev’s synthetic dist-git history replay where interleaving downstream synthetic commits with upstream commits can create a non-monotonic (time-inverted) commit timeline, which causes rpmautospec to generate a %changelog that rpmbuild rejects as not descending chronologically.

Changes:

  • Enforces a monotonic non-decreasing replay timeline by clamping replayed commit author/committer timestamps forward to a running minimum (lastWhen).
  • Threads the running minimum timestamp through upstream replay and synthetic commit creation, returning the effective timestamp for the next iteration.
  • Adds a regression test reproducing the inversion (including distinct upstream author vs committer times) and asserting both timelines are clamped and monotonic.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
internal/app/azldev/core/sources/synthistory.go Tracks lastWhen during replay and clamps upstream + synthetic commit timestamps forward to prevent inverted commit time ordering.
internal/app/azldev/core/sources/synthistory_test.go Adds a focused test that reproduces the inversion and asserts both author and committer timelines are clamped/monotonic.

Ruff's CPY001 (missing-copyright-notice) requires a 4-digit year in the
notice; the year-less header now fails CI (main is red on the same rule).
Adds the file's creation year to satisfy the rule.

Copilot-Session: 377514ba-3f63-4447-b0ba-939640c6ee62
Copilot AI review requested due to automatic review settings July 27, 2026 20:22

Copilot AI 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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

// The whole timeline must be monotonic non-decreasing (oldest → newest),
// i.e. non-increasing when read newest-first. Assert on BOTH the committer
// timeline (the one rpmautospec renders) and the author timeline.
for idx := range len(logCommits) - 1 {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is a false positive. for idx := range len(logCommits)-1 uses Go 1.22+ range-over-integer, and this module targets go 1.25.6 (go.mod), so it compiles — go vet ./... passes and the tests build and run green. The same idiom is already used in the repo, e.g. env_test.go:195 (for suggestionIndex := range suggestionCount) and synthistory_test.go:954 (for i := range 3). Keeping as-is for consistency.

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