Discussion:
TU Membership Application
(too old to reply)
Brett Cornwall via aur-general
2018-10-25 14:26:11 UTC
Permalink
I am being sponsored by dvzrv.

I've been working in the AUR since 2014 as 'Ainola'. I've had a few of
my packages adopted into [community], such as gnome-mpv, csound, and
qutecsound (the latter two being adopted by dvzrv).

I am also an active contributor to the LibreOffice infrastructure team.

I would like to get valuable tools promoted into [community], such as
residualvm or the 'pass' plasmoid (after prodding upstream to have a
formal release). Other packages that I do not maintain in the AUR would
be ripe for picking as well, such as sc-controller or ttf-lato. I would
also work with dvzrv on maintaining pro-audio packages, many of which
were abandoned when SpepS left.
David Runge
2018-10-25 18:22:00 UTC
Permalink
Post by Brett Cornwall via aur-general
I am being sponsored by dvzrv.
I hereby ACK that and apologize for the confusion the last time (again).
Post by Brett Cornwall via aur-general
I would like to get valuable tools promoted into [community], such as
residualvm or the 'pass' plasmoid (after prodding upstream to have a formal
release). Other packages that I do not maintain in the AUR would be ripe for
picking as well, such as sc-controller or ttf-lato. I would also work with
dvzrv on maintaining pro-audio packages, many of which were abandoned when
SpepS left.
That would be very awesome!

The best of luck to you and let the discussion begin!
This seems to be a good month for TU applications.

Best,
David

P.S.: As you've just created a new pgp key pair for your address, please
make sure to upload the pubkey to the keyservers!
--
https://sleepmap.de
Levente Polyak via aur-general
2018-10-26 18:44:38 UTC
Permalink
Hi Brett
Post by David Runge
P.S.: As you've just created a new pgp key pair for your address, please
make sure to upload the pubkey to the keyservers!
can you please fix this and make your gpg key available somewhere?
Brett Cornwall via aur-general
2018-10-26 20:10:07 UTC
Permalink
Post by Levente Polyak via aur-general
can you please fix this and make your gpg key available somewhere?
I've pushed 0F8E620A up.
Jelle van der Waa
2018-10-25 20:22:34 UTC
Permalink
Post by Brett Cornwall via aur-general
I am being sponsored by dvzrv.
I've been working in the AUR since 2014 as 'Ainola'. I've had a few of my
packages adopted into [community], such as gnome-mpv, csound, and qutecsound
(the latter two being adopted by dvzrv).
I am also an active contributor to the LibreOffice infrastructure team.
What kind of tasks/roles do you handle for LibreOffice in the
infrastructure team?
Post by Brett Cornwall via aur-general
I would like to get valuable tools promoted into [community], such as
residualvm or the 'pass' plasmoid (after prodding upstream to have a formal
release). Other packages that I do not maintain in the AUR would be ripe for
picking as well, such as sc-controller or ttf-lato. I would also work with
dvzrv on maintaining pro-audio packages, many of which were abandoned when
SpepS left.
I always like it when applicants want to improve/work on
orphan/neglected packages in the repos!


Couldn't find much issues in your packages!
--
Jelle van der Waa
Brett Cornwall via aur-general
2018-10-25 21:14:49 UTC
Permalink
What kind of tasks/roles do you handle for LibreOffice in the infrastructure team?
I've been working with the team for a few years now. I'd say the
majority of my work would be converting the legacy, manually-configured
environments to Saltstack states and de-dockerizing some stuff. I've
also been helping out with overhauling monitoring/alerting since it was
previously not functioning very well.

A good overview can be seen in our monthly meeting minutes:

https://pad.documentfoundation.org/p/infra

Our git repos are private, unfortunately - more of a precaution than
damning us for bad security practices. :)
Levente Polyak via aur-general
2018-11-05 18:48:19 UTC
Permalink
Post by Brett Cornwall via aur-general
I am being sponsored by dvzrv.
I've been working in the AUR since 2014 as 'Ainola'.
Hi Brett,

some small questions and hints first:

It looks like several packages have different issues preventing to build
in clean chrooted environments properly. Did you take a look at the
devtools package and building packages in clean chroots so far?

What software/tool do you using to track all the new ustream releases?

You still seem to use `mksrcinfo` for generating SRCINFO files, it was
deprecated in favor of native `makepkg --printsrcinfo` you may want to
use that in the future.

I have noticed that mostly all git packages lack sufficient
provides/conflicts on the basic non-git name schema and/or makedepends
on git itself, would be nice to keep in mind

Also i notices there are multiple packages that store a tarball in the
AUR source repo that contain things like icons, please don't miss-use
the AUR as a storage for tarball artifacts.

some findings after reading your PKGBUILDS, can you please take a look
and process the following feedback:

ags
- upstream Makefile doesn't seem to respect CPPFLAGS try to either get a
patch upstream or if that fails extend CFLAGS with CPPFLAGS in
the PKGBUILD.

creeper-world
- upstream source/url provides a https endpoint
- you can't reference the PKGBUILD like in prepare(), try building in a
clean chroot via extra-x86_64-build
- don't think pkgdesc should ever end with a dot

creeper-world2
- upstream source/url provides a https endpoint
- don't think pkgdesc should ever end with a dot
- unzip to prepare the env is more of a prepare() candidate

creeper-world3
- upstream source/url provides a https endpoint
- you can't reference the PKGBUILD like in prepare(), try building in a
clean chroot via extra-x86_64-build
- not a big fan of fiddling with PKGEXT even if its "just the AUR" but
well

csound-blue
- please don't use the AUR to store a tar.gz as a source
- provides/conflicts on itself doesn't do anything useful
- is there a reason for not stripping?
- this looks like a -bin package as you are re-distributing pre-compiled
files, why not just build from source instead? :)

gam
- don't think pkgdesc should ever end with a dot
- there is some unused client_secrets.patch sitting around in the source
repo
- python2-oauth2client dependency is missing
- this package create cluttering files across the filesystem if you run
gam as root, it will have untracked pyc files in /usr/share/gam/
you need to compile them if no distutils is provided

gnome-xcf-thumbnailer
- prepare() shall never package into $pkgdir

gtk-theme-adwaita-tweaks
- don't think pkgdesc should ever end with a dot

gtk-theme-adwaita-tweaks-git
- lack of provides/conflicts
- should pull via git+https://
- don't think pkgdesc should ever end with a dot
- git makedepends is missing
- the dark variant isn't published as "Adwaita Tweaks Dark" i don't
think this works compared to the non git variant, it should honor
the assets-dark folders and mimic what the release tarballs provide

gtk-theme-minwaita
- don't think pkgdesc should ever end with a dot

hib-dlagent
- none-url.patch is not a very unique name for a remote soruce, it could
potentially clash, prepend with $pkgname can help

imv-git
- lack of provides imv

i3lock-media-keys
- don't think pkgdesc should ever end with a dot

interception-ctrl2esc-git
- don't think pkgdesc should ever end with a dot
- git makedepends is missing
- provides/conflicts not proper
- is there a reason to have interception- prefix? imo ctrl2esc-git would
be the better naming here plus provides/conflicts on ctrl2esc
- you should use CMAKE_INSTALL_PREFIX=/usr and DESTDIR for $pkgdir

invada-studio-plugins-lv2
- don't think pkgdesc should ever end with a dot

lazy-ips-git
- don't think pkgdesc should ever end with a dot
- git makedepends is missing
- you can't reference the PKGBUILD like in prepare(), try building in a
clean chroot via extra-x86_64-build
- lack of provides/conflicts

linkchecker
- don't think pkgdesc should ever end with a dot

minecraft-technic-launcher
- don't think pkgdesc should ever end with a dot

nexuiz
- don't think pkgdesc should ever end with a dot
- upstream source/url both provides a https endpoint
- not a big fan of fiddling with PKGEXT even if its "just the AUR" but
well
- extracting and patching should be done in prepare()
- please don't use the AUR to store a tar.gz as a source, there is also
an orphan bin-links.tar.gz file additionally to the used
nex-icons.tar.gz checked into the AUR

pam_encfs
- doesn't respect neither CFLAGS, CPPFLAGS, LDFLAGS please make sure
this is always applied for c/cpp

pd-extended
- non unique filename that only contains the $pkgname, must always be
unique including the pkgver so avoid clashes
- doesn't respect neither CFLAGS, CPPFLAGS, LDFLAGS please make sure
this is always applied for c/cpp
- orphan file makefile.am.patch in the tree
- upstream source/url both provides a https endpoint

plasma5-applets-plasma-pass-git
- don't think pkgdesc should ever end with a dot
- conflicts missing on plasma5-applets-plasma-pass

python-google-auth-httplib2-git
- this is a split package creating two artifacts, you should not have
provides/conflicts on a global level, you need to put them in the
separate variants package_*() function once for python- and once for
python2-
- git makedepends is missing

python-humblebundle
- is there any reason to not just pull from github which also includes
the LICENSE? currently the LICENSE is separately pulled from git
master and it doesn't even have any unique file identifiers

python-image
- don't think pkgdesc should ever end with a dot
- don't use the hash based pythonhosted.org endpoint, there is a non
changing variant that is more suitable for packaging as it doesn't
very other then the $pkgver

python-image-git
- don't think pkgdesc should ever end with a dot
- git makedepends is missing

python-oath
- don't think pkgdesc should ever end with a dot
- each variant must at least depend in its package_* func
to at least python/python2
- there are neat tests in the sources, please try to run a
check() function

