[aur-general] TU membership application

[Eli Schwartz]

On 8/17/19 10:51 PM, Santiago Torres-Arias via aur-general wrote:
> On Fri, Aug 16, 2019 at 03:19:56PM -0400, Jean Lucas via aur-general wrote:
>> Hi all,
>>
>> Thank you for your time, and thank you to all who help make Arch a great OS!
> 
> Always happy to help! :)
> 
> It's customary to review PKGBUILDS for new applicants.  This is somewhat
> of a quick/cursory review over 3 random packages as I've been in
> conferences for the whole week.

I haven't looked at Jean's packages myself, but I'm not sure some of
these things you point to are actually problems.


> 
> == Overall ==
> 
> - It appears you need quote strings way more everywhere, from deps, to licenses
>   to variables....

Quote strings are only necessary for variable expansions which could
potentially undergo word splitting. For declaring an array (deps,
licenses) it's completely unnecessary as you know when writing the
PKGBUILD if there are spaces (and most makepkg fields don't permit
spaces anyway).

Admittedly I think single-quoting array keys looks prettier. That's a
personal opinion though.

> - Consider that base-devel is assumed to exist for makedepends and (iirc).

This is not great if it is in makedepends, but honestly we still haven't
fully fixed the official repos for this.

> == Beaker ==
> 
> - This depends array has to be wrong
> - This makedepends array too. you should make sure things aren't depending on
>   py2 anymore

What's necessarily wrong with this? I don't like py2 either, but just
because something uses it doesn't mean it has no reason to. What
specifically made you think it isn't needed?

What is wrong with the dependencies that it "must" be wrong? From a
cursory inspection it seems to be some sort of electron thingy, which
would hopefully use community/electron but life isn't perfect.

Depending on glibc and gcc-libs is a bikeshed topic that TUs/Devs don't
agree on. The rest of the dependencies could plausibly be linked to by
whichever version of prebuilt electron is being downloaded by the build
system.

> - I'm also a little confused, did you take over the namespace of another
>   project called beaker? Why not just call this beaker browser?
> 
> == Oxy ==
> 
> - I think you should document why you're cherry-picking that commit rather than
>   using a tag. Admittedly this is probably upstream's fault, but still, better
>   to be clear.

Upstream is amazing and doesn't use git tag. The cherry-picked commit
has the commit message "Call it a version". This is obvious enough
causes that I don't actually feel bad about the lack of comments. :)

> - Again, I think your depends are either too verbose or wrong.  

There's exactly one depends, which is gcc-libs. Again, a bikeshed topic.
I will loudly proclaim my own belief in not depending on gcc-libs or
anything else in *base*, but I won't tell anyone they are *wrong* for
doing so themselves.

(Obviously makedepends on base-devel is still against the packaging
guidelines.)

> == stf ==
> 
> - This appears to me it's a -bin package

Why? It looks like some sort of standard js-based source package on the
NPM registry.

> - npm -i -g --prefix seems like a good way to overwrite a bunch of system files
>   and/or cause a bunch of file conflicts

npm install -g --prefix="$pkgdir" is actually how you are supposed to
install npm stuff -- it "globally" installs it to the packaging
directory so that pacman will install it to /usr, so it should never
conflict with anything. This seems fine to me.

(I have personal issues with npm as a technology, and prefer to npm
install into $srcdir then use cp because it feels at least mildly
cleaner -- see my rapydscript-ng package -- but stf doesn't seem any
less valid and some official packages do the same thing he does).


> - I think you can use $pkgname more often, namely when resolving the url and
>   resolving the tgz file

I've seen it both ways extremely often. I think some people actually
insist on hardcoding $pkgname everywhere, because they want to preserve
the possibility of users forking the PKGBUILD, modifying the pkgname,
and still have everything work without having to fix up all pkgname
references.

> - I'm curious to know where you got those depends arays, they seem to be a
>   little off... do you really need python, graphicksmagic and protobuf to
>   basically extract a tarball?

Not to extract a tarball. This is npm. It's not just extracting a
tarball, it is also probably downloading half the internet during build,
and maybe compiling G-d-knows-what after the unauthenticated download.
Because npm. And npm sucks. But the package itself seems fine, and I'd
need to actually look in depth at the build in order to decide if those
makedepends raise a red flag.

> - I'm also not sure why *everything* is just blindly put on /usr

It's not? npm install (like make install except that npm is obviously
terrible because javascript desktop programming) is responsible when
given --prefix="$pkgdir/usr" to place "everything" in the places where
npm thinks they should go. It's like passing DESTDIR="$PKGDIR"
PREFIX=/usr to a Makefile. It seems totally like the correct thing to do
for an npm project.

> == Conclusion ==
> 
> - I think you are on the right path, but some decisions made me wonder whether
>   your sponsors actually reviewed the PKGBUILDS with you. 

If this is the worst that the applicant has, then that's not very bad at
all. :D :D

-- 
Eli Schwartz
Bug Wrangler and Trusted User

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 1601 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/aur-general/attachments/20190818/2171e91c/attachment.sig>