Skip to content

git: fix failing build on Darwin#170291

Merged
vcunat merged 1 commit intoNixOS:staging-nextfrom
reckenrode:darwin-git-fix
Apr 25, 2022
Merged

git: fix failing build on Darwin#170291
vcunat merged 1 commit intoNixOS:staging-nextfrom
reckenrode:darwin-git-fix

Conversation

@reckenrode
Copy link
Contributor

@reckenrode reckenrode commented Apr 25, 2022 •
edited
Loading

Description of changes

Fixes Darwin build failure in staging.

  • sandbox is set to relaxed.
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 22.05 Release Notes (or backporting 21.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@reckenrode reckenrode mentioned this pull request Apr 25, 2022
Merged
13 tasks
@reckenrode
Copy link
Contributor Author

reckenrode commented Apr 25, 2022

This addresses the breakage on Darwin after #169223.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Apr 25, 2022
@vcunat
Copy link
Member

vcunat commented Apr 25, 2022

Please cherry-pick against staging-next branch instead (and force-push).

@ofborg ofborg bot requested review from globin, primeos and wmertens April 25, 2022 17:29
@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 Apr 25, 2022
winterqt
winterqt requested changes Apr 25, 2022
View reviewed changes
Copy link
Member

@winterqt winterqt left a comment

Choose a reason for hiding this comment

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

I believe the convention we use is to add CoreServices to the function arguments, and then add it to the callPackage argument set in top-level with inherit. If you search through top-level you'll see a better example -- apologies for the terrible explanation.

@reckenrode reckenrode changed the base branch from staging to staging-next April 25, 2022 17:54
@reckenrode reckenrode requested review from a team, FRidh and jonringer as code owners April 25, 2022 17:54
@github-actions github-actions bot added 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. labels Apr 25, 2022
@reckenrode reckenrode force-pushed the darwin-git-fix branch 2 times, most recently from 330a240 to 6e91eb1 Compare April 25, 2022 17:56
@github-actions github-actions bot removed 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 6.topic: python Python is a high-level, general-purpose programming language. labels Apr 25, 2022
@vcunat
Copy link
Member

vcunat commented Apr 25, 2022

I didn’t add it to callPackage because I didn’t want to risk breaking anything since git is used in quite a few places with different git packages.

@reckenrode
Copy link
Contributor Author

reckenrode commented Apr 25, 2022 •
edited
Loading

Please cherry-pick against staging-next branch instead (and force-push).

Done.

I believe the convention we use is to add CoreServices to the function arguments, and then add it to the callPackage argument set in top-level with inherit. If you search through top-level you'll see a better example -- apologies for the terrible explanation.

No worries. I know what you mean. I originally avoided it here since I assumed git was doing it for a reason with darwin.Security. I went ahead and changed it to pass Security and CoreServices via callPackage.

@reckenrode
Copy link
Contributor Author

reckenrode commented Apr 25, 2022

@vcunat Should I have not pushed the change? I can revert. I’m keen to follow the prevailing convention, but I also don’t want to cause more breakage.

jonringer
jonringer reviewed Apr 25, 2022
View reviewed changes
@vcunat
Copy link
Member

vcunat commented Apr 25, 2022 •
edited
Loading

The merge base etc. look good to me now. Workflow docs, though without much detail: https://nixos.org/manual/nixpkgs/unstable/#submitting-changes-staging-next-branch

primeos
primeos approved these changes Apr 25, 2022
View reviewed changes
Copy link
Member

@primeos primeos left a comment

Choose a reason for hiding this comment

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

I cannot test Darwin changes but the diff LGTM. Thanks! :)

@vcunat vcunat merged commit 00468f7 into NixOS:staging-next Apr 25, 2022
@reckenrode reckenrode deleted the darwin-git-fix branch May 11, 2022 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 approved these changes

@winterqt winterqt winterqt approved these changes

@wmertens wmertens Awaiting requested review from wmertens

@globin globin Awaiting requested review from globin

@FRidh FRidh Awaiting requested review from FRidh

+2 more reviewers

@primeos primeos primeos approved these changes

@jonringer jonringer jonringer approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@reckenrode @vcunat @SuperSandro2000 @primeos @jonringer @winterqt