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

darwin.apple_sdk.frameworks.CoreFoundation: drop hook #284629

Closed
wants to merge 3 commits into from

Conversation

reckenrode
Copy link
Contributor

Description of changes

As of #265102, x86_64-darwin no longer uses the open-source CF release. Since there is no longer a need to switch between implementations, and the hook causes problems with cross-compilation (see #278348), drop the hook and make both darwin.CF an alias for darwin.apple_sdk.frameworks.CoreFoundation.

Thank you to @szlend for his work on #284288 and @r-burns for identifying the issue in #278348.

Testing was done by confirming the following packages built on aarch64- and x86_64-darwin:

  • bacula (requires patches in commit)
  • pkgsCross.muslpi.aws-sdk-cpp
  • wine64Packages.unstable

I am targeting this for backporting to fix the cross-compilation issue in 23.11. Since the hook is already not present by default, this change will only affect packages that explicitly use the CoreFoundation framework as a build input.

Fixes #278348
Closes #284288

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

As of NixOS#265102, x86_64-darwin no
longer uses the open-source CF release. Since there is no longer a need
to switch between implementations, and the hook causes problems with
cross-compilation (see NixOS#278348),
drop the hook and make both darwin.CF an alias for
darwin.apple_sdk.frameworks.CoreFoundation.
@reckenrode reckenrode changed the title darwin.apple_sdk.frameworks.CoreFoundation: drop hookgi darwin.apple_sdk.frameworks.CoreFoundation: drop hook Jan 28, 2024
After the CoreFoundation hook was dropped, bacula requires the following
changes to build on Darwin:

* Ensure bacula links CoreFoundation. Override incorrectly failing
  `configure` tests and add gettext as a build input. Even if those
  tests pass, bacula only links CoreFoundation when it finds libintl.
* Add Kerberos framework. Required to build on x86_64-darwin.
@szlend
Copy link
Member

szlend commented Jan 28, 2024

I am targeting this for backporting to fix the cross-compilation issue in 23.11. Since the hook is already not present by default, this change will only affect packages that explicitly use the CoreFoundation framework as a build input.

Sounds like a bit of a risky change to backport to 23.11. Is bacula the only darwin package that required patching, or was that the only one tested? Keep in mind that this could also affect custom derivations outside of nixpkgs.

Wouldn't it be safer to backport #284288 and only apply this MR to unstable?

@reckenrode
Copy link
Contributor Author

Sounds like a bit of a risky change to backport to 23.11. Is bacula the only darwin package that required patching, or was that the only one tested? Keep in mind that this could also affect custom derivations outside of nixpkgs.

bacula was tested because it was called out in the comments as needing the hook. The actual problem was that its configure script was failing to find and add CoreFoundation to its configure flags.

Wouldn't it be safer to backport #284288 and only apply this MR to unstable?

It would be less risky. If you want to retarget your PR to staging-23.11, I can drop the backport label from this one.

@reckenrode
Copy link
Contributor Author

reckenrode commented Jan 29, 2024

I dropped the label. I have a flake that builds the Darwin channel blockers plus some other packages that I’ll let run overnight. It going to build ~10k paths (total for both architectures) on top of the ones built already. I’ll report back when it’s done.

@szlend
Copy link
Member

szlend commented Jan 29, 2024

It would be less risky. If you want to retarget your PR to staging-23.11, I can drop the backport label from this one.

Alright, see #284706. I had to open a new PR because changing the base without updating the branch first caused GitHub to automatically mass-assign code reviewers.

@reckenrode
Copy link
Contributor Author

This is most likely going to get folded into the SDK refactor along with dropping other hooks where possible.

@reckenrode reckenrode marked this pull request as draft May 4, 2024 13:15
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 27, 2024
@aviallon
Copy link
Contributor

Closed by #346043

@aviallon aviallon closed this Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants