Skip to content

direwolf: include darwin and BSD deps - #498631

Merged
SuperSandro2000 merged 1 commit into
NixOS:masterfrom
sarcasticadmin:rh/1773169113
Aug 10, 2026
Merged

direwolf: include darwin and BSD deps#498631
SuperSandro2000 merged 1 commit into
NixOS:masterfrom
sarcasticadmin:rh/1773169113

Conversation

@sarcasticadmin

@sarcasticadmin sarcasticadmin commented Mar 10, 2026

Copy link
Copy Markdown
Member

Things done

This fixes the build of direwolf on darwin:

$ nix build .#direwolf
...
       > CMake Error in src/CMakeLists.txt:
       >   Found relative path while evaluating include directories of "appserver":
       >
       >     "HIDAPI_INCLUDE_DIRS-NOTFOUND"
       >
       >
       >
       > -- Generating done (0.1s)
       > CMake Warning:
       >   Manually-specified variables were not used by the project:
       >
       >     BUILD_TESTING
       >     CMAKE_EXPORT_NO_PACKAGE_REGISTRY
       >     CMAKE_INSTALL_BINDIR
       >     CMAKE_INSTALL_DOCDIR
       >     CMAKE_INSTALL_INCLUDEDIR
       >     CMAKE_INSTALL_INFODIR
       >     CMAKE_INSTALL_LIBDIR
       >     CMAKE_INSTALL_LIBEXECDIR
       >     CMAKE_INSTALL_LOCALEDIR
       >     CMAKE_INSTALL_MANDIR
       >     CMAKE_INSTALL_SBINDIR
       >
       >
       > CMake Generate step failed.  Build files cannot be regenerated correctly.
       For full logs, run:

Supersedes: #484861

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

@nixpkgs-ci
nixpkgs-ci Bot requested review from Pandapip1 and lasandell March 10, 2026 20:16
@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. labels Mar 10, 2026
@ofborg ofborg Bot added 6.topic: bsd Running or building packages on BSD 6.topic: darwin Running or building packages on Darwin labels Mar 10, 2026
Comment thread pkgs/by-name/di/direwolf/package.nix Outdated
@sarcasticadmin sarcasticadmin mentioned this pull request May 23, 2026
14 tasks
@DaGenix

DaGenix commented May 24, 2026

Copy link
Copy Markdown
Contributor

Also builds for me on aarch64-darwin

@Pandapip1 Pandapip1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm approving this just so we can finally get it merged but I'd just like it to be known that I am not a fan of shipping packages that are intentionally missing functionality for no good reason (see the Debian KeePassXC debacle for an example of this for a popular package on a popular distro; IIUC what you're suggesting we do with direwolf is an exact analog).

"Fewer dependencies" is not a good reason. If a dependency is large (like GTK), reducing closure size by a not insignificant amount is a good reason and while I still don't agree with keeping it disabled by default I can at least accept that there is a logical reason for doing so. But avahi is small; if you're running a system where the difference between bringing avahi in and keeping it out matters, then you can set the option we provide and build it yourself. I wouldn't even be opposed to a direwolfMinimal package so we can provide prebuilt binaries.

I will be submitting a follow-up PR to move this back to the sensible default once this is merged.

@nixpkgs-ci nixpkgs-ci Bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 10, 2026
@nixos-discourse

Copy link
Copy Markdown

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/3230

@sarcasticadmin

Copy link
Copy Markdown
Member Author

"Fewer dependencies" is not a good reason. If a dependency is large (like GTK), reducing closure size by a not insignificant amount is a good reason and while I still don't agree with keeping it disabled by default I can at least accept that there is a logical reason for doing so. But avahi is small; if you're running a system where the difference between bringing avahi in and keeping it out matters, then you can set the option we provide and build it yourself. I wouldn't even be opposed to a direwolfMinimal package so we can provide prebuilt binaries.

I will be submitting a follow-up PR to move this back to the sensible default once this is merged.

cups has a similar default option in nixpkgs:

@Pandapip1

Pandapip1 commented Aug 10, 2026

Copy link
Copy Markdown
Member

Yea, that still has avahi enabled by default. It just also handles the situation where it's manually removed gracefully. The package ? null syntax in a callPackage will mean that if package exists, it will be passed, and if it doesn't, null will be passed. avahi does exist, so it gets passed by default. I also know this because I rely on CUPS's mDNS/avahi integration, which does work.

@sarcasticadmin

Copy link
Copy Markdown
Member Author

Yea, that still has avahi enabled by default. It just also handles the situation where it's manually removed gracefully. The package ? null syntax in a callPackage will mean that if package exists, it will be passed, and if it doesn't, null will be passed. avahi does exist, so it gets passed by default. I also know this because I rely on CUPS's mDNS/avahi integration, which does work.

yep, direwolf now been enabled by default to align

@sarcasticadmin

Copy link
Copy Markdown
Member Author

also rebased since this was so old

@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Aug 10, 2026

@Pandapip1 Pandapip1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

TY!

@SuperSandroBot

Copy link
Copy Markdown

nixpkgs-review result

Generated using nixpkgs-review-gha (0d7e614)

Command: nixpkgs-review pr 498631
Commit: fd9663a59e42974af81373dfac61ea26d1f77e67 (subsequent changes)
Merge: d72ae0b6e23955465cf8408eadb4747dcba15daa

Logs: https://github.com/SuperSandro2000/nixpkgs-review-gha/actions/runs/31433332567/attempts/1


x86_64-linux

✅ 2 packages built:
  • direwolf
  • direwolf-unstable

aarch64-linux

✅ 2 packages built:
  • direwolf
  • direwolf-unstable

aarch64-darwin (sandbox = relaxed)

✅ 2 packages built:
  • direwolf
  • direwolf-unstable

@SuperSandro2000
SuperSandro2000 added this pull request to the merge queue Aug 10, 2026
Merged via the queue into NixOS:master with commit 280f5dc Aug 10, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: bsd Running or building packages on BSD 6.topic: darwin Running or building packages on Darwin 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. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants