Skip to content
Snippets Groups Projects
New look: Off

CI: Improve performance when linting lots of APKBUILDs

    • Closed Imported Issue created by Administrator @root

    The "aport-lint" step in CI is really slow if we are linting lots of APKBUILDs, see e.g. https://gitlab.postmarketos.org/postmarketos/pmaports/-/jobs/526586969

    I suspect this happens because we start pmbootstrap separately for each APKBUILD instead of linting multiple ones in one invocation. Maybe someone wants to investigate if there is a way to optimize it?

    Alternatively it would already help if linting issues were printed immediately once they are found. (So you can see some progress...) At the moment they are only printed once everything has been linted.

    To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information

    Child items 0

    • No child items are currently assigned. Use child items to break down this issue into smaller parts.

    Activity

    • Administrator added ci help wanted type::feature + 1 deleted label · Imported

      added ci help wanted type::feature + 1 deleted label

    • Administrator added difficulty::easy label · Imported

      added difficulty::easy label

    • Administrator
      Administrator @root · Imported
      Author Owner

      i had a look, seems like u can give pmbootstrap lint multiple arguments but with the -q you only get the errors, nothing about the package being linted so you could just have the lint run once with all the pakages at once but you wont know what file the error is from

      By Raatty on 2020-05-25T11:46:35

    • Administrator
      Administrator @root · Imported
      Author Owner

      It looks like there's not only the loop in apkbuild-linting.py but also another one in pmbootstrap itself where apkbuild-lint is called multiple times. I ran a few quick comparisons on the APKBUILDs in the cross folder to measure the difference:

      Linting one-by-one with pmbootstrap

      $ time ( for pkg in *; do pmbootstrap -q lint $pkg > /dev/null; done )

real    0m52,295s
user    0m51,895s
sys     0m3,945s

      Linting all files with a single pmbootstrap invocation

      $ time ( echo * | xargs pmbootstrap -q lint > /dev/null )

real    0m5,216s
user    0m6,981s
sys     0m1,876s

      Linting all files with a single apkbuild-lint invocation

      $ time ( find . -name APKBUILD | CUSTOM_VALID_OPTIONS='!pmb:crossdirect !pmb:kconfig-check pmb:kconfigcheck-anbox pmb:kconfigcheck-containers pmb:kconfigcheck-nftables pmb:cross-native pmb:gpu-accel pmb:strict' xargs apkbuild-lint > /dev/null )

real    0m1.477s
user    0m3.962s
sys     0m0.972s

      When linting multiple files at once, apkbuild-lint actually nicely prefixes each message with the file path:

      $ apkbuild-lint arch-bin-masquerade/APKBUILD ccache-cross-symlinks/APKBUILD 
MC:[AL49]:./arch-bin-masquerade/APKBUILD:15:invalid option 'pmb:cross-native'
MP:[AL32]:./arch-bin-masquerade/APKBUILD:35:unnecessary usage of braces: ${_bin}
MP:[AL32]:./arch-bin-masquerade/APKBUILD:37:unnecessary usage of braces: ${_bin}
MP:[AL32]:./arch-bin-masquerade/APKBUILD:37:unnecessary usage of braces: ${_hostspec}
MP:[AL32]:./ccache-cross-symlinks/APKBUILD:31:unnecessary usage of braces: ${_bin}
MP:[AL32]:./ccache-cross-symlinks/APKBUILD:31:unnecessary usage of braces: ${_hostspec}
MP:[AL32]:./ccache-cross-symlinks/APKBUILD:34:unnecessary usage of braces: ${_hostspec}
MP:[AL32]:./ccache-cross-symlinks/APKBUILD:35:unnecessary usage of braces: ${_hostspec}

      The problem is that in pmbootstrap we're copying the package folder contents to /home/pmos/build one by one and then run apkbuild-lint from within that folder so it can only print out ./APKBUILD:

      $ pmbootstrap lint arch-bin-masquerade ccache-cross-symlinks
