addpkg(main/supercollider): 3.14.0#25025
addpkg(main/supercollider): 3.14.0#25025rene-descartes2021 wants to merge 1 commit intotermux:masterfrom
Conversation
TomJo2000
left a comment
TomJo2000
left a comment
There was a problem hiding this comment.
Thank you for your contribution.
This looks mostly good, I just have feedback on some minor things.
(This is a pre-written, saved reply.)
Please make sure to keep your commits squashed by the way.
For adding to a single commit you can use git commit --amend.
Since you already have multiple commits on your branch though,
you'll need to squash those with git rebase -i HEAD~<n> first.
(Where <n> is the number of commits you want to modify.
Please make sure to only modify your commits.)
https://www.baeldung.com/ops/git-squash-commits#1-squash-the-last-x-commits
Since squashing or amending commits changes the git history you will need to force push any such changes.
e.g. git push --force,
or preferably git push --force-with-lease --force-if-includes
to make sure you aren't clobbering any refs you haven't fetched locally yet.
| TERMUX_PKG_DESCRIPTION="A platform for audio synthesis and algorithmic composition" | ||
| TERMUX_PKG_LICENSE="GPL-3.0" | ||
| TERMUX_PKG_MAINTAINER="@rene-descartes2021" | ||
| TERMUX_PKG_VERSION=3.14.0-rc1 |
There was a problem hiding this comment.
We don't usually ship rc versions.
So please either change this to the latest stable release, which seems to be 3.13.1
Or use a ~ instead of a - to indicate that this is a version before 3.14.0.
https://www.debian.org/doc/debian-policy/ch-controlfields.html#version
There was a problem hiding this comment.
Ok I'll just close this PR and reopen it with all the corrections when 3.14.0 comes out in a week or so.
3.13.1 requires a handful more patches to compile so best to just wait.
Thanks for the review!
There was a problem hiding this comment.
The PR can stay the same.
You just have to force push your changes to rebase it.
3.14.0-rc1 will be fine as the version.
We just need to use a ~ to ensure it's correctly picked up as a lower version than 3.14.0
| TERMUX_PKG_LICENSE="GPL-3.0" | ||
| TERMUX_PKG_MAINTAINER="@rene-descartes2021" | ||
| TERMUX_PKG_VERSION=3.14.0-rc1 | ||
| TERMUX_PKG_SRCURL=https://github.com/supercollider/supercollider/releases/download/Version-${TERMUX_PKG_VERSION}/SuperCollider-${TERMUX_PKG_VERSION}-Source.tar.bz2 |
There was a problem hiding this comment.
Using the ~rc1 qualifier in the version will necessitate a substitution in the source URL.
| TERMUX_PKG_SRCURL=https://github.com/supercollider/supercollider/releases/download/Version-${TERMUX_PKG_VERSION}/SuperCollider-${TERMUX_PKG_VERSION}-Source.tar.bz2 | |
| TERMUX_PKG_SRCURL=https://github.com/supercollider/supercollider/releases/download/Version-${TERMUX_PKG_VERSION/~/-}/SuperCollider-${TERMUX_PKG_VERSION/~/-}-Source.tar.bz2 |
We also usually use the GitHub provided release source tarballs.
| TERMUX_PKG_SRCURL=https://github.com/supercollider/supercollider/releases/download/Version-${TERMUX_PKG_VERSION}/SuperCollider-${TERMUX_PKG_VERSION}-Source.tar.bz2 | |
| TERMUX_PKG_SRCURL=https://github.com/supercollider/supercollider/archive/refs/tags/Version-${TERMUX_PKG_VERSION/~/-}.tar.gz |
Unless there's a specific reason to use the .tar.bz2 version of course.
There was a problem hiding this comment.
No reason I know of- I'll ask them.
There was a problem hiding this comment.
So I just looked at the difference in the source tarballs, looks like the SuperCollider provided ones have a bunch of code in subdirs of external_libraries/, probably subrepos checked out. The GitHub provided tarballs don't have those. So I'll use the SuperCollider tarball.
| -DNO_AVAHI=ON | ||
| -DSC_ABLETON_LINK=OFF | ||
| -DSYSTEM_YAMLCPP=ON | ||
| -DSYSTEM_BOOST=OFF |
There was a problem hiding this comment.
Is there a specific reason to not use the boost package available in Termux?
There was a problem hiding this comment.
Two reasons:
1)Using the system boost will result in a runtime failure, there is an error in 'boost interprocess' where it assumes and uses /tmp and that will fail on Terumx. One of the patches in this commit is to patch the bundled boost to fix that error.
2)There exists some sort of plugin to SuperCollider which requires the same version of boost to be used. I'm not really sure if this is a reason I haven't really looked into it, but I think it's the reason they bundle it in the first place.
If the first issue gets patched in boost and makes it's way to system boost I think it can be reconsidered then to use system boost.
There was a problem hiding this comment.
Two reasons: 1)Using the system boost will result in a runtime failure, there is an error in 'boost interprocess' where it assumes and uses
/tmpand that will fail on Terumx. One of the patches in this commit is to patch the bundled boost to fix that error. 2)There exists some sort of plugin to SuperCollider which requires the same version of boost to be used. I'm not really sure if this is a reason I haven't really looked into it, but I think it's the reason they bundle it in the first place.If the first issue gets patched in boost and makes it's way to system boost I think it can be reconsidered then to use system boost.
Yep that makes sense, thanks.
Your boost patch should probably also be ported to the termux package of boost.
I'll look into it.
There was a problem hiding this comment.
boost/tmp
It should now be completely fixed after
rene-descartes2021
changed the title
|
Won't let me reopen it looks like. Well 3.14.0 is on my rene-descartes2021:supercollider branch. Dunno why it didn't update here even though I force pushed. |
|
@TomJo2000 reopen? I don't have permissions to. |
|
I can't either. Just open a new PR then. |
- Fixes a problem explained here (that `boost` lacks needed `/tmp` path patches) #25025 - Methodology to find paths to prepend: - Ignore all folders named `doc` - Ignore all folders named `test` - Find them using `grep -rn $TERMUX_PKG_SRCDIR -e '"/tmp' -e 'data/local/tmp'` - Ignore codepaths that don't compile for Android (`#ifndef __ANDROID__`) - Try to calculate the code behavior to make sure every patched instance is a true absolute path (and not a path subcomponent that could be appended to a longer path) - Dependency of #25826
3 participants
Adds SuperCollider #25024.
I've built it on device successfully, but have not built it through termux-packages, I'm curious to know if I can use CI to see if it will build fine?