Skip to content

[ci](fix) license check#62495

Open
hello-stephen wants to merge 1 commit intoapache:masterfrom
hello-stephen:0414-fix
Open

[ci](fix) license check#62495
hello-stephen wants to merge 1 commit intoapache:masterfrom
hello-stephen:0414-fix

Conversation

@hello-stephen
Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen
Copy link
Copy Markdown
Contributor Author

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@hello-stephen
Copy link
Copy Markdown
Contributor Author

/review

@hello-stephen
Copy link
Copy Markdown
Contributor Author

run buildall

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Blocking issue:

  • .github/workflows/license-eyes.yml: the new changed-file detection uses git diff FETCH_HEAD...HEAD after 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 returns fatal: 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.yaml changes. 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.yaml conditional 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'
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.

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 base

The 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.

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.

1 participant