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

UTM: fix meta.mainProgram field #224435

Merged
merged 2 commits into from
Apr 7, 2023
Merged

UTM: fix meta.mainProgram field #224435

merged 2 commits into from
Apr 7, 2023

Conversation

hraban
Copy link
Member

@hraban hraban commented Apr 3, 2023

Description of changes

Fixes nix run nixpkgs#utm

To test:

$ nix run github:hraban/nixpkgs/utm-mainprogram#utm

Once merged, that will be nix run nixpkgs#utm

As far as I know nix run is the only actual use for meta.mainProgram, but if there's anything else that I overlooked please let me know.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.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.

Fixes `nix run nixpkgs#utm`
@rrbutani
Copy link
Contributor

rrbutani commented Apr 3, 2023

oh hmm..

I believe lib.getExe and friends also use mainProgram. I believe the relative path will work but I'd rather have bin/UTM exist than have an empty bin but creating a symlink does not seem to work (UTM seems to look for files in the wrong places).

Perhaps we can make a wrapper, like SwiftBar does?

# Symlinking doesnt work; The auto-updater will fail to start which renders the app useless
makeWrapper $out/Applications/SwiftBar.app/Contents/MacOS/SwiftBar $out/bin/SwiftBar

Copy link
Contributor

@rrbutani rrbutani left a comment

Choose a reason for hiding this comment

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

Perhaps something like this (GitHub won't let me leave suggestions on the appropriate lines, sorry):

diff --git a/pkgs/os-specific/darwin/utm/default.nix b/pkgs/os-specific/darwin/utm/default.nix
index 2e21945288e..c88005f09f2 100644
--- a/pkgs/os-specific/darwin/utm/default.nix
+++ b/pkgs/os-specific/darwin/utm/default.nix
@@ -1,5 +1,6 @@
 { lib
 , undmg
+, makeWrapper
 , fetchurl
 , stdenvNoCC
 }:
@@ -13,12 +14,23 @@ stdenvNoCC.mkDerivation rec {
     hash = "sha256-YOmTf50UUvvh4noWnmV6WsoWSua0tpWTgLTg+Cdr3bQ=";
   };
 
-  nativeBuildInputs = [ undmg ];
+  nativeBuildInputs = [ undmg makeWrapper ];
 
   sourceRoot = ".";
   installPhase = ''
+    runHook preInstall
+
     mkdir -p $out/Applications
     cp -r *.app $out/Applications
+
+    mkdir -p $out/bin
+    for bin in $out/Applications/UTM.app/Contents/MacOS/*; do
+      # Symlinking `UTM` doesn't work; seems to look for files in the wrong
+      # place
+      makeWrapper $bin "$out/bin/$(basename $bin)"
+    done
+
+    runHook postInstall
   '';
 
   meta = with lib; {

(The wildcard on MacOS/* is so that utmctl gets picked up; I think it'd be nice to have a passthru test that makes sure that this runs just so we have some basic testing for this package — I can take care of that in a follow up PR though)

@hraban
Copy link
Member Author

hraban commented Apr 3, 2023

You sound like you know more about it than I do so if your fix handles nix run nixpkgs#utm, and also is more idiomatic and fixes more stuff, then I'm a happy camper :) thank you for your time.

@hraban
Copy link
Member Author

hraban commented Apr 3, 2023

Pushed a new patch with your changes and it seems to work.

@ofborg ofborg bot requested a review from rrbutani April 3, 2023 18:06
@ulrikstrid
Copy link
Member

Result of nixpkgs-review pr 224435 run on x86_64-linux 1

@Artturin Artturin merged commit 1a7e2b8 into NixOS:master Apr 7, 2023
@hraban hraban deleted the utm-mainprogram branch May 2, 2023 16:03
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

4 participants