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.

Sign up for GitHub

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

Jump to bottom

davinci-resolve: fix desktop item, fix mainProgram for studio variant #278164

Merged
merged 4 commits into from
Jun 14, 2024
Merged

davinci-resolve: fix desktop item, fix mainProgram for studio variant #278164

merged 4 commits into from
Jun 14, 2024

Conversation

amarshall
Copy link
Member

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

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@amarshall amarshall mentioned this pull request Jan 1, 2024
Closed
@ofborg ofborg bot requested a review from jshcmpbll January 2, 2024 01:09
@nixos-discourse
Copy link

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

@PAI5REECHO
Copy link

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.

$ find /nix/store/*-davinci-resolve-studio-18.6.4/ -iname '*.desktop' -exec cat '{}' \; | grep -i icon | sort -u
Icon=applications-system
Icon=blackmagicraw-player
Icon=blackmagicraw-speedtest
Icon=RESOLVE_INSTALL_LOCATION/graphics/DV_Panels.png
Icon=RESOLVE_INSTALL_LOCATION/graphics/DV_Resolve.png
Icon=RESOLVE_INSTALL_LOCATION/graphics/DV_Uninstall.png
Icon=RESOLVE_INSTALL_LOCATION/graphics/Remote_Monitoring.png
$ find /nix/store/*-davinci-resolve-studio-18.6.4/ -iwholename '*/graphics/*.png' -exec basename '{}' \; | sort -u
application-x-braw-clip_256x256_mimetypes.png
application-x-braw-clip_48x48_mimetypes.png
application-x-braw-sidecar_256x256_mimetypes.png
application-x-braw-sidecar_48x48_mimetypes.png
blackmagicraw-player_256x256_apps.png
blackmagicraw-player_48x48_apps.png
blackmagicraw-speedtest_256x256_apps.png
blackmagicraw-speedtest_48x48_apps.png
DV_Panels.png
DV_ResolveBin.png
DV_Resolve.png
DV_ResolveProj.png
DV_ResolveTimeline.png
DV_ServerAccess.png
DV_Server.png
DV_TemplateBundle.png
DV_Uninstall.png
Remote_Monitoring.png
watermark.png

@PAI5REECHO
Copy link

PAI5REECHO commented Jan 7, 2024 •
edited
Loading

I think we should try to be consistent with what's in the Debian package and include the additional entries (aside from DaVinciResolveInstaller.desktop):

$ 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.

pbsds
pbsds reviewed Apr 13, 2024
View reviewed changes
pkgs/applications/video/davinci-resolve/default.nix
genericName = "Video Editor";
exec = "resolve";
exec = "davinci-resolve${lib.optionalString studioVariant "-studio"}";
Copy link
Member

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?

@eclairevoyant eclairevoyant mentioned this pull request May 6, 2024
Closed
13 tasks
jshcmpbll
jshcmpbll approved these changes May 27, 2024
View reviewed changes
Copy link
Member

@jshcmpbll jshcmpbll left a 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.

PowerUser64
PowerUser64 suggested changes May 27, 2024
View reviewed changes
Copy link
Contributor

@PowerUser64 PowerUser64 left a comment

Choose a reason for hiding this comment

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

Tried it out. Good news, the desktop item works! My only request is to add its icon.
image

eclairevoyant
eclairevoyant suggested changes May 27, 2024
View reviewed changes
Copy link
Contributor

@eclairevoyant eclairevoyant left a 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.

@amarshall
Copy link
Member Author

Rebased, resolved conflicts, added unrelated fix for mainProgram with studio.

@amarshall amarshall changed the title davinci-resolve: fix desktop item davinci-resolve: fix desktop item, fix mainProgram for studio variant Jun 14, 2024
@amarshall
Copy link
Member Author

Also added icon.

@ofborg ofborg bot requested a review from jshcmpbll June 14, 2024 20:30
@eclairevoyant eclairevoyant merged commit 9906233 into NixOS:master Jun 14, 2024
22 of 24 checks passed
@winterqt winterqt added the backport release-24.05 Backport PR automatically label Sep 17, 2024
@github-actions GitHub Actions
Copy link
Contributor

Successfully created backport PR for release-24.05:

@github-actions github-actions bot mentioned this pull request Sep 17, 2024
Merged
1 task
@nixos-discourse
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pbsds pbsds pbsds left review comments

@PowerUser64 PowerUser64 PowerUser64 requested changes

@eclairevoyant eclairevoyant eclairevoyant approved these changes

@jshcmpbll jshcmpbll Awaiting requested review from jshcmpbll

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer 12.approvals: 1 12.approved-by: package-maintainer backport release-24.05 Backport PR automatically
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DaVinci Resolve Studio is missing .desktop entry for application launchers
9 participants
@amarshall @nixos-discourse @PAI5REECHO @pbsds @eclairevoyant @jshcmpbll @PowerUser64 @wegank @winterqt