python-vipaccess
- don't think pkgdesc should ever end with a dot
- you should add all the depends from the package_* funcs
to the makedepends so the check functions don't start
to uselessly download packages via pip
- setuptools is a hard depends and must be added in package_*
as the entry_point feature is used in the setup.py for
/usr/bin/vipaccess
- /usr/bin/vipaccess is provided in both variants it should by
something like vipaccess-py2 but this looks like a pure CLI
tool that is not really usable as a library, is it? If so,
it would be appropriate to just purge the python2 variant.

python2-image-git
- don't think pkgdesc should ever end with a dot
- git makedepends is missing
- why not use a split package via python-image-git like you do
for python-image?
- conflicts missing on python2-image
- LICENSE and README should be distributed, like in python-image-git

residualvm
- don't think pkgdesc should ever end with a dot

scanbd
- please don't use a install file to reference to the wiki, its not
meant to be used for such general purpose documentation
- upstream source provides a https endpoint

ttf-material-wifi-icons-git
- should pull via git+https://
- lack of provides/conflicts

unruu
- should use unique filename prefix with $pkgname-$pkgver
- should just pass --prefix=/usr to ./configure and call
a proper make DESTDIR="${pkgdir}" install

unruu-git
- git makedepends is missing
- lack of provides/conflicts
- should just pass --prefix=/usr to ./configure and call
a proper make DESTDIR="${pkgdir}" install



cheers,
Levente
Eli Schwartz via aur-general
2018-11-05 19:32:23 UTC
Permalink
Post by Levente Polyak via aur-general
gnome-xcf-thumbnailer
- prepare() shall never package into $pkgdir
That's a write error, makepkg explicitly runs chmod on "${pkgdir}" in
order to strip read/write permissions and forbid you from touching it
before package() is run:
https://git.archlinux.org/pacman.git/tree/scripts/makepkg.sh.in#n1679

...

Actually, come to think of it, if you try touching "${pkgdir}" during
prepare() then it won't exist yet, and then after prepare() does its
thing we forcibly remove the directory anyway, recreate it, and set the
restrictive permissions. I wonder if it's a bug that we don't error out
harder on this...

But depending on whether/how makepkg errors in the previous makepkg
attempt, this directory will still exist and it will indeed be chmod
a-rw at the time prepare is executed.

The rest of the time, install -d has hidden the fact that the directory
simply does not exist at all.

Either way, this file is definitely not being packaged.
--
Eli Schwartz
Bug Wrangler and Trusted User
Brett Cornwall via aur-general
2018-11-06 16:38:59 UTC
Permalink
Post by Levente Polyak via aur-general
Hi Brett,
Thank you for such a thorough vetting, Levente!

I'm fixing these ASAP.
Brett Cornwall via aur-general
2018-11-08 00:41:06 UTC
Permalink
I'm nearly done with following your excellent suggestions but I have
responses and questions.
Post by Levente Polyak via aur-general
It looks like several packages have different issues preventing to build
in clean chrooted environments properly. Did you take a look at the
devtools package and building packages in clean chroots so far?
I must sheepishly apologize. These new tools simplify everything.
Post by Levente Polyak via aur-general
What software/tool do you using to track all the new ustream releases?
urlwatch on a daily timer.
Post by Levente Polyak via aur-general
You still seem to use `mksrcinfo` for generating SRCINFO files, it was
deprecated in favor of native `makepkg --printsrcinfo` you may want to
use that in the future.
Thank you, switched!
Post by Levente Polyak via aur-general
I have noticed that mostly all git packages lack sufficient
provides/conflicts on the basic non-git name schema and/or makedepends
on git itself, would be nice to keep in mind
A silly oversight that will be enforced now that I'm learned in the
ways of proper tooling.
Post by Levente Polyak via aur-general
Also i notices there are multiple packages that store a tarball in the
AUR source repo that contain things like icons, please don't miss-use
the AUR as a storage for tarball artifacts.
I should have known better and - at the very least - removed them before
submitting my application. I've taken care of nearly all of them and
have bowed my head in shame.
Post by Levente Polyak via aur-general
- don't think pkgdesc should ever end with a dot
The descriptions are often sentences, so would it not reason to end them
with a period?
Post by Levente Polyak via aur-general
- not a big fan of fiddling with PKGEXT even if its "just the AUR" but
well
For a package destined for the repositories I would not fiddle and
endeavor to reverse such fiddling; however, the compression time for
large games is enormous only to decompress right after. Should it, for
the sake of correct-ness, be reversed even for the packages doomed
forever to live in the AUR?
Post by Levente Polyak via aur-general
interception-ctrl2esc-git
- is there a reason to have interception- prefix? imo ctrl2esc-git would
be the better naming here plus provides/conflicts on ctrl2esc
I normally agree (and I originally had it named that way), however...

- This is an 'interception tools' plugin... not reason enough to have the package name change, but..
- caps2esc is an older X program, so interception's variant had to be
named 'interception-caps2esc'. Naming this 'interception-ctrl2esc'
follows the pattern for consistency/less confusion.

With that said, should it still be named ctrl2esc?
Ivy Foster via aur-general
2018-11-08 01:54:44 UTC
Permalink
Post by Brett Cornwall via aur-general
Post by Levente Polyak via aur-general
- don't think pkgdesc should ever end with a dot
The descriptions are often sentences, so would it not reason to end them
with a period?
In the case of actual sentences, it's just convention to be consistent
with the packages whose descriptions are sentence fragments. It's a
bit arbitrary, granted.
Post by Brett Cornwall via aur-general
Post by Levente Polyak via aur-general
- not a big fan of fiddling with PKGEXT even if its "just the AUR" but
well
For a package destined for the repositories I would not fiddle and endeavor
to reverse such fiddling; however, the compression time for large games is
enormous only to decompress right after. Should it, for the sake of
correct-ness, be reversed even for the packages doomed forever to live in
the AUR?
It's just not the prerogative of the AUR contributor to make that
decision for the end-user who's going to be building the package. They
can very easily configure their own makepkg.conf to use uncompressed
tarballs if they so desire. Basically, a PKGBUILD shouldn't override
makepkg.conf unless there's a very good reason.

Cheers,
Ivy
Brett Cornwall via aur-general
2018-11-08 00:45:01 UTC
Permalink
Post by Levente Polyak via aur-general
creeper-world2
I've had two AUR requests queued up for some time now; deleting this
package is one of them.
Santiago Torres-Arias via aur-general
2018-11-08 01:48:20 UTC
Permalink
Post by Levente Polyak via aur-general
creeper-world2
I've had two AUR requests queued up for some time now; deleting this package
is one of them.
Quick update: I addressed your request of deleting the package. That
simplifies everyone's review queue :)

Thanks,
-Santiago
Santiago Torres-Arias via aur-general
2018-11-08 02:28:59 UTC
Permalink
Hello Brett.

I took some time to randomly sample your PKGBUILDs and give some
feedback:

- ags:
- it appears that you use sed to change CFLAGS in the makefile
definition, although it appears that the Makefile itself lets you
overwrite them. I'd advice trying to use native tooling as
possible, and to try to get familiar with the toolchain of each
package as much as possible.
- The optdepends description on wine is a bit confusing in my
opinion.
- I marked the package as out-of-date, as there appears to be a new
version (3.1.4.15) as of almost two months ago.
- I noticed that you didn't add a LICENSE file for this package.

- hib-dlagent:
- I see that you backported a patch on this and ags. I was rather
surprised to see that neither patches were added to new
tags/releases. You could, however, cherry pick the commits rather
than depending on the github api (which can change) to compute the
diff for you. For this, you could use the git transport on
makepkg.
- I noticed that you didn't add a LICENSE file for this package.

- gam-git:
- I'm not sure if this would work when built in a chroot due to
those touch calls.
- After reviewing the package I doubt this doesn't need a build()
step. Otherwise I'd label this package a -bin. This is something
that we should take special consideration of, since we could be
unwittingly be introducing binaries that aren't hardened when
building.
(I could be wrong on this one, since it for some reason vendors
many well-known packages inside of it. Good job for not pulling it
those vendored deps :D)
- I'm confused as to why gam.py needs to be put inside
/usr/share/gam and add a .sh entrypoint for it in /usr/bin. The
file seems to have a shebang and be executable...
- I see that here you *also* are providing a patch. I also could
find that you submitted an issue upstream for said patch (but not
the patch itself)[1]. I like your initiative! Do try to keep the
number of backported patches to a minimum to keep things
manageable.
- I noticed that you didn't add a LICENSE file for this package.

I will probably send more feedback, but I also don't want to overwhelm
you with this and all the other reviews around.

Cheers!
-Santiago.

[1] https://github.com/jay0lee/GAM/issues/791
Eli Schwartz via aur-general
2018-11-08 03:04:00 UTC
Permalink
Post by Santiago Torres-Arias via aur-general
Hello Brett.
I took some time to randomly sample your PKGBUILDs and give some
- it appears that you use sed to change CFLAGS in the makefile
definition, although it appears that the Makefile itself lets you
overwrite them. I'd advice trying to use native tooling as
possible, and to try to get familiar with the toolchain of each
package as much as possible.
- The optdepends description on wine is a bit confusing in my
opinion.
- I marked the package as out-of-date, as there appears to be a new
version (3.1.4.15) as of almost two months ago.
- I noticed that you didn't add a LICENSE file for this package.
This checks out, Artistic2.0 is a common license.
Post by Santiago Torres-Arias via aur-general
- I see that you backported a patch on this and ags. I was rather
surprised to see that neither patches were added to new
tags/releases. You could, however, cherry pick the commits rather
than depending on the github api (which can change) to compute the
diff for you. For this, you could use the git transport on
makepkg.
I don't see why you'd need the overhead of git for this, and that url is
not going to change. They "document" it here:
https://blog.github.com/2011-10-21-github-secrets/#diff-patch

They've provided it for a very long time specifically in order to let
people do this, they're not going to change the scheme and render it
useless for the very purpose it was created for. :p

...

cgit and gitweb let you download patch files too, GitHub just doesn't
expose a visible link for it.
Post by Santiago Torres-Arias via aur-general
- I noticed that you didn't add a LICENSE file for this package.
PKGBUILD claims to be GPL2, which is a common license, but upstream has
no licensing information I can find...
Am I missing something? I only see a "gam" package.
Post by Santiago Torres-Arias via aur-general
- I'm not sure if this would work when built in a chroot due to
those touch calls.
Are you referring to the ones in package()? Not sure why upstream code
needs such weird things but AFAICT it should not break just because of a
chroot.
Post by Santiago Torres-Arias via aur-general
- After reviewing the package I doubt this doesn't need a build()
step. Otherwise I'd label this package a -bin. This is something
that we should take special consideration of, since we could be
unwittingly be introducing binaries that aren't hardened when
building.> (I could be wrong on this one, since it for some reason vendors
many well-known packages inside of it. Good job for not pulling it
those vendored deps :D)
Looks like it just copies a couple python modules into a directory and
then creates a wrapper script to run them. What would you suggest
running in build(), exactly?

Devendoring looks good to me too, though.
Post by Santiago Torres-Arias via aur-general
- I'm confused as to why gam.py needs to be put inside
/usr/share/gam and add a .sh entrypoint for it in /usr/bin. The
file seems to have a shebang and be executable...
I'm not sure what the script does either. gam.py does do:

import utils
from var import *

Which should really be explicit relative imports, but it's actually
legal in python2, and there doesn't seem to be any other reason to want
to cd to the directory, but the script does not cd there anyway.
Post by Santiago Torres-Arias via aur-general
- I see that here you *also* are providing a patch. I also could
find that you submitted an issue upstream for said patch (but not
the patch itself)[1]. I like your initiative! Do try to keep the
number of backported patches to a minimum to keep things
manageable.
I see two mostly similar issues, the other one has a diff referenced.

https://github.com/jay0lee/GAM/issues/created_by/ainola

Worth noting, it's much easier to get upstream projects to accept these
if you provide a button they can click on to implement it -- that means
pull requests.
Post by Santiago Torres-Arias via aur-general
- I noticed that you didn't add a LICENSE file for this package.
Once again, Apache license is a common license and doesn't need to be
installed.
--
Eli Schwartz
Bug Wrangler and Trusted User
Santiago Torres-Arias via aur-general
2018-11-08 03:44:16 UTC
Permalink
Post by Eli Schwartz via aur-general
Post by Santiago Torres-Arias via aur-general
- I noticed that you didn't add a LICENSE file for this package.
This checks out, Artistic2.0 is a common license.
Yes, my bad. For this and the rest of the licenses below I assumed it
was the same case as MIT and such.
Post by Eli Schwartz via aur-general
Post by Santiago Torres-Arias via aur-general
- I see that you backported a patch on this and ags. I was rather
surprised to see that neither patches were added to new
tags/releases. You could, however, cherry pick the commits rather
than depending on the github api (which can change) to compute the
diff for you. For this, you could use the git transport on
makepkg.
I don't see why you'd need the overhead of git for this, and that url is
https://blog.github.com/2011-10-21-github-secrets/#diff-patch
They are not even using an API stable url here. I hope it doesn't
change, but it wouldn't be the first time I get biten by api's like
this[1].
Post by Eli Schwartz via aur-general
They've provided it for a very long time specifically in order to let
people do this, they're not going to change the scheme and render it
useless for the very purpose it was created for. :p
It happened to me with gitlab and their releases url, which started
defaulting to "I don't recognize this branch parameter, so here's the
tarball for master"[1]
Post by Eli Schwartz via aur-general
Post by Santiago Torres-Arias via aur-general
- I'm not sure if this would work when built in a chroot due to
those touch calls.
...
Are you referring to the ones in package()? Not sure why upstream code
needs such weird things but AFAICT it should not break just because of a
chroot.
Hmm, I see they are named as noupdatecheck and nobrowser. I assume these
are to store the program state and thus need be user read-writeable?
This is just speculation, hence the "I'm not sure".
Post by Eli Schwartz via aur-general
Post by Santiago Torres-Arias via aur-general
- After reviewing the package I doubt this doesn't need a build()
step. Otherwise I'd label this package a -bin. This is something
that we should take special consideration of, since we could be
unwittingly be introducing binaries that aren't hardened when
building>
Looks like it just copies a couple python modules into a directory and
then creates a wrapper script to run them. What would you suggest
running in build(), exactly?
I'm not entirely sure, I see that there's a buildscript using
pyinstaller, although I'm not sure why exactly...
Post by Eli Schwartz via aur-general
Post by Santiago Torres-Arias via aur-general
- I'm confused as to why gam.py needs to be put inside
/usr/share/gam and add a .sh entrypoint for it in /usr/bin. The
file seems to have a shebang and be executable...
import utils
from var import *
Which should really be explicit relative imports, but it's actually
legal in python2, and there doesn't seem to be any other reason to want
to cd to the directory, but the script does not cd there anyway.
[1] https://gitlab.com/gitlab-org/gitlab-ce/issues/45507
Eli Schwartz via aur-general
2018-11-08 04:04:43 UTC
Permalink
Post by Santiago Torres-Arias via aur-general
It happened to me with gitlab and their releases url, which started
defaulting to "I don't recognize this branch parameter, so here's the
tarball for master"[1]
Yes, gitlab is prone to bugs like this. :p

