Draft: CI: aports version check: modernize with new rules

Current state:

• Please review the logic in the graph below. Does it cover all cases? ** EDIT 2022-11-11: Luca found pkgver=9999 case missing, added it
• Contains all commits from !3608 (merged) and still has a bug (see todos below), code review can be done later

Description:

Update the script once more with the goal that we no longer need to comment on pkgrel/pkgver bumps. Parse not only the version of the old and new package, but also the checksums and remote sources.

Replace the previous check of simply having $pkgver-r$pkgrel higher than the upstream APKBUILD with more detailed rules:

      APKBUILD has remote sources?      ------>       Did sha512sums change?
       |                                  No            |         |
       | Yes                                            | No      | Yes
       v                                                |         v
      Did the file name of a remote                     |     * Increase pkgver
      source change?                    ----.           |     * Set pkgrel=0
       |                                    |           |
       | Yes                                | No        |
       v                                    v           v
      Is pkgver=9999?         ----->   * Don't change pkgver
      (Forked from Alpine)     Yes     * Bump pkgrel by 1
       |
       | No
       v
      * Set the pkgver to the version
        of the source tarball
      * Set pkgrel=0

Besides the new rules, print a detailed error message below each package where the test failed that explains exactly what was expected and what the reasoning behind that is. Also add a hint that users can run this locally with 'pmbootstrap ci' now.

Use logging instead of print, so we get colored ERROR: and NOTE: statements from pmbootstrap code, this should make it easier to read.

TODO:

• extend the graph: maintainer change (needs pkgrel bump as metadata changed), _pmb_recommends change (no pkgrel bump, pmbootstrap just reads the APKBUILD)
  • implement these in the check
• FIXME: to check whether a file is remote or not, a glob must be used since the filenames mentioned in sha512sums may be in a subdir
• put this in the wiki at https://postmarketos.org/howto-bump-pkgrel-pkgver once somebody reviewed the logic above
  • also add Q&A about "can I bump pkgrel multiple times in one patchset/MR? -> no / if you REALLY want to then use ci:skip-build"
• verify all code paths work as expected
• get !3608 (merged) on which this depends merged first
• make a pmbootstrap release, so 'pmbootstrap ci' mentioned on error is actually available to users (currently only in master branch)

This branch currently contains a test commit that intentionally makes the test fail and shows the output:

https://gitlab.com/postmarketOS/pmaports/-/jobs/3273705246

By Oliver Smith on 2022-11-04T07:18:53

Doesn't look like this will cover temp/ with pkgver=9999 ? But maybe adding the CI tag there to skip this check is good enough

By Luca Weiss on 2022-11-11T07:32:02

added ci label

added 38 commits

• 2ce66dc0...955ac29f - 26 commits from branch master
• 6e83e042 - .ci/check_devices_in_wiki: remove <> around url (MR 3615)
• 675c2e85 - CI: wiki: adjust for 'pmbootstrap ci' (MR 3615)
• e932ca68 - CI: flake8: adjust for 'pmbootstrap ci' (MR 3615)
• 02b882a2 - CI: shellcheck: adjust for 'pmbootstrap ci' (MR 3615)
• c4d035be - CI: editor-config: adjust for 'pmbootstrap ci' (MR 3615)
• e13849f9 - CI: adjust .ci/common.py users to pmbootstrap ci (MR 3615)
• 79f807c2 - CI: kconfig adjust for 'pmbootstrap ci' (MR 3615)
• d548e7c9 - CI: move_logs.sh: put in .ci/lib (MR 3615)
• 94645a1f - CI: make test_unreferenced_files more generic (MR 3615)
• 3e8b82ea - CI: implement distfile-check as python test (MR 3615)
• 7bacd2b9 - CI: move parse_apkbuild_checksums to common (MR 3615)
• 4e2ce081 - CI: aports version check: modernize with new rules (MR 3615)

Compare with previous version

By Oliver Smith on 2022-11-11T07:31:47

resolved all threads

By Oliver Smith on 2022-11-11T07:32:04

changed the description

By Oliver Smith on 2022-11-11T07:33:03

added 12 commits

• a6ba5155 - .ci/check_devices_in_wiki: remove <> around url (MR 3615)
• 46044067 - CI: wiki: adjust for 'pmbootstrap ci' (MR 3615)
• 7c9fabf5 - CI: flake8: adjust for 'pmbootstrap ci' (MR 3615)
• 42906590 - CI: shellcheck: adjust for 'pmbootstrap ci' (MR 3615)
• 2e8212ed - CI: editor-config: adjust for 'pmbootstrap ci' (MR 3615)
• be67922f - CI: adjust .ci/common.py users to pmbootstrap ci (MR 3615)
• fa122fa7 - CI: kconfig adjust for 'pmbootstrap ci' (MR 3615)
• 5aadba86 - CI: move_logs.sh: put in .ci/lib (MR 3615)
• 6629d977 - CI: make test_unreferenced_files more generic (MR 3615)
• 3e5d1e57 - CI: implement distfile-check as python test (MR 3615)
• c56d3051 - CI: move parse_apkbuild_checksums to common (MR 3615)
• c5fa233f - CI: aports version check: modernize with new rules (MR 3615)

Compare with previous version

By Oliver Smith on 2022-11-11T07:35:27

added status::mr-stale label

Sorry to bother you @ollieparanoid,

but we've detected that this merge request hasn't seen any recent activity. If you need help or want to discuss your approach with developers you can ping @postmarketOS. You can also ask on matrix in #devel:postmarketos.org or #postmarketos-devel on OFTC. If no further activity occurs in this MR, postmarketOS developers may close it in the future.

Thanks for your contribution.

By * postmarketOS Bot on 2022-12-11T08:00:02

What about maintainer changes & _pmb_recommends? Those are also special cases where we bump or not bump. Maybe have them in the graph (and check if the rules are enforced)?

By Dylan Van Assche on 2023-02-16T06:45:11

good point, it would be great to add that as well.

EDIT: added todos in the first post

By Oliver Smith on 2023-02-16T06:51:51

changed the description

By Oliver Smith on 2023-02-16T06:47:39

Maybe it's worth getting a simple version of this that covers only device packages in at some point? A fully featured check seems to be quite difficult to get right, but I think device packages should be fairly simple comparatively?

By Luca Weiss on 2024-05-01T07:13:16

welp, I just discovered that I independently did the device pkg checks in 284fd42c, 970bc34c + a few follow-up commits. I re-used pmb.parse for getting APKBUILD contents for comparing SHAs, etc, whereas this branch seems to (re)implement parsing stuff.

Anyways, I guess that could be checked off the list and a rebased branch will be a little simpler (though the rebase will probably suck )

By clayton craft on 2024-05-01T07:14:55

Can't we just close this? Or are there still remarkable problems with the new approach? Seems to work pretty well for a majority of use-cases, and there seems to be no plan to pick this up in the foreseeable future, right?

By Pablo Correa Gomez on 2024-06-06T16:39:24

yeah we can close my MR, it was unfinished. thanks @craftyguy for making an actually usable version of this! :D

By Oliver Smith on 2024-06-06T19:18:06

closed