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

rpm: fix build on Darwin #346967

Merged
merged 1 commit into from
Nov 6, 2024
Merged

rpm: fix build on Darwin #346967

merged 1 commit into from
Nov 6, 2024

Conversation

reckenrode
Copy link
Contributor

RPM needs a 13.x SDK or newer.

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.11 Release Notes (or backporting 23.11 and 24.05 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.

@reckenrode reckenrode mentioned this pull request Oct 6, 2024
Merged
52 tasks
@emilazy
Copy link
Member

emilazy commented Oct 7, 2024

Finally, macOS Nix users can have a decent package manager.

@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 8.has: package (new) This PR adds a new package labels Oct 7, 2024
@ofborg ofborg bot requested a review from copumpkin October 7, 2024 01:05
@github-actions github-actions bot added 6.topic: python 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: haskell 8.has: documentation This PR adds or changes documentation 8.has: changelog 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: emacs Text editor 6.topic: ruby 6.topic: nodejs 6.topic: pantheon The Pantheon desktop environment 6.topic: module system About "NixOS" module system internals 6.topic: systemd 6.topic: nim Nim programing language 6.topic: vscode 6.topic: lib The Nixpkgs function library 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Oct 7, 2024
@reckenrode reckenrode force-pushed the reckenrode/darwin-sdk-refactor branch 2 times, most recently from 183877a to 1fd30e4 Compare October 8, 2024 02:51
@github-actions github-actions bot removed 6.topic: python 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: haskell labels Oct 8, 2024
@reckenrode reckenrode force-pushed the reckenrode/darwin-sdk-refactor branch 2 times, most recently from a1adff0 to e3f2829 Compare October 10, 2024 20:25
@emilazy emilazy marked this pull request as ready for review October 10, 2024 22:54
@emilazy emilazy deleted the branch NixOS:master October 11, 2024 00:00
@emilazy emilazy closed this Oct 11, 2024
@reckenrode reckenrode reopened this Oct 11, 2024
@reckenrode reckenrode changed the base branch from reckenrode/darwin-sdk-refactor to staging October 11, 2024 00:04
emilazy
emilazy requested changes Oct 11, 2024
View reviewed changes
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Could you try bumping to 4.20 instead, which includes these fixes? Otherwise we need to adjust the commit hashes.

pkgs/tools/package-management/rpm/default.nix Outdated
Comment on lines 41 to 46
# Resolves `error: expected expression` on clang
# See: https://github.com/rpm-software-management/rpm/issues/2435.
(fetchpatch2 {
url = "https://github.com/rpm-software-management/rpm/commit/3b7cc5b5362fc22cb6f96180741e2701e703844d.diff?full_index=1";
hash = "sha256-pBKYQK97RkK9bigroNVgeDrR37poFSUBdEpB/RjwK5A=";
})
Copy link
Member

Choose a reason for hiding this comment

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

This is included in 4.19 and 4.20; we should probably bump the version instead. If not, we should at least point to b960c0b43a080287a7c13533eeb2d9f288db1414, which is the version of the commit that was landed and won’t get GC’d.

pkgs/tools/package-management/rpm/default.nix
Comment on lines +49 to +56
(fetchpatch2 {
url = "https://github.com/rpm-software-management/rpm/commit/f07875392a09228b1a25c1763a50bbbd0f6798c2.diff?full_index=1";
hash = "sha256-DLpzMApRCgI9zqheglFtqL8E1vq9X/aQa0HMnIAQgk8=";
})
(fetchpatch2 {
url = "https://github.com/rpm-software-management/rpm/commit/b2e67642fd8cb64d8cb1cca9e759396c1c10807d.diff?full_index=1";
hash = "sha256-q3fIBfiUJVmw6Vi2/Bo/zX6/cqTM7aFnskKfMVK3DlU=";
})
Copy link
Member

Choose a reason for hiding this comment

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

This PR was also rebased for merge and is in 4.20.

@reckenrode reckenrode force-pushed the push-vvywqpsumluy branch 3 times, most recently from 0f3443f to 945c5d8 Compare October 11, 2024 22:41
emilazy
emilazy reviewed Oct 11, 2024
View reviewed changes
pkgs/tools/package-management/rpm/default.nix
# Support for darwin was removed in https://github.com/NixOS/nixpkgs/pull/196350.
# This can be re-enables for apple_sdk.version >= 13.0.
badPlatforms = platforms.darwin;
platforms = platforms.linux ++ platforms.darwin;
Copy link
Member

Choose a reason for hiding this comment

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

BTW, I’m guessing this can be platforms.unix.

@reckenrode
Copy link
Contributor Author

RPM 4.20 is a non-trivial effort to package. That’s way beyond what I want to do to update it for Darwin. If this isn’t mergable, I’d rather leave Darwin marked broken.

@emilazy
Copy link
Member

emilazy commented Oct 13, 2024

I think it’s fine to merge this and let someone else handle RPM 4.20 as long as we can point the fetches to the copies of the commits that were actually merged rather than the potentially ephemeral ones from the PRs.

@reckenrode
Copy link
Contributor Author

I fixed the commit to be the one from the repo.

@emilazy
Copy link
Member

emilazy commented Oct 13, 2024

The other PR needs its commits changing too (it seems like they do rebase merges).

@reckenrode
rpm: fix build on Darwin
7187419 
RPM needs a 13.x SDK or newer.
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 4, 2024
@reckenrode reckenrode marked this pull request as draft November 4, 2024 23:41
@reckenrode reckenrode changed the base branch from staging to master November 4, 2024 23:52
@reckenrode reckenrode marked this pull request as ready for review November 4, 2024 23:52
@reckenrode
Copy link
Contributor Author

reckenrode commented Nov 4, 2024 •
edited
Loading

No changes except I retargeted to master due to the low number of rebuilds. I’ll merge once all checks finish (except for the slow, aarch64-darwin one).

@reckenrode
Copy link
Contributor Author

@emilazy Is this good to merge? It’s blocked currently.

emilazy
emilazy approved these changes Nov 6, 2024
View reviewed changes
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Forgot I had a blocking review on this, sorry. Seems fine, hopefully someone else can figure out the new build system another time.

@emilazy emilazy merged commit 0080e28 into NixOS:master Nov 6, 2024
35 of 36 checks passed
@reckenrode reckenrode deleted the push-vvywqpsumluy branch November 6, 2024 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paparodeo paparodeo paparodeo approved these changes

@emilazy emilazy emilazy approved these changes

@copumpkin copumpkin Awaiting requested review from copumpkin

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 101-500 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@reckenrode @emilazy @paparodeo @wegank