gitlab also includes the version of git(1) inside the patch file, and
moved the tarball endpoint for legitimately good reasons in
https://gitlab.com/gitlab-org/gitlab-ce/issues/38830

Because their tarballs used to be insane. And giving you the tarball for
master is a regression they fixed, not an API they dropped. :p
Post by Santiago Torres-Arias via aur-general
Post by Eli Schwartz via aur-general
Post by Santiago Torres-Arias via aur-general
- I'm not sure if this would work when built in a chroot due to
those touch calls.
...
Are you referring to the ones in package()? Not sure why upstream code
needs such weird things but AFAICT it should not break just because of a
chroot.
Hmm, I see they are named as noupdatecheck and nobrowser. I assume these
are to store the program state and thus need be user read-writeable?
This is just speculation, hence the "I'm not sure".
It should be owned by root unless some process uses something like
install -o username -g groupname.
Post by Santiago Torres-Arias via aur-general
Post by Eli Schwartz via aur-general
Looks like it just copies a couple python modules into a directory and
then creates a wrapper script to run them. What would you suggest
running in build(), exactly?
I'm not entirely sure, I see that there's a buildscript using
pyinstaller, although I'm not sure why exactly...
Most likely in order to create some giant windows executable that ships
the entire python application runtime, plus the gam source code, in
order to spare Windows users the need to install Python.
--
Eli Schwartz
Bug Wrangler and Trusted User
Santiago Torres-Arias via aur-general
2018-11-08 04:13:35 UTC
Permalink
Post by Eli Schwartz via aur-general
Because their tarballs used to be insane. And giving you the tarball for
master is a regression they fixed, not an API they dropped. :p
It's an API url they dropped. They moved it around and instead of 404'd
they just left it broken.

True though, probably github is not prone to things like this...
Post by Eli Schwartz via aur-general
It should be owned by root unless some process uses something like
install -o username -g groupname.
Ah, I'm still not sure if the executable should setuid to update than
when run as a user (although it shouldn't be able if it's just a
script).
Post by Eli Schwartz via aur-general
Post by Santiago Torres-Arias via aur-general
Post by Eli Schwartz via aur-general
Looks like it just copies a couple python modules into a directory and
then creates a wrapper script to run them. What would you suggest
running in build(), exactly?
I'm not entirely sure, I see that there's a buildscript using
pyinstaller, although I'm not sure why exactly...
Most likely in order to create some giant windows executable that ships
the entire python application runtime, plus the gam source code, in
order to spare Windows users the need to install Python.
I think you're right, but I'm still confused as to why there's a linux
vaiant of it...

https://github.com/jay0lee/GAM/blob/master/src/linux-build.sh

Probably for the same reason as you pointed out though...

