[aur-general] TU Application: David Runge

[Eli Schwartz]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.archlinux.org/pipermail/aur-general/attachments/20171025/6fa3d346/attachment.asc>