Skip to content

Comments

darwin.darling: fix build on case-insensitive filesystems#110685

Closed
holymonson wants to merge 1 commit intoNixOS:stagingfrom
holymonson:darling
Closed

darwin.darling: fix build on case-insensitive filesystems#110685
holymonson wants to merge 1 commit intoNixOS:stagingfrom
holymonson:darling

Conversation

@holymonson
Copy link
Contributor

@holymonson holymonson commented Jan 24, 2021

Motivation for this change

Use upstream darlinghq/darling@ea5f07d to fix build on case-insensitive filesystems.
Also update headers in Libsystem due to path modified.

Tested darling but not Libsystem.

cc @veprbl @thefloweringash
fix #91480 #107431

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.

@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 24, 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: 0 This PR does not cause any packages to rebuild on Linux. labels Jan 24, 2021
@holymonson holymonson force-pushed the darling branch 2 times, most recently from 4f6d4b1 to b7a716d Compare January 25, 2021 06:51
@holymonson
Copy link
Contributor Author

holymonson commented Jan 25, 2021

@GrahamcOfBorg build darwin.Libsystem

@holymonson holymonson mentioned this pull request Jan 26, 2021
Closed
10 tasks
veprbl
veprbl requested changes Jan 27, 2021
View reviewed changes
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 breaks stdenv https://gist.github.com/veprbl/f94b093aeaaaf879506432add0422f62

@holymonson
Copy link
Contributor Author

holymonson commented Jan 27, 2021

This breaks stdenv https://gist.github.com/veprbl/f94b093aeaaaf879506432add0422f62

It's because the new os/availability.h updated here conflicted with the old Availability.h in xnu. Now we have 3 options:

  1. You directly review darwin.xnu: update and restructure #110840, which contains both new daring and new xnu, it could build Libsystem from bootstrap. This PR wouldn't be needed anymore.
  2. I cp the new Availability.h from daring temporarily (it contains anyway), get this PR merged, and replace it when darwin.xnu: update and restructure #110840 merged.
  3. I drop the new os/availability.h here, get this PR merged, and update it on darwin.xnu: update and restructure #110840 's commit.

@veprbl which one do you prefer?

@veprbl
Copy link
Member

veprbl commented Jan 27, 2021

I would try to stick to first updating darling if that is possible without breaking anything.

@holymonson
darwin.darling: fix build on case-sensitive filesystems
3600332 
Use upstream darlinghq/darling@ea5f07d to fix build on case-sensitive filesystems.
Also update headers in Libsystem due to path modified.

fix NixOS#91480
veprbl
veprbl reviewed Jan 28, 2021
View reviewed changes
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.

I think, commit title should state something like "darwin.darling: unstable-2018-02-10 -> unstable-2020-04-26"

pkgs/os-specific/darwin/apple-source-releases/Libsystem/default.nix
os/activity.h
os/log.h
os/trace.h
")
Copy link
Member

@veprbl veprbl Jan 28, 2021

Choose a reason for hiding this comment

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

/nix/store/pawa4j5ywf0s0mm1khk8f45qx8qrj49v-Libsystem-1238.60.2-osx-10.12.6/include/os/trace.h:32:10: fatal error: 'os/trace_base.h' file not found
#include <os/trace_base.h>
^~~~~~~~~~~~~~~~~

Copy link
Contributor

@r-burns r-burns Jan 28, 2021

Choose a reason for hiding this comment

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

I think this header was added in SDK 10.13. Compare:
https://github.com/linxun45/MacOSX-SDKs/tree/aea47c83334af9c27dc57c49ca268723ef5e6349/MacOSX10.12.sdk/usr/include/os
vs
https://github.com/linxun45/MacOSX-SDKs/tree/aea47c83334af9c27dc57c49ca268723ef5e6349/MacOSX10.13.sdk/usr/include/os

Copy link
Contributor Author

@holymonson holymonson Jan 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Here is a little complicated, os/trace_base.h required os/availability.h (not the one below, a new one), while the new os/availability.h required new Availability.h in xnu according to #110685 (review) . We have to do it step by step to keep minimal change in each PR.

@veprbl would you review #109368 first? Then I will try making a pure xnu update, then darling.

@stale
Copy link

stale bot commented Jul 30, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 30, 2021
@veprbl veprbl added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 30, 2021
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 30, 2021
@stale
Copy link

stale bot commented Apr 19, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 19, 2022
@wegank wegank closed this Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@veprbl veprbl veprbl requested changes

@r-burns r-burns r-burns left review comments

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@copumpkin copumpkin Awaiting requested review from copumpkin

@gridaphobe gridaphobe Awaiting requested review from gridaphobe

@thefloweringash thefloweringash Awaiting requested review from thefloweringash

@siraben siraben Awaiting requested review from siraben

Assignees

No one assigned

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: darwin Running or building packages on Darwin 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: 0 This PR does not cause any packages to rebuild on Linux.

Projects

No open projects
Staging
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@holymonson @veprbl @r-burns @wegank