Skip to content
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

devendor rtaudio #315

Open
dvzrv opened this issue Feb 19, 2020 · 22 comments
Open

devendor rtaudio #315

dvzrv opened this issue Feb 19, 2020 · 22 comments
Assignees
Milestone

Comments

@dvzrv
Copy link
Contributor

dvzrv commented Feb 19, 2020

Rtaudio is a package, that is available on most Linux distributions by default.

Currently it's not (easily) possible to build against a system installed version of rtaudio. All submodule depedendencies are not included in release tarballs on github: https://github.com/monocasual/giada/releases

It would be great to get a configure option to build against a system installed rtaudio (or handle this through auto-detection).

As a sidenote, I'd like to point out, that giada is in Arch Linux' official repositories and not in the AUR anymore. :)

@monocasual
Copy link
Collaborator

The problem here is that Giada needs a custom version of RtAudio to enable JACK transport capabilities on Linux (and macOS). I've recently opened a pull request to discuss whether our changes could be somehow merged in the official RtAudio code base.

Currently RtAudio is included as a git submodule that points to our custom version. Is this bad for packagers?

As a sidenote, I'd like to point out, that giada is in Arch Linux' official repositories and not in the AUR anymore. :)

Sounds great!

@dvzrv
Copy link
Contributor Author

dvzrv commented Feb 20, 2020

Currently RtAudio is included as a git submodule that points to our custom version. Is this bad for packagers?

