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

Enable xz by default on macOS 11+ #624

Merged
merged 1 commit into from
Jun 24, 2020

Conversation

DeeDeeG
Copy link
Contributor

@DeeDeeG DeeDeeG commented Jun 23, 2020

Pull Request

Problem

Resolves #623.

macOS 11 has been announced. At the moment, the system check in can_use_xz() only cares about the OS minor version being greater than 8, so as it stands, n would think the upcoming macOS 11.0.0 doesn't support xz decompression via tar. Downloads would fall back to fetching gzip tarballs.

(I think it's reasonable to assume Apple will keep xz support in tar, as opposed to abruptly dropping it.)

Solution

In this PR, the check in can_use_xz() now allows any major macOS version 11+, in addition to allowing any macOS with minor version 8 or greater.

An aside about `sw_vers` and why this loose logic is good enough (Click to expand.)

We check the macOS version with the sw_vers utility. The only Mac operating system versions with the sw_vers utility are versions 10+. On old enough OS X/Mac OS, sw_vers command doesn't exist; As such, I'd expect to see this on stderr:

$ macos_version=$(sw_vers -productVersion)
sw_vers: command not found

As that output is on stderr and nothing is in stdout, $macos_version holds an empty string and fails the version check. In this scenario, I would expect n to fall back to using gzip.

(I simulated that scenario by being on Ubuntu (no sw_vers command here!) and setting macos_version="$(sw_vers -productVersion)" and running the version checks against this $macos_version, which indeed was an empty string and failed the version checks.)

My mental "truth table" of this check, considering all that, is as follows:

OS X/macOS major version 10 macOS major version 11+ Some older Mac OS version <= 9
Minor version 8 or less ❌ No xz (use gzip) ✔️ Use xz ❌ No xz (use gzip)
Minor version 9+ ✔️ Use xz ✔️ Use xz ❌ No xz (use gzip)

This all looks correct to me, and I'm satisfied the logic theoretically works as intended. Typos or bugs notwithstanding.

ChangeLog

Enable xz support by default for macOS 11+

macOS 11 has been announced, and is expected to continue to support xz
@DeeDeeG DeeDeeG changed the base branch from master to develop June 23, 2020 16:54
@DeeDeeG DeeDeeG changed the title Macos 11 xz system check Enable xz by default on macOS 11+ Jun 23, 2020
Copy link
Collaborator

@shadowspawn shadowspawn left a comment

Choose a reason for hiding this comment

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

Looks good, thanks. I haven't run the code yet so will do some due diligence before merging, but no requests or suggestions from reading code.

@shadowspawn shadowspawn merged commit c9475fd into tj:develop Jun 24, 2020
@shadowspawn shadowspawn added the pending release Merged into a branch for a future release, but not released yet label Jun 24, 2020
@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

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 this pull request may close these issues.

macOS 11 has been announced; can_use_xz() should be updated
2 participants