packages icon indicating copy to clipboard operation
packages copied to clipboard

PR linters

Open silkeh opened this issue 2 years ago • 10 comments
trafficstars

Add GitHub integration(s) to detect common problems with PRs:

  • [x] Included .eopkg files (#377).
  • [x] Included Makefile (#377).
  • [x] Files in the wrong directory (ie packages/* and packages/*/*) (#377).
  • [x] Use of patch < instead of -i (#377).
  • [x] Missing homepage in package.yml (#377).
  • [x] Package is not bumped once (as a warning) (#475).
  • [x] Initial release on inclusion != 1 (#475).
  • [ ] Incorrect package component (depends on #370).
  • [x] Alphabetized build dependencies (pkgconfig,A-Z,a-z,0-9) (#695).
  • [x] The commit must not end with a ] (#889).
  • [x] Version numbers that are valid numbers must have quotations (#963).
  • [ ] pspec_x86_64.xml is invalid (cannot be parsed).
  • [x] Licences are valid SPDX qualifiers (#888).
  • [ ] Check if there is a metadata.xml file for font packages. Lint the file.
  • [ ] Appstream metadata (see below)
  • [x] abi_used_libs for packages in system.base and system.devel must be part of system.base or system.devel (https://github.com/getsolus/packages/pull/891).
  • [ ] Hardcoded python version (python3\.\d+) in package.yml.

To be refined further:

  • [ ] Single package bump per commit.
  • [ ] PR build order.
  • [ ] Package layout.

silkeh avatar Sep 18 '23 15:09 silkeh

Some things we need to check for

  • Accidentally including a file outside of a package folder in a commit - how would the builder handle that if that accidentally got through?
  • If a PR contains updates for multiple packages e.g. rebuilds. Then the PR will need to provide the build order for someone with push access to publish the packages (this is potentially scriptable with the current build server but deploying summit would solve it)
  • Package is not bumped - however, this is not a hard requirement and sometimes people only push formatting/styling fixes to a package.yml for instance.

Not specifically related to linting:

  • A PR containing multiple package updates e.g. rebuilds/self-contained stack, the packager may need to rebase specific commits, this is not explained in the packaging docs.

joebonrichie avatar Sep 20 '23 12:09 joebonrichie

  • Accidentally including a file outside of a package folder in a commit - how would the builder handle that if that accidentally got through?

So for this one, I think for the first package.yml file we come across - if the release has been bumped then we need to ensure no other files outside of that directory get committed.

It would be nice if there was a way to signify whether a commit is actually a package update, not an update in common, or a styling change.

joebonrichie avatar Sep 20 '23 13:09 joebonrichie

Linking in #370 as we can apply linters to that more easily with monorepo

joebonrichie avatar Sep 22 '23 19:09 joebonrichie

alphabeticalized builddeps/rundeps pkgconfig,A-Z,a-z,0-9

joebonrichie avatar Sep 25 '23 15:09 joebonrichie

  • Package is double bumped
  • Initial inclusion of a package the release != 1
  • pspec_x86_64.xml is invalid (cannot be parsed)
  • The commit must not end with a ] as that breaks the eopkg index due to xml CDATA shenanigans (edit: This may be specific to piksemel and might be fixed with the move to lxml with the py3 version.
  • Versions that end with a 0 must have quotations around them (classic yaml problem)
  • There has never been a hard requirement of how a package.yml file is laid out but generally its:
name:
version:
source:
homepage:
license:
component:
summary:
description:
builddeps:
rundeps:
setup:
build:
install:
check:

However, we do not have a generally accepted order for other keys such as

mancompress:
avx2:
conflicts:
replaces:
libsplit:

...and others. These typically go after description but before builddeps however, the order of them is not strictly defined. Additionally, there seem to be a recent convention of putting replaces and conflicts at the bottom of the package.yml file.

joebonrichie avatar Oct 08 '23 09:10 joebonrichie

Files from pypi should be in this url format https://files.pythonhosted.org/packages/source/u/urlscan/urlscan-1.0.1.tar.gz

joebonrichie avatar Oct 21 '23 23:10 joebonrichie

Packages with a .desktop file in /usr/share/applications/ should have appstream metadata, so either:

  • /usr/share/metainfo/<package>.metainfo.xml <- current
  • /usr/share/appdata/<package>.appdata.xml <- legacy

silkeh avatar Nov 03 '23 22:11 silkeh

Packages with a .desktop file in /usr/share/applications/ should have appstream metadata, so either:

* `/usr/share/metainfo/<package>.metainfo.xml` <- current

* `/usr/share/appdata/<package>.appdata.xml` <- legacy

Also, just learned that appstream builder can't handle symlinks. The package.yml file should use install for the metadata file rather than a symlink

TraceyC77 avatar Nov 13 '24 16:11 TraceyC77

A linter to make sure nothing is installed to /usr/local would be great; I've run across two packages in the last two weeks that were erroneously installed there.

EbonJaeger avatar Jan 10 '25 23:01 EbonJaeger

A linter warning about packages having the system.base component would be a good idea. We've had two PRs in the last week or two with it, one of which was merged without changes.

EbonJaeger avatar Mar 24 '25 01:03 EbonJaeger