-Santiago
Brett Cornwall via aur-general
2018-11-08 03:14:59 UTC
Permalink
Post by Santiago Torres-Arias via aur-general
Hello Brett.
I took some time to randomly sample your PKGBUILDs and give some
- it appears that you use sed to change CFLAGS in the makefile
definition, although it appears that the Makefile itself lets you
overwrite them. I'd advice trying to use native tooling as
possible, and to try to get familiar with the toolchain of each
package as much as possible.
I will admit that I didn't think to go through those lines when I
inherited it. You're totally right, there's no reason to do it that way.
Post by Santiago Torres-Arias via aur-general
- The optdepends description on wine is a bit confusing in my
opinion.
I removed it. There's little reason to have it there anyway. The
optdepends isn't a good place to inform people about that anyway.
Post by Santiago Torres-Arias via aur-general
- I marked the package as out-of-date, as there appears to be a new
version (3.1.4.15) as of almost two months ago.
Long story short, that was pretty much exactly during the time when I
accidentally clobbered my urlwatch file. Thanks for bringing that up to
me.
Post by Santiago Torres-Arias via aur-general
- I noticed that you didn't add a LICENSE file for this package.
Artistic2.0 is a uncommonly used common license!
(/usr/share/licenses/common/Artistic2.0/license.txt)
Post by Santiago Torres-Arias via aur-general
- I see that you backported a patch on this and ags. I was rather
surprised to see that neither patches were added to new
tags/releases. You could, however, cherry pick the commits rather
than depending on the github api (which can change) to compute the
diff for you. For this, you could use the git transport on
makepkg.
That would bring another dependency on git, though. I can surely do if
if it's more 'correct' but I wouldn't imagine that Github would change
that API anytime soon.

Or would it be better to just carry the patch locally in the repo?
Post by Santiago Torres-Arias via aur-general
- I noticed that you didn't add a LICENSE file for this package.
Yikes, the project doesn't even have a license! I should have checked
that when I inherited it (the packager just slapped a GPL2 on it).
Really, I had just uploaded it so it wouldn't have been lost after the
AUR 4 migration.

I'll bug upstream about it.
Post by Santiago Torres-Arias via aur-general
- I'm not sure if this would work when built in a chroot due to
those touch calls.
- After reviewing the package I doubt this doesn't need a build()
step. Otherwise I'd label this package a -bin. This is something
that we should take special consideration of, since we could be
unwittingly be introducing binaries that aren't hardened when
building.
(I could be wrong on this one, since it for some reason vendors
many well-known packages inside of it. Good job for not pulling it
those vendored deps :D)
- I'm confused as to why gam.py needs to be put inside
/usr/share/gam and add a .sh entrypoint for it in /usr/bin. The
file seems to have a shebang and be executable...
- I see that here you *also* are providing a patch. I also could
find that you submitted an issue upstream for said patch (but not
the patch itself)[1]. I like your initiative! Do try to keep the
number of backported patches to a minimum to keep things
manageable.
- I noticed that you didn't add a LICENSE file for this package.
Of all the packages you had to click on that one. :(

I know it doesn't really excuse it but gam is sort of a "WIP" because
it's... oddly written. I've been meaning to set aside some time to get
some patches in to make it more palatable for packaging. The patch is a
complete hack right now just to make the package "work" (when I
inherited it it was FUBAR).
Post by Santiago Torres-Arias via aur-general
I will probably send more feedback, but I also don't want to overwhelm
you with this and all the other reviews around.
I really appreciate the feedback! It always sucks when so many little
things become so glaring after the fact but
Santiago Torres-Arias via aur-general
2018-11-08 03:34:38 UTC
Permalink
Post by Brett Cornwall via aur-general
Post by Santiago Torres-Arias via aur-general
- I marked the package as out-of-date, as there appears to be a new
version (3.1.4.15) as of almost two months ago.
Long story short, that was pretty much exactly during the time when I
accidentally clobbered my urlwatch file. Thanks for bringing that up to me.
Post by Santiago Torres-Arias via aur-general
- I noticed that you didn't add a LICENSE file for this package.
Artistic2.0 is a uncommonly used common license!
(/usr/share/licenses/common/Artistic2.0/license.txt)
Yes, my bad. I was told about this on MIT, and I assumed this was the
case for most licenses...
Post by Brett Cornwall via aur-general
Post by Santiago Torres-Arias via aur-general
- I see that you backported a patch on this and ags. I was rather
surprised to see that neither patches were added to new
tags/releases. You could, however, cherry pick the commits rather
than depending on the github api (which can change) to compute the
diff for you. For this, you could use the git transport on
makepkg.
That would bring another dependency on git, though. I can surely do if if
it's more 'correct' but I wouldn't imagine that Github would change that API
anytime soon.
Or would it be better to just carry the patch locally in the repo?
True, I didn't consider the dependency on git. I'd say you could check
it in. I do not agree with Eli that you should rely on api's like this
to get a simple patch. It has been my experience that api's like this
move around and leave you trying to debug weird errors.
Post by Brett Cornwall via aur-general
Post by Santiago Torres-Arias via aur-general
- I noticed that you didn't add a LICENSE file for this package.
Yikes, the project doesn't even have a license! I should have checked that
when I inherited it (the packager just slapped a GPL2 on it). Really, I had
just uploaded it so it wouldn't have been lost after the AUR 4 migration.
I'll bug upstream about it.
Post by Santiago Torres-Arias via aur-general
...
Of all the packages you had to click on that one. :(
I know it doesn't really excuse it but gam is sort of a "WIP" because
it's... oddly written. I've been meaning to set aside some time to get some
patches in to make it more palatable for packaging. The patch is a complete
hack right now just to make the package "work" (when I inherited it it was
FUBAR).
Yes, granted I'm rather confused when I read the repository and see
things like build-linux.sh that pulls pyinstaller. I didn't know exactly
what of all was happening there...
Post by Brett Cornwall via aur-general
Post by Santiago Torres-Arias via aur-general
I will probably send more feedback, but I also don't want to overwhelm
you with this and all the other reviews around.
I really appreciate the feedback! It always sucks when so many little things
become so glaring after the fact but
Lol I've been there, no worries :)

-Santiago.
Bruno Pagani via aur-general
2018-11-08 09:38:36 UTC
Permalink
Post by Santiago Torres-Arias via aur-general
Post by Brett Cornwall via aur-general
Post by Santiago Torres-Arias via aur-general
- I noticed that you didn't add a LICENSE file for this package.
Artistic2.0 is a uncommonly used common license!
(/usr/share/licenses/common/Artistic2.0/license.txt)
Yes, my bad. I was told about this on MIT, and I assumed this was the
case for most licenses...
We have a instructions here:
https://wiki.archlinux.org/index.php/PKGBUILD#license (which redirects
to the actual licenses package for a list of what is common). ;)
Post by Santiago Torres-Arias via aur-general
Post by Brett Cornwall via aur-general
Post by Santiago Torres-Arias via aur-general
- I see that you backported a patch on this and ags. I was rather
surprised to see that neither patches were added to new
tags/releases. You could, however, cherry pick the commits rather
than depending on the github api (which can change) to compute the
diff for you. For this, you could use the git transport on
makepkg.
That would bring another dependency on git, though. I can surely do if if
it's more 'correct' but I wouldn't imagine that Github would change that API
anytime soon.
Or would it be better to just carry the patch locally in the repo?
True, I didn't consider the dependency on git. I'd say you could check
it in. I do not agree with Eli that you should rely on api's like this
to get a simple patch. It has been my experience that api's like this
move around and leave you trying to debug weird errors.
Please don’t start cloning a repo just for some small patches that can
be retrieved by this stable and long-lived GitHub API. And @Brett, no,
you should not carry the patch locally. No reason to clobber our tree
with that. ;)

