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

cd: set MACOSX_DEPLOYMENT_TARGET to 10.7 #468

Merged
merged 1 commit into from
Dec 26, 2020

Conversation

phil-blain
Copy link
Contributor

The binary release of delta 0.4.5 for macOS created by the "Continuous
Deployment" GitHub Actions workflow does not run on older macOS, like
10.11 (El Capitan) [1].

The Rust compiler, rustc, by default, will build macOS binaries
compatible with macOS 10.7 or newer [2], [3], but some *-sys crates invoke a
C compiler and thus will target the macOS version the C compiler is
running on [4], thus limiting the compatibility of the resulting
binary. The macOS virtual machines used on GitHub Actions run macOS
10.15.7, and since delta depends on some *-sys crates, the delta binary
built by cargo targets macOS 10.15, as can be verified by using the
following command:

$ otool -l ./target/x86_64-apple-darwin/release/delta | grep LC_BUILD_VERSION -A4
cmd LC_BUILD_VERSION
cmdsize 32
platform 1
sdk 10.15.6
minos 10.15

or, if building locally on an older macOS versions, (here 10.11):

$ otool -l ./target/debug/delta | grep  LC_VERSION_MIN_MACOSX -A2
cmd LC_VERSION_MIN_MACOSX
cmdsize 16
version 10.11

To restore compatibility with older macOS version, explicitely set the environment
variable MACOSX_DEPLOYMENT_TARGET to the default for rustc, "10.7".
This will make the clang C compiler invoked by *-sys crates also
target 10.7, thus restoring the compatibility of the delta binary.

For simplicity, add this variable to the environment for all platforms,
which should not have any effect on non-macOS platforms.

[1] #462
[2] https://github.com/rust-lang/rust/blob/65d053ab74d8c8c9c502b678acc265f3d7e2ac49/compiler/rustc_target/src/spec/apple_base.rs#L15-L17
[3] https://github.com/rust-lang/rust/blob/65d053ab74d8c8c9c502b678acc265f3d7e2ac49/compiler/rustc_target/src/spec/apple_base.rs#L53-L61
[4] https://users.rust-lang.org/t/compile-rust-binary-for-older-versions-of-mac-osx/38695/5

The binary release of delta 0.4.5 for macOS created by the "Continuous
Deployment" GitHub Actions workflow does not run on older macOS, like
10.11 (El Capitan) [1].

The Rust compiler, rustc, by default, will build macOS binaries
compatible with macOS 10.7 or newer [2], [3], but some *-sys crates invoke a
C compiler and thus will target the macOS version the C compiler is
running on [4], thus limiting the compatibility of the resulting
binary. The macOS virtual machines used on GitHub Actions run macOS
10.15.7, and since delta depends on some *-sys crates, the delta binary
built by cargo targets macOS 10.15, as can be verified by using the
following command:

    $ otool -l ./target/x86_64-apple-darwin/release/delta | grep LC_BUILD_VERSION -A4
    cmd LC_BUILD_VERSION
    cmdsize 32
    platform 1
    sdk 10.15.6
    minos 10.15

or, if building locally on an older macOS versions, (here 10.11):

    $ otool -l ./target/debug/delta | grep  LC_VERSION_MIN_MACOSX -A2
    cmd LC_VERSION_MIN_MACOSX
    cmdsize 16
    version 10.11

To restore compatibility with older macOS version, explicitely set the environment
variable `MACOSX_DEPLOYMENT_TARGET` to the default for rustc, "10.7".
This will make the `clang` C compiler invoked by *-sys crates also
target 10.7, thus restoring the compatibility of the delta binary.

For simplicity, add this variable to the environment for all platforms,
which should not have any effect on non-macOS platforms.

[1] dandavison#462
[2] https://github.com/rust-lang/rust/blob/65d053ab74d8c8c9c502b678acc265f3d7e2ac49/compiler/rustc_target/src/spec/apple_base.rs#L15-L17
[3] https://github.com/rust-lang/rust/blob/65d053ab74d8c8c9c502b678acc265f3d7e2ac49/compiler/rustc_target/src/spec/apple_base.rs#L53-L61
[4] https://users.rust-lang.org/t/compile-rust-binary-for-older-versions-of-mac-osx/38695/5
@marcoieni
Copy link
Contributor

Thank you for the detailed explanation!
I don't have ways to test this, but it looks good.

Just one question, do you know if we can support older MACOSX versions, like 10.5 or 10.6 for example? What's the minimum?

@phil-blain
Copy link
Contributor Author

From what I was able to understand while researching this fix, as of now Rust itself is compatible with 10.7 and later only, or maybe 10.6 if building Rust from source with some special configure (or similar) options (I'm not sure):

So I think that 10.7 should be enough, being the minimum version supported by Rust.

@marcoieni
Copy link
Contributor

Sure, thank you! It was also written in link [2].

For me this can be merged.

@phil-blain
Copy link
Contributor Author

Also, I did not modify the README to revert 2bbf189, but it might be good to do so before the next release... I can submit a subsequent PR once this one is merged.

@dandavison dandavison force-pushed the macosx-deployment-target branch from 82054c4 to 5b1bada Compare December 26, 2020 19:23
@dandavison
Copy link
Owner

Thanks very much @phil-blain -- for researching the proper solution, implementing, and providing detailed explanations.

The two Rust command line projects I tend to check against are bat and ripgrep; I should have thought to look at those when we were first discussing this. Looks like ripgrep may be coming to the same solution as here in BurntSushi/ripgrep#1737 (there is some discussion there that may be relevant). However, on a quick search I didn't see any issues regarding this in bat, nor is it obviously addressed in https://github.com/sharkdp/bat/blob/master/.github/workflows/CICD.yml.

I'll aim to get a fixup release out soon (that will be 0.5.0).

@dandavison dandavison merged commit 36d8eae into dandavison:master Dec 26, 2020
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.

3 participants