Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🧪 Let ansible-test diff ancient branches (the expensive way) #84402

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

webknjaz
Copy link
Member

@webknjaz webknjaz commented Nov 27, 2024

This patch addresses the long-standing issue with being unable to run
CI on PRs that have their branches created over 500 commits prior.

When the first git diff attempt fails, this patch attempts to
augment the Git tree with missing commits.

This is achieved by unshallowing the entire Git tree.

SUMMARY

SSIA

ISSUE TYPE
  • Maintenance Pull Request
  • Test Pull Request
ADDITIONAL INFORMATION

This is a more expensive alternative to #84375. It adds 33 seconds to each job compared to the granular implementation.

Demo log output: https://dev.azure.com/ansible/ansible/_build/results?buildId=129703&view=logs&j=f39c292d-0a23-56d4-e7e5-386951c14ece&t=3af4f6c8-91cf-5ddb-8873-9e1cb9dedca9&l=73 (from #84403)

This patch addresses the long-standing issue with being unable to run
CI on PRs that have their branches created over 500 commits prior.

When the first `git diff` attempt fails, this patch attempts to
augment the Git tree with missing commits.

This is achieved by doing the following:
1. Fetch one additional commit at the root of each orphaned
   branch.
2. Identify said commits as tree roots or "grafted".
3. Retrieve the timestamps of both and safe the oldest one.
4. Fetch again, now instructing Git to include everything past
   this date.

The idea is that the very first commit in the PR branch might
be created on the same day as the PR is opened. However, its
parent would belong to the base branch, which is immutable. So
if we take that commit's time stamp, we can tell Git to fill
in the commits between the PR branch chunk and the commit it
branched out of (the fork point). We'll also fill in the
commits on the other side of the "fork", in the base branch.
With those commits retrieved and available in the local cache
of the remote, it will be possible for git diff to locate the
merge base and compute the file list, while the tree remains
shallow. This is still a heuristic, of course, since the
commit dates can be out of order in the tree. But it's not
expected that we'll hit this corner case often or ever.
@webknjaz webknjaz requested a review from mattclay November 27, 2024 14:46
@webknjaz webknjaz self-assigned this Nov 27, 2024
@webknjaz webknjaz requested a review from nitzmahone November 27, 2024 14:47
@webknjaz webknjaz changed the title 🧪 Let ansible-test diff ancient branches (the expensive way) 🧪 Let ansible-test diff ancient branches (the expensive way) Nov 27, 2024
@webknjaz webknjaz added test This PR relates to tests. needs_triage Needs a first human triage before being processed. ansible-test Release work related to ansible-test labels Nov 27, 2024
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Dec 5, 2024
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Dec 5, 2024
Copy link
Member

@mattclay mattclay left a comment

Choose a reason for hiding this comment

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

See my comment on the related PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ansible-test Release work related to ansible-test stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants