Skip to content

fix(dist-git): use first-parent for snapshot-time commit resolution#192

Open
Tonisal-byte wants to merge 2 commits into
microsoft:mainfrom
Tonisal-byte:asalinas/update-first-parent
Open

fix(dist-git): use first-parent for snapshot-time commit resolution#192
Tonisal-byte wants to merge 2 commits into
microsoft:mainfrom
Tonisal-byte:asalinas/update-first-parent

Conversation

@Tonisal-byte
Copy link
Copy Markdown
Contributor

Problem

GetCommitHashBeforeDate walks all parents when resolving the upstream commit
via snapshot time. This can resolve to a commit from a merged-in side branch
(e.g. an f44 commit merged into f43), causing "import-commit not found"
errors during synthetic history generation because the first-parent walk
can't reach the import-commit from a side-branch commit.

Fix

Add --first-parent to the git rev-list command so the resolved commit is
always on the target branch's mainline. The merge commit that incorporated
the side branch is the correct anchor — it has the same tree content and
sits on the first-parent chain where the synthetic history walk expects it.

Copilot AI review requested due to automatic review settings May 14, 2026 17:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 adjusts dist-git snapshot commit resolution so timestamp-based upstream commit selection stays on the target branch’s first-parent history, avoiding side-branch commits that synthetic history generation cannot reach.

Changes:

  • Adds --first-parent to git rev-list in GetCommitHashBeforeDate.
  • Expands the inline comment to explain why first-parent traversal is required for snapshot-time resolution.

Comment thread internal/utils/git/git.go
@Tonisal-byte Tonisal-byte force-pushed the asalinas/update-first-parent branch from 3d453db to 5b8f6eb Compare May 14, 2026 23:23
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