Regards,
Bruno
David Runge
2018-11-09 18:05:58 UTC
Permalink
As the (newish, but still not updated [1]) discussion period of 14 days
is now over, I have started the vote.

Thanks again to all reviewers and active TUs in this discussion period!
I think Brett will make a good TU!

Let's vote! :)

[1] https://aur.archlinux.org/trusted-user/TUbylaws.html#_standard_voting_procedure
--
https://sleepmap.de
Eli Schwartz via aur-general
2018-11-09 18:11:19 UTC
Permalink
Post by David Runge
As the (newish, but still not updated [1]) discussion period of 14 days
is now over, I have started the vote.
Thanks again to all reviewers and active TUs in this discussion period!
I think Brett will make a good TU!
Let's vote! :)
[1] https://aur.archlinux.org/trusted-user/TUbylaws.html#_standard_voting_procedure
I got worried for a second there, but the section you're looking at is
the default "procedure for general proposals" when another section
doesn't override the five-day discussion period.

This definitely lists the right time period:
https://aur.archlinux.org/trusted-user/TUbylaws.html#_addition_of_a_tu
--
Eli Schwartz
Bug Wrangler and Trusted User
David Runge
2018-11-09 18:24:39 UTC
Permalink
Post by Eli Schwartz via aur-general
https://aur.archlinux.org/trusted-user/TUbylaws.html#_addition_of_a_tu
Yep, sorry, that was just a brainfart from my side. ;-)

Here's the link to the vote btw:
https://aur.archlinux.org/tu/?id=112
--
https://sleepmap.de
David Runge
2018-11-16 19:11:18 UTC
Permalink
Post by David Runge
Let's vote! :)
The results are in (the vote ended nearly an hour ago)!

Yes: 26
No: 9
Abstain: 12
Participation 90.38% (come on, don't drop the ball on this! ;-))

I have just updated your account status in the AUR to 'Trusted User'.
Congratulations!

Please follow up on the TODO for new Trusted Users [1].

See you on IRC :)

Bests,
David

[1] https://wiki.archlinux.org/index.php/AUR_Trusted_User_Guidelines#TODO_list_for_new_Trusted_Users
--
https://sleepmap.de
Brett Cornwall via aur-general
2018-11-16 19:53:25 UTC
Permalink
Post by David Runge
The results are in (the vote ended nearly an hour ago)!
Thank you all.

To those that voted 'No': Feel free to send me a message with your
feedback. Your concerns would be a valuable asset for me to keep in
mind.

Continue reading on narkive:
Loading...