Skip to content

Comments

darwin: generate apple packages preparing for update macos sdk#108590

Merged
veprbl merged 1 commit intoNixOS:stagingfrom
holymonson:apple_package_version
Jan 23, 2021
Merged

darwin: generate apple packages preparing for update macos sdk#108590
veprbl merged 1 commit intoNixOS:stagingfrom
holymonson:apple_package_version

Conversation

@holymonson
Copy link
Contributor

Motivation for this change

macos sdk is outdated for a long time (#101229), this PR provides scaffolds and candidates for future updating.
Except for normalizing package name as ${pname}-${version}-${sdkName}, in essence this PR doesn't update any packages. We may want a smooth update process divided into small tasks instead of a large package commit.

Things done

Generate package sources and rewrite some function names.
Will cause mass rebuild due to some derivation names normalized, but it should not cause any actual change in compiling.

cc @matthewbauer


  • 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.

@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. labels Jan 6, 2021
@ofborg ofborg bot added 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 Jan 6, 2021
@holymonson
Copy link
Contributor Author

Success building all apple packages and the depending bootstrap packages, except #108651.

add cc @siraben @SuperSandro2000

@SuperSandro2000
Copy link
Member

I saw this already earlier. If I understand it correctly this should make the update process easier but does not actually change any packages?

Also please change the commit message and PR title to something starting with darwin: or darwin.appke_sdk.

@holymonson
Copy link
Contributor Author

If I understand it correctly this should make the update process easier but does not actually change any packages?

Yes.

We hesitate to update apple_sdk is mainly because we make up a header-only Libsystem, which is required for bootstrap. But for many other apple packages in apple-source-releases, there are few packages depending on them individual, and could be updated without a mass impact. For example, network_cmds is marked as insecure because of openssl-1.0.2u since 2019, we could keep it up-to-date instead of sticking to osx-10.11.6.

My plan is to take down those light-depended packages one by one, the PR could make it easier.

@holymonson holymonson force-pushed the apple_package_version branch from 0ffa9d3 to 8301dfa Compare January 7, 2021 11:57
@holymonson holymonson changed the title generate apple packages preparing for update macos sdk darwin: generate apple packages preparing for update macos sdk Jan 7, 2021
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/445

@siraben siraben requested review from Mic92 and removed request for Mic92 January 15, 2021 15:22
@holymonson
Copy link
Contributor Author

@veprbl could you review this?

Copy link
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

This is a bit hard to review as orthogonal changes are implemented in one commit, but here is what I saw

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
packages = developerToolsPackages // macosPackages // stubPackages;
packages = developerToolsPackages // macosPackages // stubPackages;

Do I understand it correctly that developerToolsPackages // macosPackages is a dead code right now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, right now it is. Next step if we remove a line in stubPackages, then the package will be updated.

Copy link
Member

Choose a reason for hiding this comment

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

It would have been more clean to first regenerate the current set with your framework to prove that it works and then do the update separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hard to do that because you see versions containing multiple versions of sdk and even some of them are fake, I would rather keep it what it is. And I won't update all package in a time, instead, I'm going to create PR later one by one and test to ensure they work. At least in this PR the redundant framework won't take prior than the original stubPackages.

Copy link
Member

Choose a reason for hiding this comment

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

Well, my thought was that we could still hold on including definitions developerToolsPackages and macosPackages until at least one of the updated frameworks is used. But if you are confident that it will work out the way you've designed, then we could try.

@SuperSandro2000 SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 18, 2021
@holymonson holymonson mentioned this pull request Jan 19, 2021
10 tasks
@holymonson holymonson force-pushed the apple_package_version branch from 8301dfa to 8a7c84d Compare January 19, 2021 18:47
@holymonson
Copy link
Contributor Author

@veprbl I have rewrite some function name back and add new function with suffix _ , hope this would make it easier to review.

@holymonson holymonson requested a review from veprbl January 19, 2021 18:54
@holymonson holymonson force-pushed the apple_package_version branch from 8a7c84d to be92725 Compare January 19, 2021 18:55
@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Jan 19, 2021
@ofborg ofborg bot added the 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. label Jan 19, 2021
@holymonson holymonson force-pushed the apple_package_version branch 2 times, most recently from b5c5507 to 77096f4 Compare January 20, 2021 07:13
@holymonson holymonson force-pushed the apple_package_version branch from 77096f4 to 9d5cece Compare January 20, 2021 07:19
@holymonson holymonson requested a review from veprbl January 20, 2021 07:24
@holymonson
Copy link
Contributor Author

gentle ping @matthewbauer

Copy link
Member

@siraben siraben left a comment

Choose a reason for hiding this comment

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

LGTM

@veprbl veprbl merged commit 963286d into NixOS:staging Jan 23, 2021
Security = applePackage "Security/boot.nix" "osx-10.9.5" "1nv0dczf67dhk17hscx52izgdcyacgyy12ag0jh6nl5hmfzsn8yy" {};
};

packages = developerToolsPackages_11_3_1 // macosPackages_11_0_1 // stubPackages;
Copy link
Contributor

@bobrik bobrik Feb 5, 2021

Choose a reason for hiding this comment

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

Is the order here correct? Should stub packages take precedence over everything else?

I have #105026 locally and it tries to install libutil-47.30.1-osx-10.12.6, which doesn't seem right (and it also fails, because 10.12 doesn't know anything about aarch64).

cc @thefloweringash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the order here correct? Should stub packages take precedence over everything else?

stubPackages does take precedence, the later overwrites the formers.

I have #105026 locally and it tries to install libutil-47.30.1-osx-10.12.6, which doesn't seem right (and it also fails, because 10.12 doesn't know anything about aarch64).

libutil-47.30.1-osx-10.12.6 is in stubPackages, we haven't updated libutil yet, so it still installs the old 10.12 one, that is exactly expected. I'm a bit confused of which version is you actually want.

@bobrik bobrik mentioned this pull request Feb 6, 2021
21 tasks
@toonn toonn mentioned this pull request Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: darwin Running or building packages on Darwin 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package 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. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants