-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
addpkg(main/supercollider): 3.14.0 #25025
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
TomJo2000
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason I know of- I'll ask them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a specific reason to not use the boost package available in Termux?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
boost/tmp
It should now be completely fixed after
|
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. |
- 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
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?