feat(get-modified-packages): support retrieving diffs without fetching all

[xmfcx]

Description

Continuation from:

• ci(build-and-test-differential): set checkout fetch-depth to 1 autoware.universe#7408

• Renaming the input parameter name from base-branch to base-ref. (Mostly it defaults to main)

  • I think it is not used anywhere.
  • But we might use it for merge_group triggers in the future

• It will run: git fetch origin ${{ inputs.base-ref }} to make sure it has common merge-base commit to get diffs

• And now it is possible to only fetch the necessary commits instead of entire history of the repository. (For universe it allows time saving from about 2m30s to 6s)

• Separate the get-modified-packages action into 2 steps so it will report if it fails

  • Also exit with failure on the sh script.
  • set -e is removed from the script because it is set by default on github actions.

Tests performed

Failure condition

It fails on failure.
https://github.com/autowarefoundation/autoware.universe/actions/runs/9482100138/job/26126284417?pr=7340#step:7:29

Success condition

And it is successful on success:
https://github.com/autowarefoundation/autoware.universe/actions/runs/9482151939/job/26126437257?pr=7340#step:7:35

Effects on system behavior

Not applicable.

Interface changes

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

• I've confirmed the contribution guidelines.
• The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

• The PR follows the pull request guidelines.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

• There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

[xmfcx]

I've fixed it.

Job: https://github.com/autowarefoundation/autoware.universe/actions/runs/9470181454/job/26090502061?pr=7340

Workflow: https://github.com/autowarefoundation/autoware.universe/actions/runs/9470181454/workflow?pr=7340#L33-L51

Github action PR:
https://github.com/autowarefoundation/autoware-github-actions/pull/299/files

First part

• base-branch in the action is a bad idea because it hides the commit sha of the feature branch's base, it is just "main"
• I've replaced it with base-sha, which is github.event.pull_request.base.sha and it is much more simple to use and fits better with the code (we don't need to add origin/ before it)
• It works with existing jobs without problems

Second part

• In order to fetch only what we need, we need to do it like this:

    steps:
      - name: "Set PR fetch depth"
        run: echo "PR_FETCH_DEPTH=$(( ${{ github.event.pull_request.commits }} + 1 ))" >> "${GITHUB_ENV}"

      - name: "Checkout PR branch and all PR commits"
        uses: actions/checkout@v4
        with:
          ref: ${{ github.event.pull_request.head.sha }}
          fetch-depth: ${{ env.PR_FETCH_DEPTH }}

• credits to @polarathene 's post in How do I fetch all commits only in the PR branch actions/checkout#552 (comment)
• That's it, it will fetch only what we need. And everything will work, you can check the job link in this post.

[xmfcx]

This solution works for both fetch-depth: ${{ env.PR_FETCH_DEPTH }} and for fetch-depth: 0 so it is safe to merge.

You can test it with existing PRs if you need confirmation using uses: autowarefoundation/autoware-github-actions/get-modified-packages@v-test-shallow-copy tag. (Will remove it once this is merged.)

[xmfcx]

Outdated comment.

🖱️Click here to expand🔛

Screenshot from the job link above

[HansRobo]

Thank you for taking over the job from me and completing it!
BTW, this is a breaking change and will affect people who specify base-branch in get-modified-packages@v1.
Would you consider bumping the repository major version?

[xmfcx]

@HansRobo

If everyone used it with the default "base-branch" parameter, it won't break existing functionality, anyone using the old version can keep using the new version without modifying any code and it will work the same.

But if someone changed the "base-branch" parameter (which is github.base_ref which is main by default for most cases), then it won't work since this parameter no longer exists. But I've never seen anyone passing that variable to it. Everyone uses it to get the diffs from the place they've branched out. I think we could even remove that parameter completely, what do you think?

[xmfcx]

We can still bump versions but then it will bump the versions of everything else in this repository too. Then any new change on them needs to be updated on the repository sides too (to v2) even if they were not changed. :(

[xmfcx]

I've removed the input parameter because I don't see a use for it.
Also updated the readme to have both fetch-all and fetch-enough versions.

[HansRobo]

@xmfcx
Thank you for the discussion!
Considering the pros and cons, it seems better not to bump the major version.

[HansRobo]

Looks good to me.
Thank you for working hard on this task!!

[xmfcx]

To confirm, it is working well for regular PRs in the universe too:

• https://github.com/autowarefoundation/autoware.universe/actions/runs/9483164187/job/26129625975#step:7:35
  

[HansRobo]

I have only found one person who uses the base-branch argument... and that is me haha 😆.
https://github.com/ibis-ssl/crane/blob/8a0fae59b45e578b53064878819aa65cb2af8cd1/.github/workflows/build_test.yaml#L70
Don't worry, I'll fix it according to the new specifications.