juce: enable extras (includes Projucer) #288898
Conversation
ofborg
bot
added
11.by: package-maintainer
pkgs/top-level/all-packages.nix
Outdated
| juce = callPackage ../development/misc/juce { | ||
| juce = callPackage ../by-name/ju/juce/package.nix { |
There was a problem hiding this comment.
As part of RFC0140 we no longer require a definition in all-packages.nix. We can remove the juce definition and declaration here.
To confirm this is working you should be able to run nix-build -A juce without issue.
There was a problem hiding this comment.
The issue is that we need to override the default stdenv so that it uses the 11.0 sdk in darwin. Should this be done within the package instead ?
There was a problem hiding this comment.
Sorry, I didn't see that we were overriding stdenv for darwin. This looks fine
kashw2
added
the
12. first-time contribution
|
Addresses #252737 (comment) |
|
Result of 1 package built:
|
delroth
added
12.approvals: 1
|
Looks like this fails on: |
|
I think it's probably different paths in the build folder. I will investigate. |
liarokapisv
force-pushed
the
juce_add_extras
branch
from
70e115b to
2b47017
Compare
delroth
removed
the
12.approvals: 1
liarokapisv
force-pushed
the
juce_add_extras
branch
from
2b47017 to
9e17cd1
Compare
delroth
added
the
12.approvals: 1
|
Any idea why the |
|
@grahamc do you know why ofBorgs by-name check fails with a file that isn't even touched in this PR?
while only |
|
Tested building on a raspberry pi 4b and x86_64 (both NixOS), works just fine in both places from what I can tell! (UI is untested on arm, but I assume it works.) |
PowerUser64
added
12.approvals: 2
pkgs/by-name/ju/juce/package.nix
Outdated
| appSuffix = isApp: if appCheck isApp then ".app" else ""; | ||
| outDir = isApp: if appCheck isApp then "$out/Applications" else "$out/bin"; | ||
| artifactPath = name: isApp: "extras/${name}/${name}_artefacts/Release/${name}${appSuffix isApp}"; | ||
| mvArtefact = name: isApp: "mv ${artifactPath name isApp} ${outDir isApp}"; |
There was a problem hiding this comment.
Should this be mvArtifact? Artefact seems like a spelling error.
Edit: Turns out artefact is the British English spelling and artifact is the US English spelling. I see that it's spelled as artifact later on in this line (artifactPath). I think we should pick one or the other to go with rather than mixing and matching.
| ]; | ||
|
|
||
| cmakeFlags = [ | ||
| "-DJUCE_BUILD_EXTRAS=ON" |
There was a problem hiding this comment.
Maybe we should have an override in inputs to disable this. It can be on by default (I think this is the only way it can get cached without creating an entry in all-packages.nix?), but it might not always be desirable to have this.
| '' | ||
| ${lib.optionalString stdenv.isDarwin "mkdir $out/Applications"} | ||
|
|
||
| ${mvArtefact "Projucer" true} |
There was a problem hiding this comment.
I feel like there should be a desktop item for this, not sure if it typically exists on other distros. Thoughts?
(check out makeDesktopItem and copyDesktopItems if you want to try doing this)
PowerUser64
added
12.approvals: 1
wegank
added
the
2.status: merge conflict
pkgs/by-name/ju/juce/package.nix
Outdated
| meta = with lib; { | ||
| description = "Cross-platform C++ application framework"; | ||
| longDescription = "JUCE is an open-source cross-platform C++ application framework for desktop and mobile applications, including VST, VST3, AU, AUv3, RTAS and AAX audio plug-ins"; | ||
| homepage = "https://github.com/juce-framework/JUCE"; | ||
| license = with licenses; [ isc gpl3Plus ]; | ||
| maintainers = with maintainers; [ liarokapisv kashw2 ]; | ||
| platforms = platforms.all; | ||
| }; |
There was a problem hiding this comment.
It might be worth adding mainProgram = Projucer in here.
pkgs/by-name/ju/juce/package.nix
Outdated
|
|
||
| stdenv.mkDerivation (finalAttrs: { | ||
| pname = "juce"; | ||
| version = "7.0.9"; |
There was a problem hiding this comment.
JUCE 7.0.10 is out, so this should probably be updated.
|
Should I perhaps avoid adding the other extra programs in the derivation and just include Projucer? |
Do you mean adding an entry to If so, I think that's a good idea. Projucer especially makes sense to have available as a separate package. PS: while you're at it, it might be a good opportunity to rebase to master and fix the merge conflict. |
|
I was actually referring to multi-output derivations but having a juce-tools package or similar like debian does would not be out of the question. |
|
Ah yes, that would be very nice to have here as well. What do you think about having both a multi-derivation output "everything" package, as well as a smaller one that just has the JUCE library, for use in other packages? That way there's a package for if you want to do general JUCE development (desktop use), as well as one that is suitable for if you just need to compile something that uses JUCE but don't want to compile projucer and the kitchen sink. |
|
I am not sure about it, I think it would be better to get more opinions on this. It may be worth adding the optional juce_extras attribute for now and iterate at a later time. |
liarokapisv
force-pushed
the
juce_add_extras
branch
2 times, most recently
from
fa9d36d to
320c0de
Compare
ofborg
bot
removed
2.status: merge conflict
@NobbZ Seems that this is fixed now |
wegank
removed
the
12.approvals: 1
wegank
added
the
2.status: merge conflict
This commit adds support for the various extra Juce utilities that are hidden behind the `JUCE_BUILD_EXTRAS` cmake configuration flag. There are no installation targets for these so we manually install them.
liarokapisv
force-pushed
the
juce_add_extras
branch
from
320c0de to
41dd3cc
Compare
github-actions
bot
added
the
8.has: maintainer-list (update)
ofborg
bot
removed
the
2.status: merge conflict
wegank
added
the
2.status: merge conflict
wegank
removed
the
12.approved-by: package-maintainer
|
Hi, I tried rebasing this patch on for JUCE 8 and ran into font-related assertion failures and NULL-derefs with missing dynamic libraries. Your JUCE 7 version also complained about fonts but it seems like JUCE recently changed how they link/load the Xorg libraries. Here's the changes I had to make to get Projucer to start with JUCE 8: 🙂 |
|
@Mrmaxmeier Have you seen what the BespokeSynth package does to solve this? It looks like it's similar to what you're doing, but might be cleaner since it solves the problem earlier on in the build process. nixpkgs/pkgs/applications/audio/bespokesynth/default.nix Lines 151 to 161 in 3ffbbdb |
wegank
added
the
2.status: stale
Description of changes
This commit adds support for the various extra Juce utilities that
are hidden behind the
JUCE_BUILD_EXTRAScmake configuration flag.The most important of these is probably the Projucer program that is responsible for Juce project scaffolding.
There are no installation targets for these so we manually install them.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.