Skip to content

Comments

Support iOS 13 in cross#93596

Merged
matthewbauer merged 7 commits intoNixOS:stagingfrom
obsidiansystems:ios-13
Sep 11, 2020
Merged

Support iOS 13 in cross#93596
matthewbauer merged 7 commits intoNixOS:stagingfrom
obsidiansystems:ios-13

Conversation

@matthewbauer
Copy link
Member

@matthewbauer matthewbauer commented Jul 21, 2020

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@matthewbauer matthewbauer changed the base branch from master to staging July 21, 2020 19:32
@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jul 21, 2020
@matthewbauer matthewbauer force-pushed the ios-13 branch 2 times, most recently from 511f5d3 to 7f4d3f4 Compare July 22, 2020 16:56
Ericson2314
Ericson2314 reviewed Jul 22, 2020
View reviewed changes
pkgs/os-specific/darwin/xcode/sdk-pkgs.nix Outdated
Copy link
Member

@Ericson2314 Ericson2314 Jul 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe we should move these to the binutils override above

Copy link
Member Author

@matthewbauer matthewbauer Jul 23, 2020

Choose a reason for hiding this comment

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

Yeah - that avoids the warning

Copy link
Member Author

@matthewbauer matthewbauer Jul 23, 2020

Choose a reason for hiding this comment

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

I think the c compiler should know about minSdkVersion, so I readded miphoneos-version-min.

Copy link
Member

@Ericson2314 Ericson2314 Jul 23, 2020

Choose a reason for hiding this comment

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

Weird it knows about it, but can't also send it to the linker, ugh.

Copy link
Member Author

@matthewbauer matthewbauer Jul 23, 2020

Choose a reason for hiding this comment

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

It sends the min sdk version to the linker, but not the sdk version. It looks like this might fix that:

https://reviews.llvm.org/D71579

@matthewbauer matthewbauer force-pushed the ios-13 branch 2 times, most recently from dac26f5 to 3c830bc Compare July 23, 2020 03:15
@matthewbauer
xcode/sdk-pkgs.nix: set -platform_version in addition to -miphoneos-v…
95eabdf 
…ersion-min

The App Store looks at LC_VERSION_MIN_IPHONEOS to verify you have a
new enough SDK version. This is not just the minimum version, but also
the sdk version used. When the linker can’t figure it out, it tries to
infer it from the sdk path[1]. When no sdk version is found, it
defaults to just using the -miphoneos-version-min value[2]. So, to make
sure we don’t rely on inference (which doesn’t work in the current
directory structure), we have to specify -platform_version.

[1]:
https://github.com/tpoechtrager/cctools-port/blob/43f32a4c61b5ba7fde011e816136c550b1b3146f/cctools/ld64/src/ld/Options.cpp#L5355-L5376
[2]: https://github.com/tpoechtrager/cctools-port/blob/43f32a4c61b5ba7fde011e816136c550b1b3146f/cctools/ld64/src/ld/ld.hpp#L58
@matthewbauer matthewbauer mentioned this pull request Jul 30, 2020
Merged
13 tasks
@matthewbauer matthewbauer merged commit 9112a9d into NixOS:staging Sep 11, 2020
@thefloweringash
Copy link
Member

thefloweringash commented Sep 19, 2020 •
edited
Loading

The cctools bump seems to have added a warning on linking:

$ nix-shell ~/src/nixpkgs-staging -A hello --run 'clang -v -x c -o /dev/null /dev/null'  2>&1 | grep warning
ld: warning: passed two min versions (10.12.0, 10.12) for platform macOS. Using 10.12.

This warning seems harmless enough, but ruby's configure script treats this as a problem with LDFLAGS.

$ nix-build -A ruby
[...]
checking whether LDFLAGS is valid... no
configure: error: something wrong with LDFLAGS=""
builder for '/nix/store/2b4wc2navpjw3hm4aijyhsqrfc3yzav9-ruby-2.6.6.drv' failed with exit code 1

As pointed out in the thread above, clang adds version args when calling the linker, which duplicates the flags set in ${darwin.binutils}/nix-support/libc-ldflags-before and causes the warning.

Just as a test, reverting 3c6bd61 is sufficient to build ruby.

@zowoq zowoq mentioned this pull request Oct 24, 2020
Closed
@thefloweringash thefloweringash mentioned this pull request Feb 5, 2021
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ericson2314 Ericson2314 Ericson2314 left review comments

@nbp nbp Awaiting requested review from nbp

Assignees

No one assigned

Labels

10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@matthewbauer @thefloweringash @Ericson2314