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.

Sign up for GitHub

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

Jump to bottom

use newer libproc.h #107189

Closed
wants to merge 1 commit into from
Closed

use newer libproc.h #107189

wants to merge 1 commit into from

Conversation

holymonson
Copy link
Contributor

Motivation for this change

libproc.h is too old.

Things done

Replace it with a new one in xnu.

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

@FRidh
Copy link
Member

FRidh commented Dec 20, 2020

cc @NixOS/darwin-maintainers

@FRidh FRidh added the 6.topic: darwin Running or building packages on Darwin label Dec 20, 2020
@LnL7
Copy link
Member

LnL7 commented Dec 21, 2020

Looks reasonable, what builds did you test?

@holymonson
Copy link
Contributor Author

I have reviewed 4 versions of libproc.h:

  1. The oldest one from Libc-825.40.1 on macOS 10.8.5 Source, which is using in Libsystem-osx-10.12.6 now.
  2. The one from xnu-3789.70.16 on macOS 10.12.6 Source.
  3. The one from xnu-6153.81.5 on macOS 10.15.3 Source, which is identical to the one in MacOSX10.15.sdk.
  4. The newest one in MacOSX11.1.sdk.

They are sequentially adding some macros and functions, and those new functions are maked with suffix like __OSX_AVAILABLE_STARTING(__MAC_10_9, __IPHONE_7_0), so I think it's backward-compatible.

I'm building webkitgtk on Darwin with the (3) and success. The (2) should be fine too because it contains proc_pid_rusage(). Other builds haven't been tested because building Libsystem-osx-10.12.6 will cause too much chain
rebuilding on my PC.

@holymonson
Copy link
Contributor Author

holymonson commented Dec 21, 2020 •
edited

nix-build -A pkgs.darwin.Libsystem is OK, and its contents looks fine but not really tested (mainly Libsystem-osx-10.12.6/include/libproc.h updated).

@LnL7
Copy link
Member

LnL7 commented Dec 21, 2020 •
edited

I'm a bit confused by those versions, there's only one version of xnu in nixpkgs AFAIK.

@holymonson
Copy link
Contributor Author

I'm a bit confused by those versions, there's only one version of xnu in nixpkgs AFAIK.

Sorry for not making myself clear. The xnu in nixpkgs now is xnu-3789.70.16 on macOS 10.12.6 Source, and we can find more on https://opensource.apple.com/ againest other version of macOS. Further more, we could find newer libproc.h in /Library/Developer/CommandLineTools/SDKs/MacOSX11.1.sdk on our own macOS.

What I reviewed is mainly the availability and compatibility of several versions of libproc.h itself, NOT the impact of the PR to nixpkgs. If we use libproc.h in xnu, and in the future update it with newer xnu and Libsystem-osx, I want to make sure it will be all right. (BTW, Libsystem-osx-10.12.6 is too old and I have to backport 3 patches on headers for building webkitgtk.)

For test against nixpkgs, I merely run nix-build -A pkgs.darwin.Libsystem with this PR and check its contents. No further test been done on libs and binaries depending Libsystem-osx.

@holymonson holymonson mentioned this pull request Dec 24, 2020
Open
@holymonson holymonson changed the base branch from master to staging December 25, 2020 17:40
@stale
Copy link

stale bot commented Jun 26, 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 Jun 26, 2021
@Artturin
Copy link
Member

No longer necessary #237348

@Artturin Artturin closed this Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thefloweringash thefloweringash Awaiting requested review from thefloweringash

@LnL7 LnL7 Awaiting requested review from LnL7

Assignees
No one assigned
Labels
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+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@holymonson @FRidh @LnL7 @Artturin