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

Support building in older version of macOS #41

Closed
balajiv113 opened this issue Sep 7, 2022 · 34 comments
Closed

Support building in older version of macOS #41

balajiv113 opened this issue Sep 7, 2022 · 34 comments

Comments

@balajiv113
Copy link
Contributor

Thanks a lot for this awesome library.
For better integration with existing products, it would be great if we could support building in older version.

For Instance,
macOS 10.x - Building will successful & execution will throw error saying virtualization not supported
macOS 12.x - Building will successful & execution will be successful as well

With this support, it will be easier to integrate to existing tools that still supports macOS 10.x

@cfergeau
Copy link
Contributor

cfergeau commented Sep 7, 2022

I've been trying to do something similar, but limited to macOS 11/12 and the file sharing API, see
cfergeau@91019b6
cfergeau@9aad23f

I'm refining this a little at the moment, and was going to start a similar discussion very soon!

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 7, 2022

@balajiv113 Thanks for creating a new issue!

I think you said macOS 11.x not 10.x. right?

I supported both versions with go module versioning systems.

For 11.x, You can get go get github.com/Code-Hex/vz. For 12.x use go get github.com/Code-Hex/vz/v2

Please read README 🙏

@cfergeau
Copy link
Contributor

cfergeau commented Sep 7, 2022

I supported both versions with go module versioning systems.

For 11.x, You can get go get github.com/Code-Hex/vz. For 12.x use go get github.com/Code-Hex/vz/v2

Maybe I missed something when I looked at this schemed, but I don't think this is enough. I want to ship a prebuilt binary which will run on macOS 11 or newer, but I also want my binary to use the file sharing API when it's running on macOS 12. On macOS 11, I want my binary to print an error if the user tries to use file sharing.

My understanding is that I cannot achieve this with Code-Hex/vz at the moment? I can build a binary which works on macOS11, but which won't have filesharing, or I can build a binary which will only work on macOS12 but with the filesharing API?
Instead of making the choice of file sharing/not file sharing at compile time, I want to be able to do it dynamically at runtime.

@balajiv113
Copy link
Contributor Author

@Code-Hex
No no, am more interested in 10.x version.

I do know the macOS Virtualization is not supported in 10.x. Am looking for an option to compile in 10.x and throw error in runtime. This way, while using this library i don't need to worry about checking the base minimum version.

Also, now with new API's being release for macOS 13, we might even have /v3 released. This modal surely works for tools that update to latest version as soon as OS is released. But it won't work when we need to support multiple OS versions i believe.

Please do correct me if am wrong :)

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 7, 2022

@cfergeau @balajiv113 Thanks!

These are my thoughts.

  • C and Objective-C can detect macOS versions in compile-time. But Go cannot.
  • Go users do not know which version of each method is available as it is not explicit.
    • C and Objective-C are explicit as they use @available macros to control these in the library usage phase.

We can detect os version like below. This method was used in Go official repository.

package main

import (
	"fmt"
	"syscall"
)

func main() {
	osver, err := syscall.Sysctl("kern.osrelease")
	if err != nil {
		return
	}
	fmt.Println(osver, err)
}

Indeed, as @cfergeau points out, the @available macro needs to be used in v2. I think this is correct and I should add them. However, If I understand correctly, you should have to specify a build target for this macro to take effect. I'm not sure how this should be done in Go, so I'm looking into it now.

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 7, 2022

Maybe this?
golang/go#18400 (comment)

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 7, 2022

Do you have any suggestions?
If you want to make all versions compilable, I might modify following

  1. Provide a function like vz.IsSupport() bool (You can control if the OS version is less than 11.x)
  2. If a method is called that is not supported by the version, it will be panicked.
  3. Similarly, guest use of macOS is only supported by Apple Silicon, which also panics them if called in amd64 arch. ( But I think this is able to control with build tag)

@cfergeau
Copy link
Contributor

cfergeau commented Sep 7, 2022

I'm not sure panic() is a good outcome, I'd prefer an error to be returned. Otherwise the client application needs to have the knowledge of which API is supported with which macOS version.
Or maybe have multiple IsSupported helpers? IsFileSharingSupported, IsGraphicsSupported, and so on, and panic() if trying to use FileSharing` API when it's not supported?

@cfergeau
Copy link
Contributor

cfergeau commented Sep 7, 2022

Maybe this? golang/go#18400 (comment)

I've started trying to make use of this flag in my branch ( https://github.com/cfergeau/vz/tree/shared-folders2 ), otherwise it's indeed not very convenient. A binary built on macOS 11 with @available(macOS 12, *) will work on both macOS 11 and macOS 12, but a binary built on macOS12 with @available(macOS 12, *) will only work on macOS 12.

@cfergeau
Copy link
Contributor

cfergeau commented Sep 7, 2022

func main() {
	osver, err := syscall.Sysctl("kern.osrelease")
	if err != nil {
		return
	}
	fmt.Println(osver, err)
}

For OS version detection, I'm using a different method, which has the big advantage of also using @available

+
+/*!
+ @abstract Indicates if the macOS version being used supports file sharing. macOS 12+ is required.
+ @return True if file sharing is supported
+ */
+bool virtioFileSystemDeviceSupported()
+{
+    if (@available(macOS 12, *)) {
+       return (bool)true;
+    } else {
+       return (bool)false;
+    }
+}
+

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 7, 2022

It looks like a way of returning an error would also be good.

  • Define vz.ErrDoesNotSupportOSVersion variable
  • The user should check with errors.Is(err, vz.ErrDoesNotSupportOSVersion)
  • However, this method requires functions that do not originally have to return an error to also return an error.

I think this method does not have an advantage. Because We have to use cgo always to check the OS version. It does not seem good practice to have a C function for every such trivial change. (It takes a long time to compile.)

#41 (comment)

@cfergeau
Copy link
Contributor

cfergeau commented Sep 7, 2022

It looks like a way of returning an error would also be good.

* Define `vz.ErrDoesNotSupportOSVersion` variable

* The user should check with `errors.Is(err, vz.ErrDoesNotSupportOSVersion)`

* However, this method requires functions that do not originally have to return an error to also return an error.

Yes, this is similar to what I introduced in cfergeau@a9717d7 , though my changes are more limited in scope since at that time there was only the file sharing API. I did not add an error to all the filesharing API since some of the methods cannot be used without a vz.SharedDirectory or vz.VirtioFileSystemDeviceConfiguration so it's enough if their New method return an error.

I think this method does not have an advantage. Because We have to use cgo always to check the OS version. It does not seem good practice to have a C function for every such trivial change. (It takes a long time to compile.)

But if we have this code in virtualization.m, it's only going to be compiled when virtualization.m changes, it's not going to be re-compiled when go code changes? I did not understand what is problematic? :(

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 7, 2022

I want to say, not only in the New-related APIs, but also, for example, in the case of Stop; the same applies to CanStop. Signatures after changes are annoying maybe.

- func (v *VirtualMachine) CanStop() bool
- func (v *VirtualMachine) Stop(fn func(error))
+ func (v *VirtualMachine) CanStop() (bool, error)
+ func (v *VirtualMachine) Stop(fn func(error)) error

The library development side. I have no complaints if anyone contributes the test code :D

I did not understand what is problematic? :(

@cfergeau
Copy link
Contributor

cfergeau commented Sep 7, 2022

I have no complaints if anyone contributes the test code :D

I'll work on this in the next days, and file a PR so that we can refine this!

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 7, 2022

How about cross-compiling distributions on the latest version of the macOS and each method determine if it is available at runtime?

Even if all versions could be compiled, this problem would still occur. The case of concern

  1. Compiled on macOS 11.x (expected compilable and can use methods for 11.x)
  2. Upgrade to the 12.x

In this case, I think the user cannot use any methods for 12.x. This is because some methods that are only available in 11.x version are compiled into the binary.

Maybe the user have to install again for using the latest features.

@cfergeau
Copy link
Contributor

cfergeau commented Sep 8, 2022

I'll test again later today, but compiling on macOS 11 and then being able to use the file sharing API when running on macOS 12 works fine. This is how https://github.com/code-ready/vfkit/releases/tag/v0.0.2 was built (on macOS 11 even if there is file sharing) because I hadn't searched yet for the -mmacosx-version-min=11.0 flag.
Without this flag, a binary built on macOS 12 does not start at all on macOS 11.

@cfergeau
Copy link
Contributor

cfergeau commented Sep 8, 2022

We can detect os version like below. This method was used in Go official repository.

package main

import (
	"fmt"
	"syscall"
)

func main() {
	osver, err := syscall.Sysctl("kern.osrelease")
	if err != nil {
		return
	}
	fmt.Println(osver, err)
}

I've been looking again at this suggestion, as doing the version detection with @available looks a bit long/silly when you want to detect macOS 10/11/12/13/...

sysctl kern.osrelease is some darwin/kernel version, it returns 21.6.0 on my macbook. sysctl kern.osproductversion returns what we want (12.5.1 on my test laptop)

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 8, 2022

@cfergeau Thanks for checked!
But I think you uploaded 2 times. First is macOS 11 and second is macOS 12. (In this time overwritten)

This is your configuration.

https://github.com/code-ready/vfkit/blob/e11cc7cbaf483ca7ab2b1a2f2af7c11584433721/.github/workflows/compile.yml#L15

And I checked your actions job history :)

@cfergeau
Copy link
Contributor

cfergeau commented Sep 8, 2022

The action creates 2 binaries https://github.com/code-ready/vfkit/actions/runs/2753739630 with the macos version as a suffix.
Then I picked the macOS11 one which I manually added to the release https://github.com/code-ready/vfkit/releases/tag/v0.0.2
I just checked the sha256sum of all these binaries, and it is the macOS 11 one which is at https://github.com/code-ready/vfkit/releases/tag/v0.0.2

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 8, 2022

I also confirmed the build and execution results in https://github.com/Code-Hex/mac-build-test. (Build history)

@cfergeau's understanding seemed to be correct. However, looking at the logs during the build on GitHub actions, it appears that whether or not it can be compiled depends on the version of xcode installed on the mac.

e.g.

# github.com/Code-Hex/mac-build-test
lib.m:2[5](https://github.com/Code-Hex/mac-build-test/runs/8250342305?check_suite_focus=true#step:5:6):5: warning: 'VZSharedDirectory' is only available on macOS 12.0 or newer [-Wunguarded-availability-new]
/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/Virtualization.framework/Headers/VZSharedDirectory.h:1[6](https://github.com/Code-Hex/mac-build-test/runs/8250342305?check_suite_focus=true#step:5:7):12: note: 'VZSharedDirectory' has been marked as being introduced in macOS 12.0 here, but the deployment target is macOS 11.0.0
lib.m:25:5: note: enclose 'VZSharedDirectory' in an @available check to silence this warning
lib.m:25:32: warning: 'VZSharedDirectory' is only available on macOS 12.0 or newer [-Wunguarded-availability-new]
/Applications/Xcode_13.2.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks/Virtualization.framework/Headers/VZSharedDirectory.h:16:12: note: 'VZSharedDirectory' has been marked as being introduced in macOS 12.0 here, but the deployment target is macOS 11.0.0
lib.m:25:32: note: enclose 'VZSharedDirectory' in an @available check to silence this warning

Therefore I attempted to compile an API that will be added in macOS 13 called VZSpiceAgentPortAttachment. However, this is not supported by xcode at the moment, so the compilation failed even with @available. (commit)

https://github.com/Code-Hex/mac-build-test/runs/8250778147?check_suite_focus=true

In conclusion, whether or not you can compile depends on the xcode installed on the machine you are using.

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 8, 2022

From the above, these are my thoughts

  • Distributions are compiled on the latest version macOS, e.g. using Circle CI, and GitHub actions.
  • @available can be checked dynamically at runtime. Therefore, both v2 and v3 should use this directive for the latest API in this package.
    • However, It should not be expected that v2 and v3 will compile on older macOS versions

@cfergeau
Copy link
Contributor

cfergeau commented Sep 8, 2022

If -mmacosx-version-min=11 (or maybe even -mmacosx-version-min=10) works as expected when used on latest version of macOS (and I think it will be fine), then yes, this is also what I have in mind ^^
Started adding the flag/the annotations in https://github.com/cfergeau/vz/tree/macos11 , now adding "ErrOsUnsupported" error returns from go functions where needed.

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 8, 2022

The compilation result depends on xcode, I think this is not possible, as commented at the beginning of this issue.

macOS 10.x - Building will successful & execution will throw error saying virtualization not supported

At least the latest API is likely to require the latest xcode installed which is supported by the API.

If you have a macOS that is one major version earlier, you might be able to install xcode that meets this requirement :D

@cfergeau
Copy link
Contributor

cfergeau commented Sep 8, 2022

If you have a macOS that is one major version earlier, you might be able to install xcode that meets this requirement :D

At the moment, the xcode I have on my macOS 11 x86_64 macbook can still build vz code with file sharing. I'll try it with my macos11 branch to see how it goes.

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 8, 2022

I think it is possible because the Directory Sharing API is provided in macOS v12. (v11 is one major version earlier than v12)

For example, older versions than v10 would not compile.

Good ref: https://xcodereleases.com/ (Please pay attention Requires and SDKs)

@cfergeau
Copy link
Contributor

cfergeau commented Sep 8, 2022

When this is added to vz, we'll see if I can compile for the macOS 13 API on this macOS 11 laptop :)

@Code-Hex
Copy link
Owner

Code-Hex commented Sep 8, 2022

To add a bit more info, Xcode 12.3 is the latest version that can be installed on macOS v10. The SDK supported by this xcode is up to macOS 11.1. (from xcodereleases.com)

I need to understand xcode release cycles...

@cfergeau
Copy link
Contributor

cfergeau commented Sep 8, 2022

I have errors added in most places in https://github.com/cfergeau/vz/tree/macos11

I'm not sure how to add annotations/report errors for VZVirtualMachineView, need to look at this closer.

Same for Stop, ideally the handler passed as an arg would be invoked from go code and get a errOSNotSupported error, but I'm not sure how to do that :(

Apart from that, I need to cleanup a bit the git history, group some of the commits, and test it, and it should be good for a PR!

@cfergeau
Copy link
Contributor

@balajiv113 #48 should return proper errors when trying to use virtualization framework on macOS 10, but I could not test it. Can you give it a try if you have the appropriate setup?

@balajiv113
Copy link
Contributor Author

@cfergeau Sure will give a shot and let you know the results

@balajiv113
Copy link
Contributor Author

@cfergeau I tried compiling In macOS 10.15.7. But unfortunately ended up with the following error

In file included from ../../audio.go:6:
./virtualization.h:10:9: fatal error: 'Virtualization/Virtualization.h' file not found
#import <Virtualization/Virtualization.h>
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

@cfergeau
Copy link
Contributor

Ah yes, this enables running a binary using Code-Hex/vz on older macOS versions, but it has to be built on a newer macOS with CGO_CFLAGS=-mmacosx-version-min=10.13

@Code-Hex
Copy link
Owner

Code-Hex commented Oct 3, 2022

@balajiv113 I said, virtualization framework cannot compile under v10 because it depends on xcode sdk.
#41 (comment)

But I think if you have installed xcode 12.4, You can build on your version because xcode sdk for v11.1 is supported.

@Code-Hex Code-Hex mentioned this issue Oct 8, 2022
5 tasks
@Code-Hex
Copy link
Owner

I think this task has been done by #48
Close this issue. thanks @cfergeau

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants