Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
|
run buildall |
There was a problem hiding this comment.
Blocking issue:
.github/workflows/license-eyes.yml: the new changed-file detection usesgit diff FETCH_HEAD...HEADafter checking out the PR head and fetching the base branch with--depth=1. In a normal Actions shallow checkout, that often leaves no merge base, so the workflow fails before it can build the incremental config. I reproduced this against this PR with:
git fetch --depth=1 origin f75194aef28615ea111ae0e1dfff99e8297e1cbd && git checkout FETCH_HEAD && git fetch --no-tags --depth=1 origin master && git diff --name-only FETCH_HEAD...HEAD
which returnsfatal: FETCH_HEAD...HEAD: no merge base.
Critical checkpoints:
- Goal of the current task: improve the license check by scoping PR runs to changed files and falling back to the full config when
.licenserc.yamlchanges. The current code does not reliably accomplish that goal because the changed-file computation can fail on ordinary PRs. - Modification size and focus: yes, the change is small and focused on one workflow.
- Concurrency: not applicable; this workflow executes sequential shell steps and introduces no shared-state concurrency concerns.
- Lifecycle or static initialization: not applicable.
- Configuration items: no new repository/runtime config was added.
- Incompatible changes: none.
- Functionally parallel code paths: push and manual-dispatch paths remain unchanged; the PR path is the one regressed by the new diff logic.
- Special conditional checks: the new
.licenserc.yamlconditional is reasonable, but it depends on the broken changed-file detection. - Test coverage: insufficient. No validation was added for the new shallow-fetch diff logic, and the regression is directly reproducible.
- Test results modified: none.
- Observability: existing workflow logs are sufficient to diagnose this failure.
- Transaction/persistence/data-write concerns: not applicable.
- FE/BE variable passing: not applicable.
- Performance: incremental checking is a good direction, but correctness needs to be restored first.
- Other issues: none beyond the blocking regression above.
| ref: ${{ github.event.pull_request.head.sha }} | ||
|
|
||
| - name: Fetch base branch for diff | ||
| if: github.event_name == 'pull_request_target' |
There was a problem hiding this comment.
git diff FETCH_HEAD...HEAD needs a merge base, but this workflow is still running in a shallow checkout (actions/checkout defaults to depth 1, and this fetch is also depth 1). On a normal PR that means the merge base is often missing, so the new step fails before generating .licenserc-incremental.yaml.
I reproduced it with this PR's commits in a fresh clone:
git fetch --depth=1 origin f75194aef28615ea111ae0e1dfff99e8297e1cbd
git checkout FETCH_HEAD
git fetch --no-tags --depth=1 origin master
git diff --name-only FETCH_HEAD...HEAD
# fatal: FETCH_HEAD...HEAD: no merge baseThe previous tj-actions/changed-files implementation handled this case, so this is a workflow regression. Please fetch enough history before using ... (for example a full checkout or an explicit merge-base-aware fetch), or switch to a diff method that does not depend on the merge base.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)