On Wed, 07 Feb 2024 10:21:01 -0700,
c...@luigifab.fr wrote:
> Hi,
>
> Don't kill me, I'm not sure.
>
> I'm the upstream and packager of awf-gtk, the continuation of awf-git.
> I discovered Parabola from Repology:
> https://repology.org/project/awf-widget-factory/versions#parabola
>
> - https://aur.archlinux.org/packages/awf-git
> - https://aur.archlinux.org/packages/awf-gtk2
> - https://aur.archlinux.org/packages/awf-gtk3
> - https://aur.archlinux.org/packages/awf-gtk4
>
> The old pkgbuild files seem to be identical:
> - https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=awf-git
> - https://git.parabola.nu/abslibre.git/tree/pcr/awf-git/PKGBUILD
> (except arch?)
>
> Can I propose/submit my Arch Linux AUR pkgbuilds into Parabola PCR repo?
Hi, sorry for the late reply!
Many of the packages in PCR are indeed imported from the AUR, but
unfortunately the process is very manual, which means that packages
can easily become out-of-date. Especially since we have fewer
maintainers than we used to. (And for me personally, I dread doing
those manual syncs because it fills me with guilt that I haven't
improved the process yet.)
I can't upload just yet because the Git server is acting up, but I've
got your awf-extended imported to Parabola now. So I guess expect to
see it in Parabola tomorrow? Cheers!
If you care for feedback, let me offer a quick code-review of your AUR
PKGBUILDs:
> +pkgname=awf-gtk2
In cases like this where multiple packages are being built from the
same sources, it's preferable to have a single "split" PKGBUILD with a
`pkgbase` variable and `pkgname` set to an array, and separate
`package_${pkgname}` functions, like:
pkgbase=awf-extended
pkgname=(awf-extended-gtk{2,3,4})
package_awf-extended-gtk2() {
depends+=('gtk2')
...
}
package_awf-extended-gtk3() {
depends+=('gtk3')
...
}
package_awf-extended-gtk4() {
depends+=('gtk4')
...
}
Also, the pkgbase should probably be `awf-extended`, not just `awf`.
> +depends=('gtk2' 'hicolor-icon-theme')
> +#makedepends=('autoconf' 'automake' 'desktop-file-utils' 'gcc' 'gettext'
> 'gtk2')
It isn't nescessary to list makedepends on
- anything in the "base-devel" group (unless the package being built
is itself is in base-devel) (this means 'autoconf', 'automake',
'gcc', and 'gettext')
- anything listed in depends=() (this means 'gtk2')
And 'desktop-file-utils' is redundant, because gtk{2,3,4} all already
depend on it.
> +conflicts=('awf-git')
I'm not sure how relevant this advice is for AUR users, but: I'd also
add `replaces=('awf-git')` so that users will be prompted to upgrade
from awf-git to this.
> +source=("https://github.com/luigifab/awf-extended/archive/v${pkgver}/awf-extended-${pkgver}.tar.gz";)
> +md5sums=("f0649d6faf1eb96f49da8098d016c27e")
- md5 is a pretty weak algorithm, prefer to use sha1 or sha256 or
something.
- Tip: The `updpkgsums` program is handy for automatically updating the
{whatever}sums arrays--and I can tell you didn't use it because it
uses single-quotes and you used double-quotes :)
> +prepare() {
> + mv "awf-extended-$pkgver" "$pkgname-$pkgver"
> + cd "$pkgname-$pkgver"
> + sed -i 's/ -eq 3/ -eq -1/g' configure.ac
> + sed -i 's/ -eq 4/ -eq -1/g' configure.ac
The `sed` commands here prepare() probably deserve comments about what
they're doing. But ideally configure.ac would be enhanced to add
`--disable-gtk{2,3,4}` flags. (But in a split PKGBUILD such disabling
is unnecessary anyway.)
> + touch {NEWS,AUTHORS,README,ChangeLog}
> + mv LICENSE COPYING
These lines threw me through a loop for a minute. You should instead
simply pass the `[foreign]` flag to `AM_INIT_AUTOMAKE`. I sent you a
PR! https://github.com/luigifab/awf-extended/pull/9
> +}
> +
> +build() {
> + cd "$pkgname-$pkgver"
> + autoreconf -fi
`autoreconf` usually goes in `prepare()`, not `build()`.
> +package() {
> + cd "$pkgname-$pkgver"
> +
> + install -Dpm 755 "src/$pkgname" "$pkgdir/usr/bin/$pkgname"
Ideally, `Makefile.am` would be enhanced to do most of this stuff, so
that the `package*()` functions(s) would simply be
cd "$pkgname-$pkgver"
make install-gtk{2,3,4}
> +license=('GPL3')
> ...
> + install -Dpm 644 COPYING "$pkgdir/usr/share/licenses/$pkgname/COPYING"
No need to create `/usr/share/licenses/$pkgname/` if the values in
`license=()` all exist at `/usr/share/licenses/common/${license}`.
> + install -dm 755 "$pkgdir/usr/share/icons/hicolor/"
> + for file in icons/*/*/awf.png; do mv $file
> ${file/\/awf.png/\/$pkgname.png}; done
> + for file in icons/*/*/awf.svg; do mv $file
> ${file/\/awf.svg/\/$pkgname.svg}; done
> + cp -a icons/* "$pkgdir/usr/share/icons/hicolor/"
FWIW, I cleaned this up as:
find icons -type f -printf '%P\0' | while read -r -d '' file; do
install -D -m644 -T ./icons/"$file"
"$pkgdir/usr/share/icons/hicolor/${file/awf/$_binname}"
done
> + install -Dpm 644 "debian/$pkgname.1"
> "$pkgdir/usr/share/man/man1/$pkgname.1"
> + install -Dpm 644 "debian/$pkgname.fr.1"
> "$pkgdir/usr/share/man/fr/man1/$pkgname.1"
- This is broken in the AUR right now--these are in the `debian-gtk/`
directory (which it seems you've updated in the PKGBUILDs in
awf-extended.git, but not in the AUR).
- These should probably be moved to a `man/` folder or `src/` or
something; they aren't Debian specific.
> + for file in src/po/*.po; do
> + code=$(basename "$file" .po)
> + install -dm 755 "$pkgdir/usr/share/locale/$code/LC_MESSAGES/"
> + msgfmt "src/po/$code.po" -o
> "$pkgdir/usr/share/locale/$code/LC_MESSAGES/$pkgname.mo"
> + done
> +}
The `msgfmt` calls should probably be done in `build()`.
And as a final note: I'm generally not a fan of having packaging
(`debian-*/`, `fedora/`, `archlinux/`, `mageia/`, `opensuse/`) in the
upstream repo. It's just another opportunity for things to drift
apart, and often means that distro maintainers have twice as much
stuff to review: the build system itself, and how the upstream thinks
packaging should be done, instead of just having to look at the build
system.
--
Happy hacking,
~ Luke T. Shumaker
_______________________________________________
Dev mailing list
Dev@lists.parabola.nu
https://lists.parabola.nu/mailman/listinfo/dev
