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

open-music-kontrollers: init #106822

Merged
merged 2 commits into from Mar 20, 2022
Merged

open-music-kontrollers: init #106822

merged 2 commits into from Mar 20, 2022

Conversation

magnetophon
Copy link
Member

@magnetophon magnetophon commented Dec 13, 2020

Previously this was a WIP, now everything works.

Should I create a separate commit for each plugin?
Should each of those commits be on top of the last?

What about the entries in all-packages.nix; should I scatter them around or keep them grouped?
Maybe they should go in a subdomain, like open-music-kontrollers.vm, also because of the generic name of some of them.

@SuperSandro2000 I fixed the merge conflict.

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

pkgs/applications/audio/open-music-kontrollers/generic.nix Outdated Show resolved Hide resolved
pkgs/applications/audio/open-music-kontrollers/generic.nix Outdated Show resolved Hide resolved
pkgs/applications/audio/open-music-kontrollers/generic.nix Outdated Show resolved Hide resolved
inherit description;

src = fetchurl {
url = url;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of them at least can probably use fetchGit which should probably be used instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should I do that in this situation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure tbh because the sources are all a bit different. Maybe we can pass in src instead of url and sha256?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the downside of keeping it like this?
I find it quite elegant. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do these snapshots work? Do they need to be generated by hand? if so they could disappear and not exist for every commit so updating to another git rev would be unnecessarily hard.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are automatically generated.
Does that make my code OK, or did I misunderstand?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it might be fine. Not fully sure to be honest.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this in an ok state now?

pkgs/applications/audio/lv2lint/default.nix Outdated Show resolved Hide resolved
@stale
Copy link

stale bot commented Jan 9, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md and removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Jan 9, 2022
@auroraanna
Copy link
Contributor

@magnetophon
Copy link
Member Author

@papojari Thanks for the hint.
@pennae Would you mind taking a look at this PR?

@auroraanna
Copy link
Contributor

I think it would make sense to make a category of these packages.

@magnetophon
Copy link
Member Author

@papojari What do you mean by category?

@auroraanna
Copy link
Contributor

a package set

@magnetophon
Copy link
Member Author

Like magnetophonDSP?

@auroraanna
Copy link
Contributor

yes, you already said it in the top comment

Maybe they should go in a subdomain, like open-music-kontrollers.vm, also because of the generic name of some of them.

Copy link
Contributor

@pennae pennae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the derivations looks fine, but at least lv2lint and patchmatrix are outdated. would also agree with @papojari that at least the lv2 plugins should not live at the nixpkgs toplevel, their names are a little too generic sometimes.

if you want to take over patchmatrix maintenance that's also fine, as long as we avoid having two of them :)

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@magnetophon
Copy link
Member Author

magnetophon commented Mar 9, 2022

@papojari

yes, you already said it in the top comment

Ah, yes, of course. Sorry for being slow.
I put them in a category now.

@auroraanna
Copy link
Contributor

That's just a convenient way to scan for plugins

Shouldn't this be in the program itself?

@magnetophon
Copy link
Member Author

Shouldn't this be in the program itself?

¯\_(ツ)_/¯

pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@ventosus
Copy link

ventosus commented Mar 15, 2022

hi

Synthpod by default runs plugin guis in separate processes and thus can mix plugins with gtk2 and gtk3 and (qt4 xor qt5) guis in the same project. The build system only supports qt4 or qt5 though, not both at the same time.

The gtk/qt backends are optional as most plugin guis need a x11 backend.

EDIT:
The toolkits are not linked into the same app, which would make no sense. There are dedicated binaries to run the guis with communication to the main app via shared memory:

synthpod_sandbox_gtk2  synthpod_sandbox_kx    synthpod_sandbox_show
synthpod_sandbox_gtk3  synthpod_sandbox_qt5   synthpod_sandbox_x11

@magnetophon
Copy link
Member Author

@ventosus Thanks for clearing that up!
@SuperSandro2000 I removed gtk2 because hardly any plugins use it and I removed qt4 in favor of qt5.

@ventosus
Copy link

I removed gtk2 because hardly any plugins use it and I removed qt4 in favor of qt5.

I'd argue in the other direction, there are hardly any Gtk3 Uis but loads of Gtk2, but well, I'm not running NixOS ;-)

$ grep -l GtkUI $( find /usr/lib/lv2 -name '*.ttl' 2>/dev/null ) 2>/dev/null | wc -l
83
$ grep -l Gtk3UI $( find /usr/lib/lv2 -name '*.ttl' 2>/dev/null ) 2>/dev/null | wc -l
2

@magnetophon
Copy link
Member Author

I'd argue in the other direction

Thanks, done.

@SuperSandro2000
Copy link
Member

@magnetophon please rebase on master.

@magnetophon
Copy link
Member Author

@magnetophon please rebase on master.

OK, done.

@magnetophon
Copy link
Member Author

Thanks!

@magnetophon magnetophon deleted the OMK branch March 20, 2022 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants