-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
davinci-resolve: fix desktop item, fix mainProgram for studio variant #278164
Conversation
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/davinci-resolve-studio-install-issues/37699/12 |
|
I think we should try to be consistent with what's in the Debian package and include the additional entries (aside from $ find /nix/store/*-davinci-resolve-studio-18.6.4/ -iname '*.desktop' -exec basename '{}' \;
davinci-resolve.desktop
DaVinciResolve.desktop
DaVinciResolveCaptureLogs.desktop
DaVinciControlPanelsSetup.desktop
blackmagicraw-player.desktop
DaVinciResolveInstaller.desktop
DaVinciRemoteMonitoring.desktop
blackmagicraw-speedtest.desktop Maybe it makes more sense to copy the original entries and patch them instead. |
0d0b2ea
to
3faf320
Compare
genericName = "Video Editor"; | ||
exec = "resolve"; | ||
exec = "davinci-resolve${lib.optionalString studioVariant "-studio"}"; |
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.
could you also add this logic to meta.mainProgram
?
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 changes LGTM, and I have no objections to the inclusion of the .desktop file in the package. However, my experience with desktopItems is limited. Therefore, while I support these changes, I recommend that they be further reviewed by someone with more expertise in this area to confirm their correctness.
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.
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.
It's still pending the requested change of setting meta.mainProgram
correctly, and there is a merge conflict.
- The exec was wrong—perhaps correct once-upon-a-time, but no more - Differentiate for studio version
3faf320
to
51e585c
Compare
Rebased, resolved conflicts, added unrelated fix for mainProgram with studio. |
Also added icon. |
Successfully created backport PR for |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/davinci-resolve-does-not-show-up-in-applications-of-the-de/52298/2 |
Description of changes
Should fix #278133. Unfortunately no icon file, perhaps it’s somewhere, but I just looked for “icon” files in the drv output and came up with only seemingly-irrelevant results.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-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.