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

lima: 0.13.0 -> 0.14.2 #206285

Merged
merged 5 commits into from
Jan 23, 2023
Merged

lima: 0.13.0 -> 0.14.2 #206285

merged 5 commits into from
Jan 23, 2023

Conversation

jbgosselin
Copy link
Contributor

@jbgosselin jbgosselin commented Dec 15, 2022

Description of changes

Updates performed

  • Golang update
  • Added dependency to xcrun on darwin to detect if can compile with Virtualization.framework
  • Added dependency to sigtool on darwin for needed codesign command
  • Modify Makefile to --force codesign due to golang signing aarch64-darwin binaries by default

To inspect upstream changes

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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@deejayem
Copy link
Member

Result of nixpkgs-review pr 206285 run on x86_64-darwin 1

2 packages built:
  • colima
  • lima

Copy link
Member

@deejayem deejayem left a comment

Choose a reason for hiding this comment

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

Built and tested successfully (via colima and docker/docker-compose) on x86_64-darwin.

But ofborg says lima.passthru.tests on aarch64-darwin failed:

> building
> mkdir -p _output/bin
> cp -a ./cmd/lima _output/bin/lima
> # The hostagent must be compiled with CGO_ENABLED=1 so that net.LookupIP() in the DNS server
> # calls the native resolver library and not the simplistic version in the Go library.
> CGO_ENABLED=1 go build -ldflags="-s -w -X github.com/lima-vm/lima/pkg/version.Version=v0.14.1" -tags "no_vz" -o _output/bin/limactl ./cmd/limactl
> codesign --entitlements vz.entitlements -s - ./_output/bin/limactl
> libc++abi: terminating with uncaught exception of type std::runtime_error: file is already signed. pass -f to sign regardless.
> make: *** [Makefile:182: codesign] Abort trap: 6

@jbgosselin
Copy link
Contributor Author

Built and tested successfully (via colima and docker/docker-compose) on x86_64-darwin.

But ofborg says lima.passthru.tests on aarch64-darwin failed:

> building
> mkdir -p _output/bin
> cp -a ./cmd/lima _output/bin/lima
> # The hostagent must be compiled with CGO_ENABLED=1 so that net.LookupIP() in the DNS server
> # calls the native resolver library and not the simplistic version in the Go library.
> CGO_ENABLED=1 go build -ldflags="-s -w -X github.com/lima-vm/lima/pkg/version.Version=v0.14.1" -tags "no_vz" -o _output/bin/limactl ./cmd/limactl
> codesign --entitlements vz.entitlements -s - ./_output/bin/limactl
> libc++abi: terminating with uncaught exception of type std::runtime_error: file is already signed. pass -f to sign regardless.
> make: *** [Makefile:182: codesign] Abort trap: 6

I have a little feeling that it may be because the binary was already built and not cleaned up or something like that. I added a rm -rf _output and will try again.

@jbgosselin
Copy link
Contributor Author

Actually what happens is that go binary built on aarch64-darwin requires to be signed. I assume that it signs them by default. But we do want to sign them with vz.entitlements so I modified the Makefile to --force the signature.

@jbgosselin jbgosselin requested review from SuperSandro2000 and removed request for voanhduy1512 December 18, 2022 03:49
@pecigonzalo pecigonzalo mentioned this pull request Dec 29, 2022
13 tasks
@jbgosselin jbgosselin changed the title lima: 0.13.0 -> 0.14.1 lima: 0.13.0 -> 0.14.2 Dec 29, 2022
@ofborg ofborg bot requested a review from voanhduy1512 December 29, 2022 19:23
Copy link
Member

@tricktron tricktron left a comment

Choose a reason for hiding this comment

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

Builds and works on my aarch64-darwin machine.

dhess added a commit to hackworthltd/hacknix that referenced this pull request Jan 1, 2023
Until fixed upstream.

Source:

NixOS/nixpkgs#206285
@dhess
Copy link
Contributor

dhess commented Jan 1, 2023

Thanks for this.

vz appears not to be supported:

limactl start template://experimental/vz                                                                                                                                             ~
? Creating an instance "vz" Proceed with the current configuration
FATA[0005] vm driver 'vz' needs macOS 13 or later (Hint: try recompiling Lima if you are seeing this error on macOS 13) 

I assume this is because nixpkgs doesn't use the macOS 13 SDK? All of the required entitlements are present on the executable, and I'm running (and building on) macOS 13.1, but I see that lima's Makefile disables it when the SDK is too old:

