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

CI: reusable_build: #18

Open
systemcrash opened this issue Apr 16, 2024 · 8 comments
Open

CI: reusable_build: #18

systemcrash opened this issue Apr 16, 2024 · 8 comments

Comments

@systemcrash
Copy link
Contributor

I think the || logic here is wrong. All packages are checked even if only a single package has changed in a (force) push. It would seem that any push event always triggers make package/check (if [ "${{ github.event_name }}" = "push" ])

      - name: Download and check packages
        if: inputs.build_all_modules == true || inputs.build_all_kmods == true || inputs.build_full == true
        shell: su buildbot -c "sh -e {0}"
        working-directory: openwrt
        run: |
          # With push events or check_packages_list set to 'all', check all packages
          if [ "${{ github.event_name }}" = "push" ] || [ "${{ inputs.check_packages_list }}" = "all" ]; then
            make package/download package/check FIXUP=1 -j$(nproc) BUILD_LOG=1 || ret=$? .github/workflows/scripts/show_build_failures.sh
          # With every other event check only changed packages (if provided)
          elif [ -n "${{ inputs.check_packages_list }}" ]; then
            for package in ${{ inputs.check_packages_list }}; do
              make package/$package/download package/$package/check FIXUP=1 -j$(nproc) BUILD_LOG=1 || ret=$? .github/workflows/scripts/show_build_failures.sh
            done
          fi

Also, if only checksums have changed in a Makefile, package/check FIXUP=1 seems to do nothing locally (to warrant a git add blah; git commit --amend ; git push --force).

@Ansuel
Copy link
Member

Ansuel commented Apr 16, 2024

That is intended as the comment suggest. Push events on openwrt main repo are on main branch and stable branch and we want to check every package.

About the package/check not correctly fixing stuff, that is a bug i think ? probably FIXUP=1 is not propagated?

@systemcrash
Copy link
Contributor Author

we want to check every package.

maybe better to have two separate event handlers in the yaml - one for pushes to main, and one for PRs and their (force) pushes. Then the check and build is much quicker for those PRs where only a single package changed.

About the package/check not correctly fixing stuff, that is a bug i think ? probably FIXUP=1 is not propagated?

🤷

@Ansuel
Copy link
Member

Ansuel commented Apr 16, 2024

maybe better to have two separate event handlers in the yaml - one for pushes to main, and one for PRs and their (force) pushes. Then the check and build is much quicker for those PRs where only a single package changed.

but push to pr (on our side) are pr event so the condition of checking all the package should not be triggered.

@systemcrash
Copy link
Contributor Author

Hmm - my action logs for single package changes show they check every package.

@Ansuel
Copy link
Member

Ansuel commented Apr 16, 2024

@systemcrash can you link the action? Are you sure it's not run on your repository?

@systemcrash
Copy link
Contributor Author

The runs are in my repo - but that should not matter, or?

@Ansuel
Copy link
Member

Ansuel commented Apr 16, 2024

It does as you push commits to a branch, push on your repository are not handled by openwrt runner but by your runner.

Check here

https://github.com/openwrt/openwrt/actions
"Pull request openwrt/openwrt#14448 synchronize by roshii"

For push we have instead
Build Kernel #10217: Commit 12137cb pushed by openwrt-bot

But here (my repository) (ignore the gravestone of failed actions ahahha)
https://github.com/ansuel/openwrt/actions?page=2

#920: Commit 23df902 pushed by Ansuel

@systemcrash
Copy link
Contributor Author

systemcrash commented Apr 16, 2024

The Determine Changed Packages step correctly determines the only changed package. But the Download and check packages code in the first post checks everything. So something isn't right, or could be better.

( Irrespective of which repo or runner )

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

No branches or pull requests

2 participants