On Thu, 2022-07-28 at 22:55 -0400, Ben Westover wrote:
> Hello Luca and Michel,
>
> On 7/28/22 5:50 PM, Michel Alexandre Salim wrote:
> > Hi Ben,
> > I was independently working on pacman, and (thanks to the name
> > collision with the existing pacman package) didn't notice this until
> > it's mostly done.
>
> That existing package is orphaned anyway, so it's annoying that it's
> already there and not even being maintained. I do think it's still a
> good idea to keep the current package name so people don't get confused
> and accidentally install a potentially dangerous package manager for
> another distribution instead of an arcade game.
>
> > My use case is helping make systemd/mkosi CI easier (since it's hosted
> > on GitHub, which provides Ubuntu LTS builders) - I'll flag this to
> > some relevant people so they can help get this sponsored.
> >
> > PS archlinux-keyring is on its way to unstable, and per some feedback
> > the keyring target directory is moved to the standard Debian path:
> >
> > https://salsa.debian.org/michel/archlinux-keyring/-/blob/main/debian
> > /patches/use_std_keyring_dir.diff
> >
> > might want to apply this to your pacman, and configure pacman to use
> > this path:
> >
> > https://gitlab.archlinux.org/pacman/pacman/-/merge_requests/11
>
> Okay, I've added the patch to my package and configured the keyringdir
> option in debian/rules.
>
> On 7/28/22 6:27 PM, Luca Boccassi wrote:
> > Hi,
> >
> > I can sponsor this.
> >
> > A few remarks, aside from the keyring change mentioned by Michel:
> >
> > - all the doc build-dependencies (asciidoc, doxygen, help2man) can be
> > marked with <!nodoc> so that nodoc builds can be done
> > - are curl and fakechroot really needed to build the package, or are
> > they just used by self tests? if they are used only by tests, mark them
> > as <!nocheck>
> > - is pkgconf really needed instead of pkgconfig, which is the default?
> > - you need to add a libalpm-dev and ship the headers, pkgconfig file,
> > unversioned .so and manpage in it, instead of in libalpm13, and remove
> > the lintian override
> > - libalpm13 is missing Pre-Depends: ${misc:Pre-Depends}
> > - no need to specify the libarchive-tools and libgpgme11 dependencies
> > on libalpm13, they will be autogenerated
> > - does libalpm13 really need to depend on the binary curl executable?
>
> Thanks, all of those have been fixed.
>
> > - makepkg should not depend on build-essential nor on sudo
>
> I was under the impression that makepkg depended on sudo, but after a
> deeper look into the code, it appears to fall back on su if sudo is not
> detected. I put build-essential there since makepkg expects base-devel,
> the Arch Linux equivalent of build-essential, to be installed. I've now
> removed sudo from Depends and moved build-essential to Recommends since
> in most cases you would want it installed but it's not required.
>
> > - no need to manually specify the dependency on libalpm13 in makepkg,
> > it will be autogenerated
> > - libalpm13 is missing the symbols file, you can generate it after
> > building the library with:
> > dpkg-gensymbols -plibalpm13 -edebian/tmp/usr/lib/x86_64-linux-gnu/libalpm.so.13.0.1 -Odebian/libalpm13.symbols
> > - makepkg is missing a dependency on ${perl:Depends}
>
> Fixed
>
> > - are you sure all of these can run on GNU/Hurd and debian/kFreeBSD? If
> > not, use 'linux-any' instead of 'any' as the architecture
>
> Funny you should say that; I actually have a Debian GNU/kFreeBSD system
> that I installed out of curiosity last year. When I was originally
> making this package, I tested it out on that system and it worked.
>
> > - it is not necessary anymore to specify the build system in
> > debian/rules, meson is autodetected
> > - use execute_before_dh_auto_clean instead of override_
>
> Fixed
>
> > - 228 tests fail when running in a pbuilder chroot, this is a strong
> > hint that the build might fail once uploaded
>
> I use sbuild instead of pbuilder since it's considered to be legacy at
> this point. As Michel noted, these failures are solved by two additional
> build depends (one of which you should've already had) which I've added.
>
> > - you should try and fix the reproducible build, rather than disabling
> > it in the CI
>
> I've tested reproducibility with reprotest and it's perfectly
> reproducible. Salsa CI fails a bunch of tests on its second
> reproducibility build, and I haven't been able to find the root cause.
> It's been suggested to me that Salsa CI's reprotest job might be faulty.
> I'll do some more research into the topic when I have time.
>
> > - the GPL-2+ in debian/copyright says in the last paragraph:
> > "On Debian systems, the full text of the GNU General Public
> > License version 3 can be found in the file"
> > instead of version 2
>
> Fixed
>
> Thank you both for your interest and deep review of this package! I
> learned a lot of minor Debian things that I didn't know about before.
> I've published my changes to Salsa and Mentors. You've helped me to
> address the weaker points of my package that I wasn't so sure about
> (dependencies and packaging a library) and I've learned more in my
> Debian packaging journey as a result!
Still a couple of Lintian warnings to fix, then you can finalize the
changelog and I'll upload:
I: pacman-package-manager source: duplicate-long-description libalpm-dev libalpm13 [debian/control]
I: libalpm-dev: extended-description-is-probably-too-short
I: libalpm13: extended-description-is-probably-too-short
I: libalpm13: package-contains-empty-directory [usr/share/libalpm/hooks/]
I: makepkg: package-contains-empty-directory [usr/share/makepkg-template/]
This one doesn't need to be fixed in the package, just send a patch
upstream and it can be picked up in the next version:
I: pacman-package-manager: typo-in-manual-page "allows to" "allows one to" [usr/share/man/man8/pacman.8.gz:264]
--
Kind regards,
Luca Boccassi
Attachment:
signature.asc
Description: This is a digitally signed message part