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

macOS 11 has been announced; can_use_xz() should be updated #623

Closed
DeeDeeG opened this issue Jun 22, 2020 · 9 comments · Fixed by #624
Closed

macOS 11 has been announced; can_use_xz() should be updated #623

DeeDeeG opened this issue Jun 22, 2020 · 9 comments · Fixed by #624

Comments

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jun 22, 2020

Hi,

Apple has announced a macOS 11 release for some time in the future.

For inferring xz support, we currently look for the minor macOS version number (>= 9), but I reckon we should now also accept major version >= 11.

@shadowspawn
Copy link
Collaborator

Um, yes, we didn't check the major version! Has not changed in soooooo long.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jun 22, 2020

By the way, nvs does something clever for version comparisons. For comparing major.minor.patch versions, it converts each component to having three digits (padded with leading zeroes) and concatenates just the digits (no more dots). For example, 1.22.333 becomes 001022333. With versions represented this way, you can just use normal integer comparison on them.

(This amount of padding would be enough up until any of the version components requires more than three digits to write. I find it unlikely that macOS will ever release a version with one of the components being 1000 or larger, so it should be good enough.)

  • This "clever" way would work by checking for macOS >= 10.9.0 (>= "010009000")
  • or else we can do two separate checks (macOS major version >= 11, or minor version >= 9).

@shadowspawn
Copy link
Collaborator

I would prefer two separate checks for this. The unified version approach is handy if doing sorting or multiple checks, but for a single check, no need to be clever.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jun 23, 2020

I also just remembered that in macOS releases with xz support there is a shared/dynamic library /usr/lib/liblzma.dylib (which is actually a symlink to /usr/lib/liblzma.5.dylib).

Checking for the existence of this file should work for macOS, irrespective of OS version.

So the options are down to:

  • macOS major version check, followed by a macOS minor version check.
  • Check for the existence of /usr/lib/liblzma.dylib on macOS.

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jun 23, 2020

A PR is up with the simple fix/staying close to the existing solution. #624

@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jun 24, 2020

I can do a version of this at nvh too.

(It will take until there is a beta of macOS 11 to really confirm this is the right approach, but I'd be surprised if it isn't. And I think it's worth being ahead of the curve, so it "just works" for more users right on day one if macOS 11 usage. And to preempt any issue spam.)

@shadowspawn
Copy link
Collaborator

I agree, in that ideally I would try it on macOS 11 first, but I am happy shipping sooner as the change is consistent with the existing implementation and likely to Just Work.

PR for nvh welcome, or I'll get to it myself eventually. :-)

@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Jun 24, 2020
@DeeDeeG
Copy link
Contributor Author

DeeDeeG commented Jun 30, 2020

FWIW there is a developer beta for macOS 11 "Big Sur", but I don't own a Mac, I only borrow them from time to time... And I'm not going to pay $100 to join the Apple Developer Program.

https://www.digitaltrends.com/computing/how-to-download-macos-big-sur/

@shadowspawn shadowspawn removed the pending release Merged into a branch for a future release, but not released yet label Jul 4, 2020
@shadowspawn
Copy link
Collaborator

Shipped in n v6.6.0

Thanks @DeeDeeG

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

Successfully merging a pull request may close this issue.

2 participants