[aur-general] TU Application: David Runge
Hi all, my name is David Runge. You may have come across me somewhere on the wiki [1], the forums [2] (not so recently), the AUR [3] or the mailing lists (mainly arch-general). I'm sponsored by Ray Rashif. I'm 31, currently live in Berlin and have just finished a Master's degree in audio communication and technology. I've been working in (sadly Debian) Linux based computer pools and electronic music studios (at the Technical University of Berlin) over the past six years and am currently a freelancing audio engineer and programmer. I've been with Arch for roughly ten years now, after a short period of time spent with Debian and Ubuntu and disliking the clutter and the amount of preset environments. I have a huge interest in pro-audio on Linux (I organized the Linux Audio Conference 2016 and have been using Arch for small scale stereo projects to massive multi-channel applications) and web applications and their security (probably a good idea when running one's own infrastructure). In my free time I build modular synthesizers and make music with them, code weird music things using SuperCollider or try to optimize my packages and systems. As a TU I'd like to help Ray test and maintain the motherload of pro-audio packages he's taking care of [4] and take over some (or all) of the ones that are currently leftover from speps [5]. Additionally, I would like to spend some time to bring further useful dsp programming languages, such as faust [6], or spatial audio renderers, such as the ssr [7] to [community]. Best, David [1] https://wiki.archlinux.org/index.php/Special:Contributions/Davezerave [2] https://bbs.archlinux.org/profile.php?id=20912 [3] https://aur.archlinux.org/packages/?K=dvzrv&SeB=m [4] https://www.archlinux.org/packages/?sort=&q=&maintainer=schiv&flagged= [5] https://www.archlinux.org/packages/?sort=&q=&maintainer=speps&flagged= [6] https://aur.archlinux.org/packages/faust/ [7] https://aur.archlinux.org/packages/ssr/ -- https://sleepmap.de
On 18 October 2017 at 05:07, David Runge <dave@sleepmap.de> wrote:
Hi all,
my name is David Runge. You may have come across me somewhere on the wiki [1], the forums [2] (not so recently), the AUR [3] or the mailing lists (mainly arch-general). I'm sponsored by Ray Rashif.
I confirm my sponsorship. I have to be honest that I have not checked David's PKGBUILDs thoroughly, but he has been in contact with me with contributions. He has the skill set necessary to both maintain packages and debug software, as well as test them correctly. If his application is successful, he will be co-maintainer for all my and Spep's packages in [community]. The discussion begins now and will last until 23 October, after which the voting period will begin on 24 October. Good luck David! -- GPG/PGP ID: C0711BF1
On 10/17/2017 07:07 PM, David Runge wrote:
my name is David Runge. You may have come across me somewhere on the wiki [1], the forums [2] (not so recently), the AUR [3] or the mailing lists (mainly arch-general). I'm sponsored by Ray Rashif.
General commentary on your AUR packages, since xxarhtna is being a slacker who slacks: crypted-backups-git: - empty variables and arrays are messy - unquoted $pkgdir, though $srcdir is consistently quoted - the pkgver() fallback for non-tagged repos can be simplified - build() does nothing, why does it exist? - source urls for git repositories should be cloned over https:// rather than git:// in order to take advantage of cacert verification - source urls do not need to be prepended with "${_pkg}::" when they don't actually need to be renamed - there is no need to `install -d ... && install -Dm644 ...`, just do the second part easytranscript: - unquoted $pkgdir, though $srcdir is consistently quoted - inconsistent quoting of array elements; why does license have double quotes? - there is no need to `install -d ... && install -Dm644 ...`, just do the second part firefox-extension-tab-tree: - pkgdesc is self-referential, see the wiki for guidance on a good pkgdesc: https://wiki.archlinux.org/index.php/PKGBUILD#pkgdesc - depends does not need to have a versioned Firefox requirement - why is "${source##*/}" obfuscated when it will always be the same thing? grub-customizer: - empty variables and arrays are messy - unquoted $pkgdir, though $srcdir is consistently quoted - there is no need to `install -d ... && install -Dm644 ...`, just do the second part - package() also runs the compilation and `sed`s the desktop file, there is a good reason we now do split build/package functions... - the use of `|| return 1` is deprecated, makepkg sets errexit - inconsistent quoting of array elements; why is provides=() alone in not quoting this? why does license have double-quotes? - why is there a backslash escape for a multiline source=() array? - there is no need for pkgbranch, use ${pkgver%.*} instead, but if you do use it you should name it _pkgbranch to mark it as a custom var - the install file is outdated, we have pacman hooks for that now - consider specifying a reason for the optdepends jackcpp-git: - unquoted $pkgdir and $srcdir - source urls for git repositories should be cloned over https:// rather than git:// in order to take advantage of cacert verification - source urls do not need to be prepended with "$_basename::" when they don't actually need to be renamed - the url should use HTTPS - this is marked as an "any" package when it contains compiled ELF binaries which is a huge no-no - by the way, the nonstandard ordering of makepkg variables had me looking for a second to make sure you actually listed everything khard-git: - inconsistent quoting of array elements; why does license have double quotes? - source urls do not need to be prepended with "$_basename::" when they don't actually need to be renamed - by the way, the nonstandard ordering of makepkg variables had me looking for a second to make sure you actually listed everything - unquoted $pkgdir - build() does nothing, it should run python setup.py build - setuptools installations usually also use --skip-build --optimize=1 in the package() function - why do you explicitly use !emptydirs instead of the makepkg.conf default, does the package depend on it? liblo-git: - unquoted $pkgdir and $srcdir - source urls for git repositories should be cloned over https:// rather than git:// in order to take advantage of cacert verification; in sourceforge's case that means using the following url: https://git.code.sf.net/p/liblo/git - source urls do not need to be prepended with "$_basename::" when they don't actually need to be renamed, but see the above point - provides a hardcoded version of liblo, $pkgver should be fine but if not, consider using ${pkgver%%.r*} - autogen should really go in prepare(), also autogen should die and be replaced by autoreconf in nearly every case. If they really have weird special sauce in there, at least tell autogen to not run ./configure mantisbt: - this incredibly gross install script messes around with unpackaged files, please don't do this nextcloud-news-updater: owncloud-news-updater: - what is a "parllel" feed updater anyway? - source tarball violates shared SRCDEST, $pkgver.tar.gz clashes with other packages that have the same version. - unquoted $pkgdir and $srcdir - `python setup.p install --skip-build` in package() should be preceded by `python setup.py build` in build() the same way `make` is separated from `make install` in Makefile powered packages. - I'm not sure why your lengthy post_install script which does nothing other than echo documentation instructions, needs to also tell the user how to systemctl, this is probably massive overkill. If there's something unusual about this package more than any other systemd services, then that's probably okay to warn the user about. patchbook-git - cloning a git source to a $pkgver specific directory more or less destroys the purpose of using git in the first place. Moreso for a foo-git package since the pkgver will update on every revision, invalidating the cached source - there is no need to `install -d ... && install -Dm644 ...`, just do the second part - when cat'ing and echo'ing text around to create the primary executable, this belongs in prepare() not build() -- also noticed you *did* file an issue upstream to add this pd-git: - unquoted $pkgdir and $srcdir - source urls for git repositories should be cloned over https:// rather than git:// in order to take advantage of cacert verification; in - autogen should really go in prepare(), also autogen should die and be replaced by autoreconf in nearly every case. But then their compat script does nothing but run `autoreconf -fi` anyway, so you should *definitely* be all modern and use it properly. - why do you override the user's choice of buildflags and strip, does the package break if you try stripping it or using non-upstream CFLAGS? If you just want to provide debugging symbols by default, don't as you should not be taking that choice away from the user prosody-mod-admin-message-hg: prosody-mod-admin-web-hg: - must install a copy of the MIT license. - get_deps.sh seems kind of ugly, I want to say you should do that yourself in source=() and it doesn't seem like it has changed since 2014 anyway... python-iwlib: - source tarball violates shared SRCDEST, $pkgver.tar.gz clashes with other packages that have the same version. - `python setup.p install --skip-build` in package() should be preceded by `python setup.py build` in build() the same way `make` is separated from `make install` in Makefile powered packages. - `python setup.py test` is obviously a check(), not a build() - this is marked as an "any" package when it contains compiled C extensions which is a huge no-no python-osc: - source tarball violates shared SRCDEST, $pkgver.tar.gz clashes with other packages that have the same version. - `python setup.p install --skip-build` in package() should be preceded by `python setup.py build` in build() the same way `make` is separated from `make install` in Makefile powered packages. - `python setup.py test` is obviously a check(), not a build() roundcube-rcmcarddav: - unquoted $pkgdir rts-git: - pkgdesc is really really ugly - empty variables and arrays are messy - unquoted $pkgdir, though $srcdir is consistently quoted - source urls do not need to be prepended with "${_basename}::" when they don't actually need to be renamed ssr: - provides, conflicts, and replaces itself. - pkgdesc is self-referential, which it should not be - versioned depends are bloat - generally boost-libs is a depends and boost is a makedepends but here only boost-libs is present and as a makedepends, not sure that that is correct - why are you tee'ing everything to logfiles? that's why makepkg has a dedicated log option ssr-git: - source urls do not need to be prepended with "${_pkgname}::" when they don't actually need to be renamed - pkgdesc is self-referential, which it should not be - dependencies differ from non -git package, one or the other is probably wrong and should be fixed - autogen should really go in prepare(), also autogen should die and be replaced by autoreconf in nearly every case. But then their compat script does nothing but run `autoreconf -fi` anyway, so you should *definitely* be all modern and use it properly. - patching should also go in prepare(), and come before autogen on principle - why are you exporting LIBS, which is not set by makepkg and indicates something wrong with the build system (which should probably be patched properly) - is the patch for gcc7 necessary for the non-git version? supercollider-git: - source urls do not need to be prepended with "${_name}::" when they don't actually need to be renamed - why are there commented-out functions with legacy patches still in-tree? why is the pkgvere() commented out and replaced by one that `git describe`s HEAD? - Why does build() update submodules that are not cloned in source=()? See https://wiki.archlinux.org/index.php/VCS_package_guidelines#Git_Submodules - why are you tee'ing everything to logfiles? that's why makepkg has a dedicated log option - why some big complicated switch for configuring based on the CARCH, why don't you just use the $CARCH variable you already have available, and if you actually have to do different things depending on the CARCH, then you can make this a lot easier to read by including extra options in a bash array for safe tokenization. - why are you overriding the user's MAKEFLAGS from makepkg.conf by specifying the number of concurrent jobs to run? Is the upstream build system bugged and you must use that exact number of jobs to avoid erros due to race conditions and a badly-defined targets graph? uenv: - empty variables and arrays are messy - provides itself - unquoted $pkgdir - there is no need to `install -d ... && install -Dm644 ...`, just do the second part - this appears to be your personal configurations, does that really belong in the AUR? uenv-git: - mostly the same as uenv - source urls do not need to be prepended with "${_basename}::" when they don't actually need to be renamed veejay-client-git: veejay-server-git: veejay-utils-git: - pkgdesc is self-referential - in no sane world does a -git package "replaces" the non-git version just because it is a drop-in replacement - autogen should really go in prepare(), also autogen should die and be replaced by autoreconf in nearly every case. But then their compat script does nothing but run `autoreconf -fi` anyway, so you should *definitely* be all modern and use it properly. - why are you tee'ing everything to logfiles? that's why makepkg has a dedicated log option - consider making these a split package, since they all have the same version and build from the same sources vrpn-git: - source urls do not need to be prepended with "${_pkg}::" when they don't actually need to be renamed - Why does prepare() update submodules that are not cloned in source=()? See https://wiki.archlinux.org/index.php/VCS_package_guidelines#Git_Submodules - there is no need to `msg` the user using makepkg's private API every time you run a sed command - CMake builds may need to be out-of-tree, but there is no rule that says you have to rm -rf an incremental build, unless this package is special somehow? -- Eli Schwartz
On October 18, 2017 2:40:55 PM GMT+02:00, Rashif Ray Rahman <schiv@archlinux.org> wrote:
I confirm my sponsorship. I have to be honest that I have not checked David's PKGBUILDs thoroughly, but he has been in contact with me with contributions.
At this point my feedback goes to the sponsor instead of the applicant: Sorry, but I must say that I really dislike sponsoring a TU applicant without looking at any PKGBUILD and give some advice. In my world this is part of the sponsorship and one of the jobs of a "TU mentor". I understand that you could certainly make some judgment based on the contributions you mentioned, but still. I hope this will not become a trend, otherwise we could also just get rid of the whole sponsorship (which we shouldn't). Cheers, Levente
On Tue, 24 Oct 2017 07:50:01 +0200, Levente Polyak wrote:
On October 18, 2017 2:40:55 PM GMT+02:00, Rashif Ray Rahman wrote:
I confirm my sponsorship. I have to be honest that I have not checked David's PKGBUILDs thoroughly, but he has been in contact with me with contributions.
At this point my feedback goes to the sponsor instead of the applicant:
Sorry, but I must say that I really dislike sponsoring a TU applicant without looking at any PKGBUILD and give some advice. In my world this is part of the sponsorship and one of the jobs of a "TU mentor". I understand that you could certainly make some judgment based on the contributions you mentioned, but still.
I hope this will not become a trend, otherwise we could also just get rid of the whole sponsorship (which we shouldn't).
Please take into account that pro-audio package maintainers should be those, who also use the packages for audio work on at least a nearly professional quality level, IOW they need to have skills that are at least close to a professional audio engineering niveau. Obviously not many users have very good audio engineering skills + Linux skills + Arch packaging skills + time to maintain packages, so you should lower your sights for such a nice domain.
Hi Eli, thanks for the many remarks. On 2017-10-22 23:44:39 (-0400), Eli Schwartz wrote:
General commentary on your AUR packages, since xxarhtna is being a slacker who slacks: Due to two work arrangements this week, I won't have time to fix them before this evening.
I'll get back to it later! Best, David -- https://sleepmap.de
On October 24, 2017 8:13:33 AM GMT+02:00, Ralf Mardorf <ralf.mardorf@alice-dsl.net> wrote:
On Tue, 24 Oct 2017 07:50:01 +0200, Levente Polyak wrote:
On October 18, 2017 2:40:55 PM GMT+02:00, Rashif Ray Rahman wrote:
I confirm my sponsorship. I have to be honest that I have not checked David's PKGBUILDs thoroughly, but he has been in contact with me with contributions.
At this point my feedback goes to the sponsor instead of the applicant:
Sorry, but I must say that I really dislike sponsoring a TU applicant without looking at any PKGBUILD and give some advice. In my world this is part of the sponsorship and one of the jobs of a "TU mentor". I understand that you could certainly make some judgment based on the contributions you mentioned, but still.
I hope this will not become a trend, otherwise we could also just get rid of the whole sponsorship (which we shouldn't).
Please take into account that pro-audio package maintainers should be those, who also use the packages for audio work on at least a nearly professional quality level, IOW they need to have skills that are at least close to a professional audio engineering niveau.
Obviously not many users have very good audio engineering skills + Linux skills + Arch packaging skills + time to maintain packages, so you should lower your sights for such a nice domain.
You take the wrong conclusions. Im not saying he may not be suitable to do the job but I'm saying there is a difference between a random AUR packager and one that packages for our repositories. The path in getting the tiny differences in experience and best packaging practices should be led by the sponsor by discussing packages he/she maintains. This way the sponsor can aid and lead an applicant to get some possibly missing. pieces straight. It's cool we have someone for an area that not many cover, but that doesn't free him/her from proper packaging and getting to know the bits that make up a good packager. Especially here would a good guidance be sane and the very minimum a sponsor can do to bring in his/her applicant. In short: Being someone rare doesn't free up following best practices, that's the difference between a random dude and a good TU and the sponsor must help him achieving this.
On Tue, 24 Oct 2017 10:01:56 +0200, Levente Polyak wrote:
You take the wrong conclusions. Im not saying he may not be suitable to do the job but I'm saying there is a difference between a random AUR packager and one that packages for our repositories. The path in getting the tiny differences in experience and best packaging practices should be led by the sponsor by discussing packages he/she maintains. This way the sponsor can aid and lead an applicant to get some possibly missing. pieces straight. It's cool we have someone for an area that not many cover, but that doesn't free him/her from proper packaging and getting to know the bits that make up a good packager. Especially here would a good guidance be sane and the very minimum a sponsor can do to bring in his/her applicant. In short: Being someone rare doesn't free up following best practices, that's the difference between a random dude and a good TU and the sponsor must help him achieving this.
My apologies, since I agree with your point of view, OTOH Ray is the one who has got all the needed skills. Actually he just needs somebody to assist him maintaining packages. I'm often using packages from jhernberg and schivmeister/Ray and edit them to build more recent packages, so the packages much likely wont become too disgusting. David has not to do much more, especially if we should have an Arch audio mailing list again and all Arch audio user, including myself, are willing to help, if David _should_ run into an issue. I understand your concerns against Ray's "freakish" sponsorship, anyway, consider to vote for David, because Arch audio has got no other opportunity and Ray's sponsorship is based upon a good impression of David's help, he already provided.
On 24 October 2017 at 11:50, Levente Polyak <anthraxx@archlinux.org> wrote:
At this point my feedback goes to the sponsor instead of the applicant:
Sorry, but I must say that I really dislike sponsoring a TU applicant without looking at any PKGBUILD and give some advice. In my world this is part of the sponsorship and one of the jobs of a "TU mentor". I understand that you could certainly make some judgment based on the contributions you mentioned, but still.
I hope this will not become a trend, otherwise we could also just get rid of the whole sponsorship (which we shouldn't).
Err, I never wrote that I did not check his packages. I only wrote I did not check his packages "thoroughly". Perhaps I should have been so honest, but thanks for raising your concern. What are you referring to as a "trend"? A TU sponsor usually just confirms a sponsorship. The assumption is that before sending in the proposal, the sponsor has already done the pre-requisite review and screening. I am not going to be pedantic about many things which are fixable (I couldn't care less about them myself). I am satisfied with what I have seen and leave the rest up to the community. The by-laws prescribe a discussion period for exactly this aspect of peer review. If perhaps you (or anyone else) thought I simply picked him at random, then that is a misunderstanding. There were several other candidates, and I had to spend the time to calculate several metrics to decide on one. That includes checking _some_ packages -- it's called sampling. -- GPG/PGP ID: C0711BF1
On 18 October 2017 at 18:40, Rashif Ray Rahman <schiv@archlinux.org> wrote:
The discussion begins now and will last until 23 October, after which the voting period will begin on 24 October. Good luck David!
The formal discussion period is over, please cast your votes! The voting period will last until 31 October. I do have one thing to say to David: please do think about the time commitment associated with this. If you think that you won't be able to commit much starting out, then it will be quite sad moving forward. Otherwise, I say again that David's proposal has followed standard procedure, including a holistic review by myself the sponsor. Some issues were raised based on automated tool reports, which I think David will be able to resolve. Good luck! -- GPG/PGP ID: C0711BF1
Em outubro 24, 2017 14:43 Rashif Ray Rahman escreveu:
On 18 October 2017 at 18:40, Rashif Ray Rahman <schiv@archlinux.org> wrote:
The discussion begins now and will last until 23 October, after which the voting period will begin on 24 October. Good luck David!
The formal discussion period is over, please cast your votes! The voting period will last until 31 October.
I do have one thing to say to David: please do think about the time commitment associated with this. If you think that you won't be able to commit much starting out, then it will be quite sad moving forward.
Otherwise, I say again that David's proposal has followed standard procedure, including a holistic review by myself the sponsor. Some issues were raised based on automated tool reports, which I think David will be able to resolve.
Good luck!
The link to the vote for the lazy: https://aur.archlinux.org/tu/?id=97 Regards, Giancarlo Razzolini
On 10/24/2017 12:43 PM, Rashif Ray Rahman wrote:
Some issues were raised based on automated tool reports, which I think David will be able to resolve.
Excuse me? I am hardly an "automated tool report". :p You seem to have derived that conclusion out of thin air. The reason I never got around to remarking on his packages until very nearly the last day of the discussion period was not just because I was expecting anthraxx to do so first -- I also spent no little time reading every single PKGBUILD of his in the AUR, and cloning the sources for perusal for several of them too. I've seen a lot of PKGBUILDs, and many/most of the simple mistakes or general style failures that people tend to make; I have a list of things to raise red flags over. But that still doesn't mean I can review a PKGBUILD without even looking at it! Maybe one day... -- Eli Schwartz
Hey again, multiple occurences are not quoted again. On 2017-10-22 23:44:39 (-0400), Eli Schwartz wrote:
crypted-backups-git: - empty variables and arrays are messy Fixed.
- unquoted $pkgdir, though $srcdir is consistently quoted Fixed.
- the pkgver() fallback for non-tagged repos can be simplified Fixed.
- build() does nothing, why does it exist? Reasons unknown. Deleted.
- source urls for git repositories should be cloned over https:// rather than git:// in order to take advantage of cacert verification Thanks! Completely forgot to enable that on my server and used git:// ever since...
- source urls do not need to be prepended with "${_pkg}::" when they don't actually need to be renamed Fixed.
- there is no need to `install -d ... && install -Dm644 ...`, just do the second part Ah... the source's expansion prevented a successful -Dm***, so I used -d to create the destination first. I should've used the -t switch. Fixed.
easytranscript: - inconsistent quoting of array elements; why does license have double quotes? Reasons unknown. Fixed.
firefox-extension-tab-tree: - pkgdesc is self-referential, see the wiki for guidance on a good pkgdesc: https://wiki.archlinux.org/index.php/PKGBUILD#pkgdesc Fixed.
- depends does not need to have a versioned Firefox requirement Fixed.
- why is "${source##*/}" obfuscated when it will always be the same thing? I'm not sure I get, what you mean by that. "${source##*/}" retrieves the .xpi file name from the link stored in ${source}. I could of course also write "${_plugin_name}-${pkgver}-fx-linux.xpi", but does that make it better?
grub-customizer: - there is no need to `install -d ... && install -Dm644 ...`, just do the second part - package() also runs the compilation and `sed`s the desktop file, there is a good reason we now do split build/package functions... Fixed.
- the use of `|| return 1` is deprecated, makepkg sets errexit Fixed.
- inconsistent quoting of array elements; why is provides=() alone in not quoting this? why does license have double-quotes? Fixed.
- why is there a backslash escape for a multiline source=() array? Fixed.
- there is no need for pkgbranch, use ${pkgver%.*} instead, but if you do use it you should name it _pkgbranch to mark it as a custom var Fixed.
- the install file is outdated, we have pacman hooks for that now Fixed.
- consider specifying a reason for the optdepends Will have to ask upstream about that one.
jackcpp-git: - unquoted $pkgdir and $srcdir Fixed.
- source urls for git repositories should be cloned over https:// rather than git:// in order to take advantage of cacert verification Fixed.
- source urls do not need to be prepended with "$_basename::" when they don't actually need to be renamed Fixed.
- the url should use HTTPS Fixed.
- this is marked as an "any" package when it contains compiled ELF binaries which is a huge no-no Fixed.
- by the way, the nonstandard ordering of makepkg variables had me looking for a second to make sure you actually listed everything Security through obscurity ;-) Fixed.
khard-git: - build() does nothing, it should run python setup.py build Fixed.
- setuptools installations usually also use --skip-build --optimize=1 in the package() function Fixed.
- why do you explicitly use !emptydirs instead of the makepkg.conf default, does the package depend on it? Legacy problem. Removed.
liblo-git: - source urls for git repositories should be cloned over https:// rather than git:// in order to take advantage of cacert verification; in sourceforge's case that means using the following url: https://git.code.sf.net/p/liblo/git - source urls do not need to be prepended with "$_basename::" when they don't actually need to be renamed, but see the above point Now it does.
- provides a hardcoded version of liblo, $pkgver should be fine but if not, consider using ${pkgver%%.r*} Okay.
- autogen should really go in prepare(), also autogen should die and be replaced by autoreconf in nearly every case. If they really have weird special sauce in there, at least tell autogen to not run ./configure Switched to autoreconf, seems to work fine with minor fix. Thanks for that hint!
mantisbt: - this incredibly gross install script messes around with unpackaged files, please don't do this Okay. What would be a good alternative though? Using the application during upgrade might potentially break the database. However, the mantis_offline.php file prevents users from interacting with the application. I know that hooks such as the one introduced for nextcloud quite recently (for a while) won't work either.
nextcloud-news-updater: owncloud-news-updater: - what is a "parllel" feed updater anyway? A typo. Nice one.
- source tarball violates shared SRCDEST, $pkgver.tar.gz clashes with other packages that have the same version. Fixed.
- `python setup.p install --skip-build` in package() should be preceded by `python setup.py build` in build() the same way `make` is separated from `make install` in Makefile powered packages. Fixed.
- I'm not sure why your lengthy post_install script which does nothing other than echo documentation instructions, needs to also tell the user how to systemctl, this is probably massive overkill. If there's something unusual about this package more than any other systemd services, then that's probably okay to warn the user about. True. Now only the 'Note:' is left.
patchbook-git - cloning a git source to a $pkgver specific directory more or less destroys the purpose of using git in the first place. Moreso for a foo-git package since the pkgver will update on every revision, invalidating the cached source Fixed.
- when cat'ing and echo'ing text around to create the primary executable, this belongs in prepare() not build() -- also noticed you *did* file an issue upstream to add this Fixed. Noted, but it hasn't changed (yet).
pd-git: - autogen should really go in prepare(), also autogen should die and be replaced by autoreconf in nearly every case. But then their compat script does nothing but run `autoreconf -fi` anyway, so you should *definitely* be all modern and use it properly. Fixed.
- why do you override the user's choice of buildflags and strip, does the package break if you try stripping it or using non-upstream CFLAGS? If you just want to provide debugging symbols by default, don't as you should not be taking that choice away from the user It was mainly because of the following bug: https://bugs.archlinux.org/task/45006 This seems to not apply to pd>0.48 anymore. Yay! Removed.
Also: Debugging seems to be activated in the main repository as well. oO
prosody-mod-admin-message-hg: prosody-mod-admin-web-hg: - must install a copy of the MIT license. Fixed.
- get_deps.sh seems kind of ugly, I want to say you should do that yourself in source=() and it doesn't seem like it has changed since 2014 anyway... It's unclear, if the package still works as expected with the current version of prosody. I might drop or have it deleted soon.
python-iwlib: - this is marked as an "any" package when it contains compiled C extensions which is a huge no-no Fixed.
python-osc: Fixed.
roundcube-rcmcarddav: Fixed
rts-git: - pkgdesc is really really ugly I made it more beautiful. ;-)
ssr: - provides, conflicts, and replaces itself. Fixed.
- versioned depends are bloat Fixed.
- generally boost-libs is a depends and boost is a makedepends but here only boost-libs is present and as a makedepends, not sure that that is correct Fixed.
- why are you tee'ing everything to logfiles? that's why makepkg has a dedicated log option Bad habit. Fixed.
ssr-git: - pkgdesc is self-referential, which it should not be - dependencies differ from non -git package, one or the other is probably wrong and should be fixed No. boost-libs has been dropped in favor of asio.
- patching should also go in prepare(), and come before autogen on principle Sadly I have to patch configure. Upstream is veeeeeeeeeeeery slow on merging pull requests, so the switch to qt5, that would make this all go away, is still not upon us :( Moved.
- why are you exporting LIBS, which is not set by makepkg and indicates something wrong with the build system (which should probably be patched properly) There is something wrong with the build system. If not exporting LIBS, make will complain about missing -lpthread (undefined reference to symbol). Semi-recent changes in Arch's build system has caused this to fail. Still about to investigate further, when I have time to do so.
- is the patch for gcc7 necessary for the non-git version? No. The patch is necessary, because the IP interface of the application was ported from boost to asio. A pull request was made and approved, but it's still not merged.
supercollider-git: - why are there commented-out functions with legacy patches still Fixed.
in-tree? why is the pkgvere() commented out and replaced by one that `git describe`s HEAD? There is no backporting of patches per version. The SuperCollider devs additionally tend to use lightweight and normal tags chaoticly. https://github.com/supercollider/supercollider/issues/2857
- Why does build() update submodules that are not cloned in source=()? See https://wiki.archlinux.org/index.php/VCS_package_guidelines#Git_Submodules Fixed.
- why some big complicated switch for configuring based on the CARCH, why don't you just use the $CARCH variable you already have available, and if you actually have to do different things depending on the CARCH, then you can make this a lot easier to read by including extra options in a bash array for safe tokenization. Sounds interesting. Will rework this, as soon as I have my Pis up and running again. Note however, that some hacks are necessary for supercollider to function on a headless system (which embedded systems often are).
- why are you overriding the user's MAKEFLAGS from makepkg.conf by specifying the number of concurrent jobs to run? Is the upstream build system bugged and you must use that exact number of jobs to avoid erros due to race conditions and a badly-defined targets graph? Fixed.
uenv: - provides itself Fixed. - this appears to be your personal configurations, does that really belong in the AUR? On the contrary, according to its pkgdesc ("Useful scripts, systemd timer/service units and their configuration"), this is not personal configurations, but pretty generic unit files, etc.
uenv-git: Fixed.
veejay-client-git: veejay-server-git: veejay-utils-git: - in no sane world does a -git package "replaces" the non-git version just because it is a drop-in replacement Fixed.
- autogen should really go in prepare(), also autogen should die and be replaced by autoreconf in nearly every case. But then their compat script does nothing but run `autoreconf -fi` anyway, so you should *definitely* be all modern and use it properly. - consider making these a split package, since they all have the same version and build from the same sources Yes, that's my idea eventually. I'm working on a split package, that also includes the plugins of that project.
vrpn-git: I adopted this quite recently to see if I can make it work properly. It is not very beautiful...
- there is no need to `msg` the user using makepkg's private API every time you run a sed command Fixed.
- CMake builds may need to be out-of-tree, but there is no rule that says you have to rm -rf an incremental build, unless this package is special somehow? Fixed.
Thanks again for the valuable input. Best, David -- https://sleepmap.de
On 2017-10-24 22:43:50 (+0600), Rashif Ray Rahman wrote:
I do have one thing to say to David: please do think about the time commitment associated with this. If you think that you won't be able to commit much starting out, then it will be quite sad moving forward. I have thought about this a lot of course, before applying. Like anyone else I have a life with a job (or several from time to time). This week is quite extreme though (even by my standards) work wise and I should have communicated that before applying. Some projects or new things (in work life) turn out to eat more time and energy than intially expected though.
Otherwise, I say again that David's proposal has followed standard procedure, including a holistic review by myself the sponsor. Some issues were raised based on automated tool reports, which I think David will be able to resolve. I hope my mail from earlier could clear things up.
Good luck! Thanks!
Best, David -- https://sleepmap.de
On 10/24/2017 10:18 PM, David Runge wrote:
- why is "${source##*/}" obfuscated when it will always be the same thing? I'm not sure I get, what you mean by that. "${source##*/}" retrieves the .xpi file name from the link stored in ${source}. I could of course also write "${_plugin_name}-${pkgver}-fx-linux.xpi", but does that make it better?
Well, yeah. It's more clear what you're doing. FWIW, I typically use a source url like this: _file=742770 source=("${_pkgname}-${pkgver}.xpi::https://addons.mozilla.org/firefox/downloads/file/${_file}/") See e.g. how I used to do firefox-extension-https-everywhere, and currently do lastpass. It's a bit safer than user-media/addons/$_addon_id/$_upload_filename as the filename sometimes changes in obscure meaningless ways, thus invalidating the download url. I also have a testupdate.sh script uploaded with the packages which scrapes a-m-o for the latest pkgver= and _file= It's a pity that Mozilla doesn't provide a stable API to get an extension by name and version, and the only way is with a file instead. ... Oh, and they do actually provide platform-specific downloads. So this would be all annoying in a different way.
- autogen should really go in prepare(), also autogen should die and be replaced by autoreconf in nearly every case. If they really have weird special sauce in there, at least tell autogen to not run ./configure Switched to autoreconf, seems to work fine with minor fix. Thanks for that hint!
Gotta love autotools. :D
mantisbt: - this incredibly gross install script messes around with unpackaged files, please don't do this Okay. What would be a good alternative though? Using the application during upgrade might potentially break the database. However, the mantis_offline.php file prevents users from interacting with the application. I know that hooks such as the one introduced for nextcloud quite recently (for a while) won't work either.
Hmm, actually I'm not sure what the solution to that is. Has upstream ever considered doing something more sane like autotriggering offline mode when the version in the *database* indicates it is out of date? :D But the major thing that I didn't like was the rm -rf in the post_remove. I cannot think of a good reason to be doing that, unless the application stores user data there, and in that case I don't think you should be deleting user data. The user can clean up their leftover data by hand once they are sure they don't need it anymore, which is safer than causing it to accidentally get deleted when they didn't intend to do so.
- when cat'ing and echo'ing text around to create the primary executable, this belongs in prepare() not build() -- also noticed you *did* file an issue upstream to add this Fixed. Noted, but it hasn't changed (yet).
Yeah, but I'm more accustomed to seeing people not file an upstream issue at all. :D So this made me happy.
- why do you override the user's choice of buildflags and strip, does the package break if you try stripping it or using non-upstream CFLAGS? If you just want to provide debugging symbols by default, don't as you should not be taking that choice away from the user It was mainly because of the following bug: https://bugs.archlinux.org/task/45006 This seems to not apply to pd>0.48 anymore. Yay! Removed.
Oh nice.
Also: Debugging seems to be activated in the main repository as well. oO
I have no good explanation for this, but speps isn't really around much to ask him anymore... Originally added in pd 0.46.7-1 back in 2015 and never changed. This is why it would be nice to have debugging repos, of course. Also special --enable-debug flags are generally not needed when using options=('debug') will enable debug flags in CFLAGS anyway, so if for some reason debug was enabled at all, I'd expect to see that happen in the options array.
- dependencies differ from non -git package, one or the other is probably wrong and should be fixed No. boost-libs has been dropped in favor of asio.
Ah, okay then, fair enough.
- patching should also go in prepare(), and come before autogen on principle Sadly I have to patch configure. Upstream is veeeeeeeeeeeery slow on merging pull requests, so the switch to qt5, that would make this all go away, is still not upon us :( Moved.
Well, no. You "need" to patch configure.ac instead. :D And while I am at it, the `which moc` might also want to be changed.
supercollider-git: - why are there commented-out functions with legacy patches still Fixed.
in-tree? why is the pkgvere() commented out and replaced by one that `git describe`s HEAD? There is no backporting of patches per version. The SuperCollider devs additionally tend to use lightweight and normal tags chaoticly. https://github.com/supercollider/supercollider/issues/2857
git describe HEAD is the same as git describe, the HEAD there just seems a bit out of place (and introduces a very minor degree of complexity to analyze). As per git-describe(1), "Commit-ish object names to describe. Defaults to HEAD if omitted." As for their tagging problems, seems to me there's a blatantly simple solution. :D Just use annotated tags for all versioned releases, and use lightweight tags for whatever inaccurate tag-which-should-be-a-branch they want.... But if you want real fun, see the qbittorrent-git PKGBUILD which I co-maintain. It appears to be policy to maintain a master branch for development and a vX_X_X branch for cherry-picking release commits just in time for tagging a release. Or something.
- why some big complicated switch for configuring based on the CARCH, why don't you just use the $CARCH variable you already have available, and if you actually have to do different things depending on the CARCH, then you can make this a lot easier to read by including extra options in a bash array for safe tokenization. Sounds interesting. Will rework this, as soon as I have my Pis up and running again. Note however, that some hacks are necessary for supercollider to function on a headless system (which embedded systems often are).
Hacks are fine, making the application of those hacks more readable is even better. ;)
Thanks again for the valuable input.
Best, David
Happy to help, this is a bit of a hobby of mine. :) Thanks for the quick response, seeing how TU applicants respond to such critiques is I am pretty sure nearly as important as getting things right the first time. ;) Good luck. -- Eli Schwartz
On 25 October 2017 at 06:12, Eli Schwartz <eschwartz@archlinux.org> wrote:
On 10/24/2017 12:43 PM, Rashif Ray Rahman wrote:
Some issues were raised based on automated tool reports, which I think David will be able to resolve.
Excuse me? I am hardly an "automated tool report". :p You seem to have derived that conclusion out of thin air.
Ahh, crap. Before I get to that, automated tool or not, I forgot to thank you for providing your feedback for David. Now, about that automated tool thing: I did a search for 'xxarhtna' in my archives and got some lines indicating an execution of a script. So, it was not really out of thin air, but most likely my misinterpration. It's likely my absence from IRC that's causing this faux pas. But again, even if it were an automated tool report, it would have been "issues based on automated tool reports", which is feedback nevertheless. And for that one must be thanked.
The reason I never got around to remarking on his packages until very nearly the last day of the discussion period was not just because I was expecting anthraxx to do so first -- I also spent no little time reading every single PKGBUILD of his in the AUR, and cloning the sources for perusal for several of them too.
No, of course, it's not wrong to not be able to commit to anything here. If you choose to provide feedback that means you went out of your way. And in this case you definitely went out of your way to support the applicant's sponsor in raising concerns he did not, which is healthy.
I've seen a lot of PKGBUILDs, and many/most of the simple mistakes or general style failures that people tend to make; I have a list of things to raise red flags over. But that still doesn't mean I can review a PKGBUILD without even looking at it! Maybe one day...
Again, that comment of mine was not really meant to say nobody did anything. I really just wanted to say "issues were raised" or "feedback was given". Me being a wordy person, sometimes extra words get added for free! But yes, if there are indeed red flags that make the applicant look totally incapable or untrustworthy, we should know, and raise hell ourselves. -- GPG/PGP ID: C0711BF1
On 10/25/2017 09:16 PM, Ray Rashif via aur-general wrote:
Now, about that automated tool thing: I did a search for 'xxarhtna' in my archives and got some lines indicating an execution of a script. So, it was not really out of thin air, but most likely my misinterpration. It's likely my absence from IRC that's causing this faux pas.
That's a bit of an in-joke, xxarhtna is just anthraxx's alter-ego.
Again, that comment of mine was not really meant to say nobody did anything. I really just wanted to say "issues were raised" or "feedback was given". Me being a wordy person, sometimes extra words get added for free!
Indeed, I can sympathize. :) Anyway, no harm done. -- Eli Schwartz
On 2017-10-25 00:08:31 (-0400), Eli Schwartz wrote:
Well, yeah. It's more clear what you're doing.
FWIW, I typically use a source url like this:
_file=742770 source=("${_pkgname}-${pkgver}.xpi::https://addons.mozilla.org/firefox/downloads/file/${_file}/") Hmm, this doesn't seem to provide a valid download url for tab tree. In any case, I changed it to a more sane/readable file name already.
It's a pity that Mozilla doesn't provide a stable API to get an extension by name and version, and the only way is with a file instead. Yes.
mantisbt: - this incredibly gross install script messes around with unpackaged files, please don't do this Okay. What would be a good alternative though? Using the application during upgrade might potentially break the database. However, the mantis_offline.php file prevents users from interacting with the application. I know that hooks such as the one introduced for nextcloud quite recently (for a while) won't work either.
Hmm, actually I'm not sure what the solution to that is. Has upstream ever considered doing something more sane like autotriggering offline mode when the version in the *database* indicates it is out of date? :D I think that would indeed be a good feature request. ;-)
But the major thing that I didn't like was the rm -rf in the post_remove. I cannot think of a good reason to be doing that, unless the application stores user data there, and in that case I don't think you should be deleting user data. The user can clean up their leftover data by hand once they are sure they don't need it anymore, which is safer than causing it to accidentally get deleted when they didn't intend to do so. That's correct and I removed it (also: version bump). I should have probably thought about the install script more, when mantisbt was dropped from community [1]. Maybe it was introduced to take care of dead copies of the mantis_offline.php file... but also that doesn't make much sense.
Also: Debugging seems to be activated in the main repository as well. oO
I have no good explanation for this, but speps isn't really around much to ask him anymore...
Originally added in pd 0.46.7-1 back in 2015 and never changed.
This is why it would be nice to have debugging repos, of course. Also special --enable-debug flags are generally not needed when using options=('debug') will enable debug flags in CFLAGS anyway, so if for some reason debug was enabled at all, I'd expect to see that happen in the options array. It would be indeed a very nice thing to have. Often, when running into issues with more complex pieces of software, such as DAWs, it can be very helpful (if you can actually reproduce your issue).
- patching should also go in prepare(), and come before autogen on principle Sadly I have to patch configure. Upstream is veeeeeeeeeeeery slow on merging pull requests, so the switch to qt5, that would make this all go away, is still not upon us :( Moved.
Well, no. You "need" to patch configure.ac instead. :D Whoops. That's correct. Guess I jumped a step, as I rebuilt it very frequently for my thesis. What would be the thing to do for the non-git version of the package though, as configure is already present? I could run autoreconf again I guess.
supercollider-git: git describe HEAD is the same as git describe, the HEAD there just seems a bit out of place (and introduces a very minor degree of complexity to analyze). As per git-describe(1), "Commit-ish object names to describe. Defaults to HEAD if omitted." Fixed.
As for their tagging problems, seems to me there's a blatantly simple solution. :D Just use annotated tags for all versioned releases, and use lightweight tags for whatever inaccurate tag-which-should-be-a-branch they want....
But if you want real fun, see the qbittorrent-git PKGBUILD which I co-maintain. It appears to be policy to maintain a master branch for development and a vX_X_X branch for cherry-picking release commits just in time for tagging a release. Or something. "Not listening..." :>
- why some big complicated switch for configuring based on the CARCH, why don't you just use the $CARCH variable you already have available, and if you actually have to do different things depending on the CARCH, then you can make this a lot easier to read by including extra options in a bash array for safe tokenization. Sounds interesting. Will rework this, as soon as I have my Pis up and running again. Note however, that some hacks are necessary for supercollider to function on a headless system (which embedded systems often are).
Hacks are fine, making the application of those hacks more readable is even better. ;) True. Hope I can (finally) unpack my ARMs this week again.
Happy to help, this is a bit of a hobby of mine. :) Thanks for the quick response, seeing how TU applicants respond to such critiques is I am pretty sure nearly as important as getting things right the first time. ;) Well, I'm always happy to improve things. Especially, if it helps maintainability.
Good luck. Thanks!
[1] https://aur.archlinux.org/cgit/aur.git/commit/?h=mantisbt&id=fd9c1e837b6ccf980b2d760684614b51f76d5a8f -- https://sleepmap.de
On 10/30/2017 06:20 PM, David Runge wrote:
But the major thing that I didn't like was the rm -rf in the post_remove. I cannot think of a good reason to be doing that, unless the application stores user data there, and in that case I don't think you should be deleting user data. The user can clean up their leftover data by hand once they are sure they don't need it anymore, which is safer than causing it to accidentally get deleted when they didn't intend to do so. That's correct and I removed it (also: version bump). I should have probably thought about the install script more, when mantisbt was dropped from community [1]. Maybe it was introduced to take care of dead copies of the mantis_offline.php file... but also that doesn't make much sense.
Yeah, well, just because it was in the official repos doesn't necessarily prove it was a well-written PKGBUILD. :D Even Devs/TUs are human. You'll see a fair number of mistakes in repo PKGBUILDs, e.g. the relatively benevolent unquoted srcdir/pkgdir, which doesn't usually cause trouble in clean chroot builds which the repos coincidentally use... And people do tend to get better over time. *whispers* (Some are more human than others.)
Well, no. You "need" to patch configure.ac instead. :D Whoops. That's correct. Guess I jumped a step, as I rebuilt it very frequently for my thesis. What would be the thing to do for the non-git version of the package though, as configure is already present? I could run autoreconf again I guess.
I've seen it done both ways, changing a string is fairly innocuous... but configure.ac is probably more stable just in terms of future autotools releases potentially changing what the generated configure looks like and causing diff fuzz. Re-running autotools is cheap.
But if you want real fun, see the qbittorrent-git PKGBUILD which I co-maintain. It appears to be policy to maintain a master branch for development and a vX_X_X branch for cherry-picking release commits just in time for tagging a release. Or something. "Not listening..." :>
Coward! :D -- Eli Schwartz
On 24 October 2017 at 22:54, Giancarlo Razzolini <grazzolini@archlinux.org> wrote:
Em outubro 24, 2017 14:43 Rashif Ray Rahman escreveu:
On 18 October 2017 at 18:40, Rashif Ray Rahman <schiv@archlinux.org> wrote:
The discussion begins now and will last until 23 October, after which the voting period will begin on 24 October. Good luck David!
The formal discussion period is over, please cast your votes! The voting period will last until 31 October.
I do have one thing to say to David: please do think about the time commitment associated with this. If you think that you won't be able to commit much starting out, then it will be quite sad moving forward.
Otherwise, I say again that David's proposal has followed standard procedure, including a holistic review by myself the sponsor. Some issues were raised based on automated tool reports, which I think David will be able to resolve.
Good luck!
The link to the vote for the lazy: https://aur.archlinux.org/tu/?id=97
Results are in: Yes No Abstain Total Participation 25 6 8 39 82.98% Congratulations, David, you are now an Arch Linux Trusted User! What's more: - I have updated your account on AUR; - I see you have two usernames on our bugtracker, let us know which username to upgrade. Enjoy! -- GPG/PGP ID: C0711BF1
On Wed, 1 Nov 2017 01:17:40 +0600, Ray Rashif wrote:
Congratulations, David, you are now an Arch Linux Trusted User!
Hi, that is good news. Congratulations David :). Regards, Ralf
On 10/31/2017 03:17 PM, Ray Rashif via aur-general wrote:
Results are in:
Yes No Abstain Total Participation 25 6 8 39 82.98%
Congratulations, David, you are now an Arch Linux Trusted User!
What's more:
- I have updated your account on AUR; - I see you have two usernames on our bugtracker, let us know which username to upgrade.
Congratulations, David! As Ray said, you seem to have two bugtracker usernames, dave.ze.rave and davezerave, but Flyspray throws errors when I try to access the former and the latter is the only one that has actually seen activity, so I have assumed your bugtracker account is https://bugs.archlinux.org/user/9560 I have upgraded it to be part of the Trusted Users group in the Community Packages project, and given you access to the Keyring project as well. -- Eli Schwartz
On November 1, 2017 11:17:27 AM GMT+01:00, Eli Schwartz <eschwartz@archlinux.org> wrote:
On 10/31/2017 03:17 PM, Ray Rashif via aur-general wrote:
Congratulations, David, you are now an Arch Linux Trusted User! Thanks for... well, the trust I guess!
Congratulations, David!
As Ray said, you seem to have two bugtracker usernames, dave.ze.rave and davezerave, but Flyspray throws errors when I try to access the former and the latter is the only one that has actually seen activity, so I have assumed your bugtracker account is https://bugs.archlinux.org/user/9560
I have upgraded it to be part of the Trusted Users group in the Community Packages project, and given you access to the Keyring project as well. Thanks, Eli! I think the former had an issue and might be registered under a now non-existent mail address. It can probably even be deleted, if flyspray supports that.
Anyone lurking in the TU IRC channel and able to send me the access password? Will get around the rest of the TU todos later in the evening. Best, David -- https://sleepmap.de
participants (7)
-
David Runge
-
Eli Schwartz
-
Giancarlo Razzolini
-
Levente Polyak
-
Ralf Mardorf
-
Rashif Ray Rahman
-
Ray Rashif