[14:34:21] (native) linting arch-bin-masquerade with apkbuild-lint
MP:[AL32]:./APKBUILD:35:unnecessary usage of braces: ${_bin}
MP:[AL32]:./APKBUILD:37:unnecessary usage of braces: ${_bin}
MP:[AL32]:./APKBUILD:37:unnecessary usage of braces: ${_hostspec}
[14:34:21] (native) linting ccache-cross-symlinks with apkbuild-lint
MP:[AL32]:./APKBUILD:31:unnecessary usage of braces: ${_bin}
MP:[AL32]:./APKBUILD:31:unnecessary usage of braces: ${_hostspec}
MP:[AL32]:./APKBUILD:34:unnecessary usage of braces: ${_hostspec}
MP:[AL32]:./APKBUILD:35:unnecessary usage of braces: ${_hostspec}
[14:34:21] NOTE: chroot is still active (use 'pmbootstrap shutdown' as necessary)
[14:34:21] DONE!

      I think the easiest would be have pmbootstrap copy the package folders into subdirectories of /home/pmos/build and then invoke apkbuild-lint a single time with all APKBUILDs.

      By Johannes Marbach on 2021-08-29T12:41:18

    • Administrator
      Administrator @root · Imported
      Author Owner

      Thanks for investigating this @cherrypicker, your findings are very useful!

      I think the easiest would be have pmbootstrap copy the package folders into subdirectories of /home/pmos/build and then invoke apkbuild-lint a single time with all APKBUILDs.

      • we only want to run it on the packages that were modified in the MR
      • actually we don't need to copy all APKBUILDs to /home/pmos/build, pmaports.git is available in its entirety in /mnt/pmbootstrap-git/pmaports inside the chroot
      • so I suggest we modify pmbootrap as follows:
        • run apkbuild-lint inside /mnt/pmbootstrap-git/pmaports
        • give paths to APKBUILDs for the packages we are interested in as argument to apkbuild-lint
        • (pmbootstrap lint already accepts packages as argument, we only need to turn them into APKBUILD paths relative to /mnt/pmbootstrap-git/pmaports)
      • and then modify the CI script to run pmbootstrap only once. as you pointed out above, apkbuild-lint will nicely print the path to the APKBUILD in question.

      By Oliver Smith on 2021-08-29T17:19:11

    • Administrator
      Administrator @root · Imported
      Author Owner

      we only want to run it on the packages that were modified in the MR

      Yes, sorry, I didn't mean to say we should copy the whole package tree. The logic for selecting the relevant packages already exists in apkbuild-linting.py and shouldn't have to change.

      actually we don't need to copy all APKBUILDs to /home/pmos/build, pmaports.git is available in its entirety in /mnt/pmbootstrap-git/pmaports inside the chroot

      Holy cow, that's helpful to know! I had already started to hack pmbootstrap to copy the respective package folders but if the tree is already mounted, that'll make it even easier / faster.

      By Johannes Marbach on 2021-08-29T19:06:44

      Edited by Administrator
    • Administrator
      Administrator @root · Imported
      Author Owner

      Actually, browsing through the code I think /mnt/pmaports might be better. If I understand correctly, that's the one that's set in the config whereas /mnt/pmbootstrap-git/pmaports is the pmaports checkout that pmbootstrap manages itself?

      As an example, I use a manual checkout of pmaports with pmbootstrap and I would expect it to lint files from there rather than from its internal checkout.

      $ pmbootstrap config
[pmbootstrap]
aports = /home/jm/Code/forks/pmaports
...

      By Johannes Marbach on 2021-08-29T19:21:50

    • Administrator mentioned in merge request pmbootstrap!2100 (merged) · Imported

      mentioned in merge request pmbootstrap!2100 (merged)

      By Johannes Marbach on 2021-08-30T07:50:20

    • Administrator mentioned in merge request !2472 (merged) · Imported

      mentioned in merge request !2472 (merged)

      By Johannes Marbach on 2021-08-30T09:34:26

    • Administrator closed · Imported

      closed

    • Administrator closed with commit 14d2eda4 · Imported

      closed with commit 14d2eda4

    • Administrator mentioned in commit 14d2eda4 · Imported

      mentioned in commit 14d2eda4

      By Johannes Marbach on 2021-09-16T05:55:24

    Please register or sign in to reply