https://github.com/lima-vm/lima/blob/baca0e7ecd1b3515f471d2eb60fdbb0864e9a487/Makefile#L19

@jbgosselin
Copy link
Contributor Author

Thanks for this.

vz appears not to be supported:

limactl start template://experimental/vz                                                                                                                                             ~
? Creating an instance "vz" Proceed with the current configuration
FATA[0005] vm driver 'vz' needs macOS 13 or later (Hint: try recompiling Lima if you are seeing this error on macOS 13) 

I assume this is because nixpkgs doesn't use the macOS 13 SDK? All of the required entitlements are present on the executable, and I'm running (and building on) macOS 13.1, but I see that lima's Makefile disables it when the SDK is too old:

https://github.com/lima-vm/lima/blob/baca0e7ecd1b3515f471d2eb60fdbb0864e9a487/Makefile#L19

Yeah you're right, looks like xcrun is only running at 10.12 whatever the version of macos installed. I don't know how to tell nix to use a more up to date version of the SDK if available on the host machine.

@jonathanlking
Copy link
Contributor

I'm not that familiar with nix, and especially not with darwin/macOS, but I wanted to share what I tried just in case it's helpful.

I was naively hoping that an overlay like:

(final: prev: {
    lima = prev.lima.override (old: { xcbuild = old.xcbuild.override(_: { sdkVer = "13.0"; }); });
  })

would get things building, but unfortunately it errors with:

building
mkdir -p _output/bin
cp -a ./cmd/lima _output/bin/lima
# The hostagent must be compiled with CGO_ENABLED=1 so that net.LookupIP() in the DNS server
# calls the native resolver library and not the simplistic version in the Go library.
CGO_ENABLED=1 go build -ldflags="-s -w -X github.com/lima-vm/lima/pkg/version.Version=v0.14.2" -tags "" -o _output/bin/limactl ./cmd/limactl
# github.com/Code-Hex/vz/v3
In file included from vendor/github.com/Code-Hex/vz/v3/audio.go:6:
In file included from ./virtualization_11.h:9:
./virtualization_helper.h:25:9: warning: macOS 12.3 API has been disabled [-W#pragma-messages]
#pragma message("macOS 12.3 API has been disabled")
        ^
./virtualization_helper.h:32:9: warning: macOS 13 API has been disabled [-W#pragma-messages]
#pragma message("macOS 13 API has been disabled")
        ^
In file included from vendor/github.com/Code-Hex/vz/v3/audio.go:6:
./virtualization_11.h:10:9: fatal error: 'Virtualization/Virtualization.h' file not found
#import <Virtualization/Virtualization.h>
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings and 1 error generated.
make: *** [Makefile:94: _output/bin/limactl] Error 2

I think lima is pulling in vz, which the lima derivation is building too?

virtualization_helper.h checks the value of __MAC_OS_X_VERSION_MAX_ALLOWED, so this might need to be set/overridden too?

@jbgosselin
Copy link
Contributor Author

Thanks for the help @jonathanlking will look into the way you did if I can make it work.

I'm also a bit concerned how to implement this to be available for users using nix on machine prior to 13.0 as well as after.

@dhess
Copy link
Contributor

dhess commented Jan 4, 2023

lima provides binaries, and this may be a case where it makes sense to use them for macOS. nixpkgs could provide a lima-binary package so that the user has a choice about which one to use.

@jbgosselin
Copy link
Contributor Author

lima provides binaries, and this may be a case where it makes sense to use them for macOS. nixpkgs could provide a lima-binary package so that the user has a choice about which one to use.

Could be an idea also.
I'm honestly more concern over having it breaks on users using a version of macos prior to 13.0.

@roblabla
Copy link
Contributor

roblabla commented Jan 4, 2023

Using 13.x SDK should work even on older version of macos. It only makes new APIs available, but if the program doesn't call them (or only calls them after checking the host os version), it shouldn't cause issues. Lima uses vz to access the Virtualization API, and it has this kind of doc: https://github.com/Code-Hex/vz/blob/957dfd2add9a0badf25dee8cd1dc01a79e8fee06/bootloader.go#L137

// NewEFIBootLoader creates a new EFI boot loader.
//
// This is only supported on macOS 13 and newer, error will
// be returned on older versions.

Of course, vz-based VMs won't work on macos12 and before, but the qemu-based driver should still work without issues.

@jbgosselin
Copy link
Contributor Author

@roblabla great insight, thanks !

@jbgosselin
Copy link
Contributor Author

#206285 (comment)
@jonathanlking did you try this on a machine running macos Ventura ?

@jonathanlking
Copy link
Contributor

jonathanlking commented Jan 5, 2023

Hey, I did a tiny bit more exploring earlier today (again, please take what I'm saying with extreme caution!).

Something concerning me was the fatal error: 'Virtualization/Virtualization.h' file not found error.

I think prior to lima-vm/lima@2679326 there was no dependency on macOS frameworks? (I'm not super confident about this claim). I think you might need to use a different callPackage to make them available (if this isn't already happening), I got that impression from reading:

To use it, reference apple_sdk_11_0 instead of apple_sdk in your derivation and use pkgs.darwin.apple_sdk_11_0.callPackage instead of pkgs.callPackage. On Linux, this will have the same effect as pkgs.callPackage, so you can use pkgs.darwin.apple_sdk_11_0.callPackage regardless of platform.

on #176661.

Anyway, there is certainly no Virtualization in the pkgs.darwin.apple_sdk_11_0, which makes sense as the framework wasn't yet introduced, so I think it's something we'd need to patch/introduce?
I think the way it works is the nix derivations just refer to/copy the libraries on your mac computer.

FYI I found a copy of Virtualization.h under /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/Virtualization.framework/Versions/A/Headers/Virtualization.h.

My feeling at the moment is that a lima-binary package might be considerably easier 😅

@jonathanlking
Copy link
Contributor

#206285 (comment) @jonathanlking did you try this on a machine running macos Ventura ?

@dennajort Yes, this is on macOS 13.0.1 running on an M1

@dhess
Copy link
Contributor

dhess commented Jan 5, 2023

To use it, reference apple_sdk_11_0 instead of apple_sdk in your derivation and use pkgs.darwin.apple_sdk_11_0.callPackage instead of pkgs.callPackage. On Linux, this will have the same effect as pkgs.callPackage, so you can use pkgs.darwin.apple_sdk_11_0.callPackage regardless of platform.

I tried this as well, but it doesn't help, as the vz feature requires macOS SDK 13.0, but the most recent nixpkgs has is 11.0 (via the apple_sdk_11_0 stdenv).

My feeling at the moment is that a lima-binary package might be considerably easier 😅

That's what I concluded after trying the apple_sdk_11_0 option. Even if someone started adding support for macOS SDK 13.0 today, it would likely be months before it was ready, if past experience is any guide.

@tricktron
Copy link
Member

@jonathanlking @dhess
I tried going down that path with the following changes:

diff --git a/pkgs/applications/virtualization/lima/default.nix b/pkgs/applications/virtualization/lima/default.nix
index 29dd9b27232..a282e4c30de 100644
--- a/pkgs/applications/virtualization/lima/default.nix
+++ b/pkgs/applications/virtualization/lima/default.nix
@@ -7,6 +7,8 @@
 , xcbuild
 , sigtool
 , makeWrapper
+, Virtualization
+, Cocoa
 }:
 
 buildGoModule rec {
@@ -22,6 +24,8 @@ buildGoModule rec {
 
   vendorSha256 = "sha256-l53MTxLY/uid+0U/eY96l0aBWKImST1gN2BARilh2K0=";
 
+  buildInputs = lib.optionals stdenv.isDarwin [ Virtualization Cocoa ];
+
   nativeBuildInputs = [ makeWrapper installShellFiles ]
     ++ lib.optionals stdenv.isDarwin [ xcbuild.xcrun sigtool ];
 
@@ -29,7 +33,8 @@ buildGoModule rec {
   postPatch = ''
     substituteInPlace Makefile \
       --replace 'binaries: clean' 'binaries:' \
-      --replace 'codesign --entitlements vz.entitlements -s -' 'codesign --force --entitlements vz.entitlements -s -'
+      --replace 'codesign --entitlements vz.entitlements -s -' 'codesign --force --entitlements vz.entitlements -s -' \
+      --replace 'MACOS_SDK_VERSION=$(shell xcrun --show-sdk-version | cut -d . -f 1)' 'MACOS_SDK_VERSION=13'
   '';
 
   # It attaches entitlements with codesign and strip removes those,
diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix
index 8fa2336e176..601d86d8f43 100644
--- a/pkgs/top-level/all-packages.nix
+++ b/pkgs/top-level/all-packages.nix
@@ -36777,8 +36777,9 @@ with pkgs;
 
   colima = callPackage ../applications/virtualization/colima { };
 
-  lima = callPackage ../applications/virtualization/lima {
+  lima = darwin.apple_sdk_11_0.callPackage ../applications/virtualization/lima {
     inherit (darwin) sigtool;
+    inherit (darwin.apple_sdk_11_0.frameworks) Virtualization Cocoa;
   };
 
   logtop = callPackage ../tools/misc/logtop { };

This then leads to the next errors:

vendor/github.com/Code-Hex/vz/v3/virtualization_helper.h:25:9: warning: macOS 12.3 API has been disabled [-W#pragma-messages]
vendor/github.com/Code-Hex/vz/v3/virtualization_helper.h:32:9: warning: macOS 13 API has been disabled [-W#pragma-messages]
virtualization_12.m:14:52: error: property 'canStop' not found on object of type 'VZVirtualMachine *'
virtualization_12.m:27:42: warning: instance method '-stopWithCompletionHandler:' not found (return type defaults to 'id') [-Wobjc-method-access]
/nix/store/lv5xsqihygkhyb9djf840nhlymi7j77m-apple-framework-Virtualization-11.0.0/Library/Frameworks/Virtualization.framework/Headers/VZVirtualMachine.h:57:12: note: receiver is instance of class declared here
virtualization_12.m:42:18: error: unknown receiver 'VZGenericPlatformConfiguration'; did you mean 'VZSerialPortConfiguration'?
/nix/store/lv5xsqihygkhyb9djf840nhlymi7j77m-apple-framework-Virtualization-11.0.0/Library/Frameworks/Virtualization.framework/Headers/VZVirtualMachineConfiguration.h:16:8: note: 'VZSerialPortConfiguration' declared here
virtualization_12.m:42:56: error: 'init' is unavailable
/nix/store/lv5xsqihygkhyb9djf840nhlymi7j77m-apple-framework-Virtualization-11.0.0/Library/Frameworks/Virtualization.framework/Headers/VZSerialPortConfiguration.h:26:1: note: 'init' has been explicitly marked unavailable here
virtualization_12.m:55:50: warning: instance method '-setDirectorySharingDevices:' not found (return type defaults to 'id') [-Wobjc-method-access]
/nix/store/lv5xsqihygkhyb9djf840nhlymi7j77m-apple-framework-Virtualization-11.0.0/Library/Frameworks/Virtualization.framework/Headers/VZVirtualMachineConfiguration.h:46:12: note: receiver is instance of class declared here
virtualization_12.m:70:88: error: expected expression
virtualization_12.m:70:63: error: use of undeclared identifier 'VZPlatformConfiguration'

According to this pr it should be possible to use vz on sdk 11.0, however the guarding in https://github.com/Code-Hex/vz/blob/957dfd2add9a0badf25dee8cd1dc01a79e8fee06/virtualization_12.m#L9-L20
is not working properly.

Even if get all that to work, it seems fragile to me because the root problem is that we are two major sdk versions behind. So I guess a binary version of lima is indeed the better approach for those who want the bleeding edge experimental vz emulation.

@tricktron
Copy link
Member

I opened a pr for the lima binary derivation #209171.

@jbgosselin
Copy link
Contributor Author

Thanks for your help on that !
My idea is to not try to fiddle too much with the apple sdk at the moment. As is, if the new apple sdks are added to nix, it may work automatically if I understand ?

@tricktron
Copy link
Member

My idea is to not try to fiddle too much with the apple sdk at the moment. As is, if the new apple sdks are added to nix, it may work automatically if I understand?

Yes. Bumping the apple sdk is hard work, however (see the monthly macOS roundup: https://discourse.nixos.org/t/nix-macos-monthly/12330), so don't expect that to happen anytime soon.

@farcaller
Copy link
Contributor

Result of nixpkgs-review pr 206285 run on aarch64-darwin 1

2 packages built:
  • colima
  • lima

@farcaller
Copy link
Contributor

lima uname -a works just fine 👍

@tricktron
Copy link
Member

@SuperSandro2000, I think this is ready to merge.

@wegank
Copy link
Member

wegank commented Jan 23, 2023

@ofborg build lima colima

@wegank wegank merged commit 0797f5f into NixOS:master Jan 23, 2023
@jbgosselin jbgosselin deleted the lima-0.14.1 branch January 23, 2023 16:43
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants