Skip to content

mumble: Enable build on Darwin#384691

Merged
FliegendeWurst merged 10 commits intoNixOS:masterfrom
dwt:mumble-fix-darwin-build
May 31, 2025
Merged

mumble: Enable build on Darwin#384691
FliegendeWurst merged 10 commits intoNixOS:masterfrom
dwt:mumble-fix-darwin-build

Conversation

@dwt
Copy link
Contributor

@dwt dwt commented Feb 24, 2025 •
edited
Loading

Things done

  • Modernize mumble build, to use lib.cmake* functions to specify the cmake options
  • Rename configureFlags to cmakeFlags, as the build had both, but it is actually a cmake build
  • Add Install-Phase to copy the built Application and create a wrapper to start mumble from the shell.

There is more work to be done, and I'd appreciate some guidance:

  • The app is missing it's icon, no idea why
  • There should be some libraries built, and I seem to be missing them? Other builds depend on mumble to get their libraries. This might be a mistake, it might be that the overlay build target is actually the one that builds should depend on, but I don't understand this yet.
  • …

This is based on my poco pull request, as poco is a dependency that didn't build on Darwin before, so that update is required. This is also one of the reasons I'm marking this as draft, to get early feedback.

  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Feb 24, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Feb 24, 2025
@dwt dwt changed the title Mumble: Enable build on Darwin mumble: Enable build on Darwin Feb 24, 2025
@dwt dwt force-pushed the mumble-fix-darwin-build branch 3 times, most recently from 19554b3 to 6026c68 Compare March 3, 2025 19:48
@dwt
Copy link
Contributor Author

dwt commented Mar 3, 2025

This PR builds on the changes proposed in #383163. I need the darwin build of poco here, so this probably has to stay a draft until that other pull request is merged.

@dwt dwt force-pushed the mumble-fix-darwin-build branch from 6026c68 to 1376ee6 Compare March 3, 2025 21:08
@dwt dwt mentioned this pull request Mar 3, 2025
Merged
13 tasks
@dwt
Copy link
Contributor Author

dwt commented Mar 3, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 384691

aarch64-linux

✅ 2 packages built:
  • mumble
  • murmur

aarch64-darwin

✅ 1 package built:
  • mumble

@felixsinger felixsinger self-requested a review March 3, 2025 23:09
@felixsinger
Copy link
Member

felixsinger commented Mar 3, 2025

Please rebase on master.

@dwt dwt force-pushed the mumble-fix-darwin-build branch from 1376ee6 to b53ac45 Compare March 4, 2025 10:32
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 4, 2025
@dwt dwt force-pushed the mumble-fix-darwin-build branch from b53ac45 to 9bee67a Compare March 4, 2025 11:15
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Mar 4, 2025
@dwt dwt marked this pull request as ready for review March 4, 2025 11:18
@dwt dwt force-pushed the mumble-fix-darwin-build branch from 9bee67a to 455d118 Compare March 4, 2025 11:21
@github-actions github-actions bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. and removed 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Mar 4, 2025
@dwt
Copy link
Contributor Author

dwt commented Mar 4, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 384691

aarch64-linux

✅ 2 packages built:
  • mumble
  • murmur

aarch64-darwin

✅ 1 package built:
  • mumble

@nix-owners nix-owners bot requested a review from Lilacious March 4, 2025 11:26
@dwt dwt force-pushed the mumble-fix-darwin-build branch from dcac7b2 to 5200b39 Compare March 4, 2025 11:38
@dwt
Copy link
Contributor Author

dwt commented Mar 4, 2025

Hm, seems that moving the package to pkgs/by-name/ is not as easy as I thought, might need to revert that.

@dwt
Copy link
Contributor Author

dwt commented Mar 4, 2025

The builds itself seem fine:

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 384691

aarch64-linux

✅ 2 packages built:
  • mumble
  • murmur

aarch64-darwin

✅ 1 package built:
  • mumble

felixsinger
felixsinger suggested changes Mar 4, 2025
View reviewed changes
@dwt
Copy link
Contributor Author

dwt commented Mar 4, 2025

@felixsinger Do you happen to know why the vet script is unhappy with the move of the package to /pkgs/by-name/? The code seems straightforward enough, but …

@dwt dwt force-pushed the mumble-fix-darwin-build branch from 5200b39 to 809f7de Compare March 4, 2025 15:18
@afh
Copy link
Member

afh commented Mar 4, 2025

@dwt, while I don't have an overly specific analysis for you as to why the vet check fails, I'd like to point out that mumble is a "package" of packages, i.e. it provides not only mumble, but also at least murmur.
There are several other packages like this, texinfo and boost come to mind. So it might be worthwhile to have a look at the differences how these packages are constructed and "published" in pkgs/top-level/all-packages.

@dwt
Copy link
Contributor Author

dwt commented May 24, 2025

Merge Conflicts resolved, and version display also works great.

image

@dwt
Copy link
Contributor Author

dwt commented May 24, 2025

@siraben Anything else I can provide?

felixsinger
felixsinger approved these changes May 24, 2025
View reviewed changes
Copy link
Member

@felixsinger felixsinger left a comment

Choose a reason for hiding this comment

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

awesome. let's get it in.

@siraben
Copy link
Member

siraben commented May 24, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 384691

aarch64-darwin

❌ 1 package failed to build:
  • ioq3-scion
✅ 1 package built:
  • mumble

@felixsinger
Copy link
Member

felixsinger commented May 24, 2025

1 package failed to build:
ioq3-scion

Not sure why that is listed anyway. It only supports Linux platforms as far as I can see.

@dwt
Copy link
Contributor Author

dwt commented May 25, 2025

1 package failed to build:

ioq3-scion

Not sure why that is listed anyway. It only supports Linux platforms as far as I can see.

I think, it is not marked as not supported on Mac, because it was never built for Mac in the past, due to mumble not supporting Darwin.

@FliegendeWurst
Copy link
Member

FliegendeWurst commented May 30, 2025

It would be good to update meta.platforms of ioq3-scion then, but that can also be done in another PR

FliegendeWurst
FliegendeWurst reviewed May 30, 2025
View reviewed changes
FliegendeWurst
FliegendeWurst reviewed May 30, 2025
View reviewed changes
@FliegendeWurst
Copy link
Member

FliegendeWurst commented May 30, 2025

Other changes look good to me.

@ofborg build mumble

dwt and others added 10 commits May 31, 2025 19:07
@dwt
mumble: rename configureFlags > cmakeFlags
3d6d1c6 
As a reader I first thought that this package was built using automatke,
when in fact it is built using cmake. This rename should help lessen that confusion.
@dwt
mumble: use lib helpers for cmake options where it makes them simpler
8dcb8d4 
These are the cases where bools have to be converted to options, so the
helpers help make the code shorter and easier to read.
@dwt
mumble: CMAKE_PREFIX_PATH is not required to build the server.
87952b4
@dwt
mumble: Add dependency on nix provided microsoft-gsl library
97d7f7c
@dwt
mumble: Add dependency on nix provided nlohmann_json
ae734d0
@dwt
mumble: Add correct dependency for use-speex option
d679111 
Support for the speex codec has long since been removed from upstream.

See mumble-voip/mumble#5869 for details.

the use-bundled-speex option now means to use the speex-dsp library
which is packaged separately in nix.
@dwt
mumble: Remove superfluous bundled-opus option
6263af2 
There is no such option anymore, as unbundled opus is now the only
supported option. See upstream d11fd05062f5684b3ffb698eb4cd3130356b6e9a
for details.
@dwt
mumble: Remove superfluous bundled-celt option
353f43c 
This option has been removed by upstream for a long time, as nobody uses
that codec anymore.

For details see upstream 4d05018c2e4f1bda48f6244b38f1e7bdc06de808
@dwt
mumble: enable building on darwin
b3277d1
@dwt
mumble: darwin: Fix missing version information in finder
50a753f 
Reported upstream at mumble-voip/mumble#6741
@dwt dwt force-pushed the mumble-fix-darwin-build branch from d6174e2 to 50a753f Compare May 31, 2025 17:08
@dwt
Copy link
Contributor Author

dwt commented May 31, 2025

@FliegendeWurst That should do it, anything else you see that can be improved?

@FliegendeWurst FliegendeWurst merged commit b94166b into NixOS:master May 31, 2025
14 of 18 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented May 31, 2025

Successfully created backport PR for release-25.05:

@nixpkgs-ci nixpkgs-ci bot mentioned this pull request May 31, 2025
Merged
1 task
@github-actions github-actions bot added the 8.has: port to stable This PR already has a backport to the stable release. label May 31, 2025
@dwt dwt deleted the mumble-fix-darwin-build branch July 26, 2025 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@FliegendeWurst FliegendeWurst FliegendeWurst approved these changes

@Lilacious Lilacious Awaiting requested review from Lilacious

@K900 K900 Awaiting requested review from K900

+4 more reviewers

@bengsparks bengsparks bengsparks left review comments

@keenanweaver keenanweaver keenanweaver left review comments

@returntoreality returntoreality returntoreality approved these changes

@felixsinger felixsinger felixsinger 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 8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@dwt @felixsinger @afh @Hartmnt @bengsparks @siraben @FliegendeWurst @returntoreality @keenanweaver @wegank