openwrt / actions-shared-workflows

4 stars 14 forks source link

CI: Run package/check on all packages #3

Closed hauke closed 1 year ago

hauke commented 1 year ago

This will check some basic properties on all package Makefiles after the full build. It checks for example if the PKG_HASH is correct and will fail if it is not correct. This should detect such problems before they get merged.

This will always be executed when we do a full package build, the check is pretty fast.

Previously located here: https://github.com/openwrt/openwrt/pull/12803

hauke commented 1 year ago

@Ansuel and @ynezz How do I test this? I also would like to test this with a broken package Makefile.

hauke commented 1 year ago

I am now testing it here: https://github.com/hauke/openwrt/actions/runs/6343912946 https://github.com/hauke/openwrt/actions?query=branch%3Aci-run-package-check-test

Ansuel commented 1 year ago

@hauke now that i'm noticing it... I would put the check as a separate step and not after the build. What do you think? The check should be a quicker step

hauke commented 1 year ago

I will try this tomorrow

ynezz commented 1 year ago

The check should be a quicker step

I'm using following elsewhere and thats basically all the steps which are needed to download and check the package integrity:

    - make defconfig
    - make -j $(nproc) tools/tar/compile
    - make -j $(nproc) download check FIXUP=1

    - >
      git diff-index --exit-code HEAD || {
        ret=$?
        echo "Package integrity issues, please check packages-hash-issues.patch from artifacts"
        git diff | tee packages-hash-issues.patch
        exit $ret
      }
hauke commented 1 year ago

@hauke now that i'm noticing it... I would put the check as a separate step and not after the build. What do you think? The check should be a quicker step

I placed it now into an own step. I am testing if it works: https://github.com/hauke/openwrt/actions?query=branch%3Aci-run-package-check-test https://github.com/hauke/openwrt/actions?query=branch%3Aci-run-package-check-test-pass

hauke commented 1 year ago

I moved the check now to the beginning let's see if it works. We have to select a configuration before t will only check selected packages. Maybe we can move the configuration set also into the setup step.

hauke commented 1 year ago

I added the missing make tools/tar/compile step. Otherwise it worked before. It found already a problem, see https://github.com/openwrt/openwrt/commit/954142f4774aa6c0dca0a44179b3b0d119782f38

Ansuel commented 1 year ago

@hauke i see this is only for check runs. With the quilt patch we can also add the refresh one?

hauke commented 1 year ago

@hauke i see this is only for check runs. With the quilt patch we can also add the refresh one?

When quilt refresh for all packages is working we should add it here too.

Ansuel commented 1 year ago

Ok something we can think about later. Think I will merge this.

@ynezz Should we really block the build step if check fails?

ynezz commented 1 year ago

Should we really block the build step if check fails?

Yes, would be nice to be aware, its just a CI for additional QA. Or do you've any argument against?

Ansuel commented 1 year ago

Should we really block the build step if check fails?

Yes, would be nice to be aware, its just a CI for additional QA. Or do you've any argument against?

nope nothing against, might be relevant when we will introduce patches check but I think it's better to me more conservative with runs

Ansuel commented 1 year ago

@hauke do we have variant like tools/check toolchain/check and packages/check? I think I will improve your version and by default run only the packages check (and make the toolchain one for the toolchain run and the tools in the tools workflow)

Also more broken packages came... Can you by chance take care of fixing them?

hauke commented 1 year ago

@hauke do we have variant like tools/check toolchain/check and packages/check? I think I will improve your version and by default run only the packages check (and make the toolchain one for the toolchain run and the tools in the tools workflow)

Then we have to copy this code into other workflows, maybe make it shared? Most hashes are for packages so it is good to have it there first.

Also more broken packages came... Can you by chance take care of fixing them?

I only saw one broken package and fixed it ( https://github.com/openwrt/openwrt/commit/954142f4774aa6c0dca0a44179b3b0d119782f38 ), are there more? I haven't tested the feed.

Ansuel commented 1 year ago

Then we have to copy this code into other workflows, maybe make it shared? Most hashes are for packages so it is good to have it there first.

In theory it would be placed in reusable_build and reusable_build_tools. So no idea if it's worth to generalize.

I only saw one broken package and fixed it ( openwrt/openwrt@954142f ), are there more? I haven't tested the feed.

yufut also had the hash broken. But took care of fixing it.

hauke commented 1 year ago

Then we have to copy this code into other workflows, maybe make it shared? Most hashes are for packages so it is good to have it there first.

In theory it would be placed in reusable_build and reusable_build_tools. So no idea if it's worth to generalize.

I am fine if we detect such problems for the packages, we should not invest too much resources into this small topic.

I only saw one broken package and fixed it ( openwrt/openwrt@954142f ), are there more? I haven't tested the feed.

yufut also had the hash broken. But took care of fixing it. thanks.

Ansuel commented 1 year ago

@hauke yep every change has already been done, if you are curious, check the latest commits