If the version is not included in a tarball, yes, as in this case packagers have to rely on (in giada's case) unverifiable - as in: not signed - tags/commits of the repository directly to get to the code, which might or might not work for some distros/downstreams.

In general it's of course also advisable to use rtaudio verbatim for releases and modified versions at best in test releases (e.g. alpha). The current release (0.16.2) doesn't allow using a system version of rtaudio (even if the upstream packages the correct version) due to the modification (and prevents me from packaging this version currently).

@monocasual
Copy link
Collaborator

monocasual commented Feb 22, 2020

@dvzrv please forgive my extreme ignorance when it comes to packaging stuff for Linux distros - I'm willing to learn new things :) So, if I understand correctly, an external library is verifiable (i.e. signed) only if it comes from the distro repository or is included in a signed tarball. Right?

Since we cannot use - so far - the system version of rtaudio, the only option is to hardcode it into the tarball.

@dvzrv
Copy link
Contributor Author

dvzrv commented Feb 26, 2020

No worries. There's always plenty of things developers can not be aware of. ;-)

So, if I understand correctly, an external library is verifiable (i.e. signed) only if it comes from the distro repository or is included in a signed tarball. Right?

No and yes, the verification can be established by upstream (i.e. you) by e.g. adding a detached PGP signature for a specific artifact/ asset (e.g. source tarball, prebuilt binary tarball), uploading the public key in use to the keyservers and notifying any users about the keypair in use (e.g. README or elsewhere).
Another way to go about it is to sign commits and/or tags with the aforementioned PGP keypair. This way downstreams are able to check out a specific version/commit and verify it (and its submodules).

The latter is particularly useful in the case where git submodules are being used, as they are not included in snapshot tarballs (the ones, that are automatically created upon tagging a repository on github).
When tarballs are being created by the author (i.e. with the submodules at the given commit included) the first variant is enough, as trust is derived from the upstream creating the tarball and signing it. In said case even the plain tarball with all required sources (without a detached signature) would be at least something downstreams can check (by comparing a checksum of a tarball).

As currently neither is possible, I can not simply check out e.g. 0.16.2 and include the submodules, as this would degrade the current security guards: Tarball retrieval (i.e. downloading a tarball that can be compared against a checksum) with TOFU.

btw: A possible way around the tarball issues when using git submodules is to rely upon git-subtree!

Since we cannot use - so far - the system version of rtaudio, the only option is to hardcode it into the tarball.

Yes, currently the only way to release a proper tarball would be to create one e.g. by script which includes all the submodules and then upload it as a separate asset (which has been done for 0.16.0).
Additionally it is of course always awesome, if upstreams start using PGP signatures for (annotated!) tags and /or even for commits (when only lightweight tags are in use) and the released artifacts.

@monocasual
Copy link
Collaborator

monocasual commented Feb 29, 2020

@dvzrv , first of all thank you very much for the detailed explanation! I'm about to push a new version 0.16.2.1 with the fix.

Now, let's pretend the RtAudio issue is solved (that is, our changes are included in the official repo). What if:

  • all external dependencies are git submodules;
  • a regular user that just wants to build Giada would simply check out the git repo, initialize submodules and build;
  • a packager would pass a new --enable-system-dependencies flag to the configure script to instruct it to link against system libraries instead of the git submodules.

We already have something like this for Catch (the unit-test library) as you may see here. Could this be a viable solution?

monocasual pushed a commit that referenced this issue Mar 4, 2020
@dvzrv
Copy link
Contributor Author

dvzrv commented Mar 7, 2020

Yes, ideally the dependencies are detected by the build system and action is being taken due to that (either the system version of a dependency is used, or the vendored one if available). This can also be done manually via flags of course.
At any rate, there should be some sort of feedback in output in regards to this.

The external dependencies as git submodules only work (from a packaging perspective), if they will be included in a source release tarball (as is done for 16.1) , or if the git tags/commits are verifiable (i.e. PGP) themselves.

As a sidenote: The catch flag doesn't work (for me), because I still have to modify the includes of the tests afterwards (the headers are namespaced):

# fixing catch2 include for system library
sed -e 's|catch\.hpp|catch2/catch\.hpp|g' -i tests/*.cpp

@monocasual monocasual self-assigned this Mar 16, 2020
@monocasual monocasual added this to the 1.0 milestone Mar 16, 2020
@dvzrv
Copy link
Contributor Author

dvzrv commented Apr 1, 2020

@monocasual has there been any progress on getting a source tarball for 0.16.2.2, that includes the sources of your modified rtaudio and the nlohmann-json submodule?

@dvzrv
Copy link
Contributor Author

dvzrv commented Apr 1, 2020

If you're interested I can provide a script to create a source tarball with all submodules included, that can optionally also sign that source tarball with your PGP key.
I've provided a similar script to other projects such as SuperCollider already.

@monocasual
Copy link
Collaborator

@dvzrv the script would be great, while waiting for the --enable-system-dependencies flag.

@monocasual
Copy link
Collaborator

@dvzrv , @eeickmeyer I have generated an experimental tarball thanks to the super cool script by @dvzrv (no PGP signature, this is just a test). The package looks good to me, temporarily hosted here in case you want to give it a spin. Please let me know if you find something weird or missing.

@eeickmeyer
Copy link

I'll go ahead and give that a spin sometime today. @dvzrv rocks my face off. :)

@eeickmeyer
Copy link

Ok, I can confirm that the script worked without a hitch. I was able to build the package locally. Do you want to do a point-release with the new tarball?

@dvzrv
Copy link
Contributor Author

dvzrv commented Apr 8, 2020

@monocasual this builds for me (apart from #328) and runs (I was even able to devendor nlohmann-json).

@monocasual
Copy link
Collaborator

Good news then :). @eeickmeyer sure, as soon as #328 is solved.

So, just to recap (forgive me, I'm slow on the uptake today):

  • we (Giada) currently package and publish tarballs on our website by using the new script;
  • you (packagers) grab that tarball on our website and build it for your distros;

When the --enable-system-dependencies flag discussed here will be implemented in our build system:

  • we (Giada) can get rid of the tarball on our website;
  • regular users will build Giada by cloning the GitHub repo;
  • you (packagers) will build Giada by cloning the GitHub repo WITH the --enable-system-dependencies flag enabled.

OK?

@dvzrv
Copy link
Contributor Author

dvzrv commented Apr 10, 2020

When the --enable-system-dependencies flag discussed here will be implemented in our build system:

Upon further thinking about this, I believe that a per-vendered library flag would make more sense.
The rationale being, that a) not all downstreams might have the required libraries b) not all downstreams might have one or all required libraries in the required versions c) a one-size-fits-all usually doesn't work too well and would have less control over the situation (for both sides).

* we (Giada) can get rid of the tarball on our website;

Not to be a party pooper, but it would actually be great to provide it in the future as well, as it ensures, that anyone can build it from source without using git and without having all the system libraries available.
The rationale is as explained earlier: No modification to the trust model applied by a downstream (e.g. a Linux distribution) to an upstream (giada) has to be done (e.g. change from verifying a tarball checksum to using a (currently) non-verifiable git commit). Note, that some distributions might not even properly support cloning from a git repository for packaging purposes (maybe @eeickmeyer can say more about that doing packaging for both Ubuntu Studio and Fedora Jam).

* regular users will build Giada by cloning the GitHub repo;

Or use the tarball and have everything to go for that specific version out-of-the-box.

* you (packagers) will build Giada by cloning the GitHub repo WITH  the  `--enable-system-dependencies` flag enabled.

As mentioned earlier: It would probably make more sense to provide a per-lib flag to have better control over what is supposed to be built with system libraries instead of the vendored ones.
It also allows for better control over the libraries you can currently support to be used from the system (e.g. rtaudio currently is not possible due to the modification).

@monocasual
Copy link
Collaborator

@dvzrv very interesting and insightful feedback as always, thank you very much. I agree, adding more granularity with the per-library flag seems a sensible choice. Good idea also on keeping the tarball on the website in case some distributions don't support/allow the git workflow.

@monocasual
Copy link
Collaborator

New tarball for 0.16.2.2 published here. I've also optimized the archive a bit (from ~150 down to ~5 MB).

@monocasual monocasual modified the milestones: 0.17 series, 0.17.1 Oct 2, 2020
@monocasual monocasual removed their assignment Nov 23, 2020
@gvnnz gvnnz self-assigned this Nov 23, 2020
@gvnnz
Copy link
Contributor

gvnnz commented Dec 11, 2020

Commit eac7498 includes, beyond experimental support for vcpkg, the ability to use system libraries. Namely:

  • WITH_SYSTEM_RTMIDI
  • WITH_SYSTEM_FLTK
  • WITH_SYSTEM_LIBSNDFILE
  • WITH_SYSTEM_LIBSAMPLERATE
  • WITH_SYSTEM_CATCH

The default value is OFF, so by default CMake uses the version provided by vcpkg. When ON, the system-wide package is used instead.

RtAudio dependency is now a git submodule pointing to our fork and the source will be included in the tarball as usual. @dvzrv looks good to you?

@dvzrv
Copy link
Contributor Author

dvzrv commented Dec 12, 2020

@gvnnz thanks, that's a good set of options.

In the case of rtaudio, are there any advances of upstreaming your customizations?

@gvnnz
Copy link
Contributor

gvnnz commented Dec 12, 2020

In the case of rtaudio, are there any advances of upstreaming your customizations?

WIP in thestk/rtaudio#251. I'll keep you posted...

@gvnnz
Copy link
Contributor

gvnnz commented Dec 26, 2020

Errata: the whole WITH_SYSTEM_* was plain wrong when used together with vcpkg. Now CMake always searches for system libraries (except for RtAudio), unless the DCMAKE_TOOLCHAIN_FILE option is passed.
Also, I'm moving this to 1.0 milestone as I suspect the RtAudio customization will take some time.

@gvnnz gvnnz modified the milestones: 0.17.1, 1.0 Dec 26, 2020
@gvnnz gvnnz modified the milestones: 1.0, 1.future Dec 17, 2023
@gvnnz
Copy link
Contributor

gvnnz commented Dec 17, 2023

...aaand after 4 years we are still stuck with the custom RtAudio lib. I'm sorry guys, I fear this will take some/undefined time. Moving to 1.future. 💔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants