/Transferring OP's reply to the proper mailing list/.
On Sun, Jul 2, 2023 at 6:59 PM Robin Candau <antiz@archlinux.org> wrote:
Le 02/07/2023 à 04:20, james smith a écrit :
I am trying to package this https://github.com/lemmygtk/lemoa this is my pkgbuild any tips on how to improve it before I send it to aur is appreciated
Hi James,
_/Quick note before reviewing your PKGBUILD:/_ /When asking for a review, try to make your PKGBUILD publicly readable somewhere (by posting it in paste service for instance, like https://bpa.st/ or https://gist.github.com/). This is way easier to track as the discussion progresses and way more "trust worthy" than an attached file sent to a mailing list. //For everyone's context, here's the initial PKGBUILD provided for a review:/ https://bpa.st/TXEW2
In it's current state, your PKGBUILD won't work and needs some modifications and improvements. Here's what I can list after a quick review:
- The comments at the top provided by the PKGBUILD example file should be removed.
- You can remove variables/arrays that are empty/unused in your PKGBUILD (epoch, groups, checkdepends, etc...).
- Consider filling out the maintainer info at the top of the PKGBUILD. This isn't mandatory but it is recommended and appreciated.
- The pkgname you chose refers to a VCS package ("lemoa*-git*"). If you indeed want your package to be a VCS package (meaning that it relies on the latest upstream commit instead of a specific tag/release) then you have to adapt your PKGBUILD accordingly with a proper "pkgver()" function [1]. Otherwise, switch your "pkgname" to "lemoa" and update the rest of the PKGBUILD accordingly.
- 'GNU' isn't a valid license. The correct license for "lemoa" currently is 'GPL3' [2]. See also the license section of the PKGBUILD wiki page for more information [3].
- The 'Gtk' package listed in the "depends" array isn't a valid package. "Lemoa" seems to depends on GTK-4 [4] which can be installed via the "gtk4" package in Arch [5].
- Upstream documentation also lists "rust, cargo and pkg-config" as "Build dependencies" [4]. According to that, you should add the "cargo" package (which also provides "rust") to your "makedepends" array. However, you don't have to explicitly list the "pkg-config" package (provided by the "pkgconf" package in Arch [6]) because it is a member of the "base-devel" meta-package [7] which is assumed installed during build-time [8].
- The "install" variable expects the name of an optional ".install" script used to perform tasks before or after the install, the upgrade and/or the removal of the package [9], it isn't meant to store a "raw" command like you did. At first glance, there's no need for an install script in your case so you can remove that variable.
- The "source" array is wrong and will fail to download the repo. It should be "git*+*$url" instead of "git*=*$url". See the VCS sources section of the wiki for more information [10]. Just for the sake of it, I'll add the usual trailing ".git" at the end, like so: source=("git+${url}.git")
- Because the sources are not static in a VCS package, you have to explicitly skip the checksum by adding 'SKIP' to the "md5sums" array [11]. Letting it empty will not work. On a side note, I recommend switching to a more modern and secure hash algorithm generally speaking ("sha256sums" for instance).
- You shouldn't "git clone" the repo inside the "build()" function. This is implicitly done beforehand by filling the correct/proper sources in the "source" array.
- The "ninja -C _build install" part as to be called within the "package()" function. You'll also need to setup the destination directory to "$pkgdir" which is the temporary directory where "makepkg" builds and bundles the package [12]. See the "package()" section of the Meson packaging guidelines for more information [13].
Overall, I advice you to read closely the three following Arch wiki pages which should contain most of the information you'll need to write a proper PKGBUILD for "lemoa-git": - PKGBUILD [14] - VCS Packaging Guidelines [15] - Meson Packaging Guidelines [16]
I hope this helps :) Don't hesitate to ask for details or further questions if needed!
[1] https://wiki.archlinux.org/title/VCS_package_guidelines#The_pkgver()_functio... [2] https://github.com/lemmygtk/lemoa/blob/main/LICENSE [3] https://wiki.archlinux.org/title/PKGBUILD#license [4] https://github.com/lemmygtk/lemoa#build-dependencies [5] https://archlinux.org/packages/extra/x86_64/gtk4/ [6] https://archlinux.org/packages/core/x86_64/pkgconf/ [7] https://archlinux.org/packages/core/any/base-devel/ [8] https://wiki.archlinux.org/title/PKGBUILD#makedepends [9] https://wiki.archlinux.org/title/PKGBUILD#install [10] https://wiki.archlinux.org/title/VCS_package_guidelines#VCS_sources [11] https://wiki.archlinux.org/title/VCS_package_guidelines#Authentication_and_s... [12] https://wiki.archlinux.org/title/Creating_packages#Defining_PKGBUILD_variabl... [13] https://wiki.archlinux.org/title/Meson_package_guidelines#package() [14] https://wiki.archlinux.org/title/PKGBUILD [15] https://wiki.archlinux.org/title/VCS_package_guidelines [16] https://wiki.archlinux.org/title/Meson_package_guidelines
-- Regards, Robin Candau / Antiz
Le 02/07/2023 à 21:26, james smith a écrit : fixed some of the suggestions you had https://mystb.in/AuroraStreamAdjacent is it ok that my email is md5 hashed instead of just the email? Well, according to the fact that filling your email address at the top of the PKGBUILD isn't mandatory per say, this is better than nothing but not so "accessible". If this is a matter of protecting yourself from eventual bots or something, you could do something like "address[at]mail[dot]com" which is eventually more accessible. But the final decision is up to you, no obligation there. Regarding your PKGBUILD, there are still some modifications to make as far as I can see: - The maintainer information has to be contained inside a comment, otherwise your PKGBUILD won't work: # Maintainer: Your Name <Your email address> - The "pkgver()" function does not replace the "pkgver" variable, it just updates it automatically each time makepkg is ran against the PKGBUILD (according to the latest upstream commit). You should put back the "pkgver" variable where it was and put the "pkgver()" function before the "build()" function. Once done, building the package against the PKGBUILD will automatically update the "pkgver" value. - Generally speaking, you should list every first level dependencies even if some are already implicitly satisfied by another package. However, some exceptions can be done when you're absolutely sure that a package cannot ever stop pulling another one as a dependency. According to that, you don't necessarily have to explicitly list "rust" as a dependency as it is already pulled by "cargo" and we're sure that "cargo" won't ever stop pulling "rust" as a dependency since "cargo" is the "rust package manager". See the "depends" section of the PKGBUILD wiki page for more information [1]. - No need to keep the empty "noextract" array. - One thing I forgot to mention in my initial reply is that your package has to "conflict" and "provide" the "lemoa" package in order to avoid issues if someone tries to install both on the same system (even though the "lemoa" package does not exists */yet/*). To do so you have to use a "provides" and "conflicts" arrays in your PKGBUILD. See the related section of the PKGBUILD arch wiki page [2] [3]. [1] https://wiki.archlinux.org/title/PKGBUILD#depends [2] https://wiki.archlinux.org/title/PKGBUILD#provides [3] https://wiki.archlinux.org/title/PKGBUILD#conflicts -- Regards, Robin Candau / Antiz
participants (1)
-
Robin Candau