Skip to content

buildDubPackage: split into hooks and into importDubLock#344744

Merged
philiptaron merged 1 commit intoNixOS:masterfrom
TomaSajt:dub-refactor
Aug 29, 2025
Merged

buildDubPackage: split into hooks and into importDubLock#344744
philiptaron merged 1 commit intoNixOS:masterfrom
TomaSajt:dub-refactor

Conversation

@TomaSajt
Copy link
Contributor

@TomaSajt TomaSajt commented Sep 26, 2024

Description of changes

This PR splits buildDubPackage into dub{Setup,Build,Check}Hook and importDubLock

importDubLock just creates the directory structure that will be copied into $DUB_HOME by the setup hook. It's not compressed.


I wasn't able to include the compiler parameter of buildDubPackage in the build hook, since it uses lib.getExe and it has extra stripping logic attached to it.


A question that still remains is: should it be dubSetupHook or dubConfigHook and which hook should it be the part of. (postPatch and preConfigure seem like good options to me.)


The API did not change, so the docs do not really have to be changed.
The default values are still marked with ? defaultVal in the docs, whereas the new hooks use shell parameter expansion for defaults, but I think that's fine.

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.

@TomaSajt TomaSajt requested a review from jtbx September 26, 2024 21:44
Copy link
Member

@jtbx jtbx left a comment

Choose a reason for hiding this comment

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

From a quick glance looks good. If it works it works, I would prefer if the build support files were in a subdirectory away from the package.

@AndersonTorres
Copy link
Member

I would suggest you to keep them inside pkgs/by-name/dub/build-support/.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Sep 27, 2024
@TomaSajt
Copy link
Contributor Author

I reverted the part that moved from the original directories. We don't really have to do that in this PR.

@TomaSajt TomaSajt changed the title Migrate dub-support to pkgs/by-name and use setup hooks buildDubPackage: split into hooks and into importDubLock Sep 27, 2024
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 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 Sep 27, 2024
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one person. label Sep 29, 2024
@TomaSajt TomaSajt force-pushed the dub-refactor branch 3 times, most recently from 09dc047 to 5e3e1cd Compare October 29, 2024 17:09
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 10, 2024
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 5, 2025
@TomaSajt TomaSajt force-pushed the dub-refactor branch 2 times, most recently from fdfc2a5 to 096b782 Compare March 5, 2025 20:55
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 25, 2025
@nixpkgs-ci nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 3, 2025
@philiptaron
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 344744
Commit: d3f69ec125f908a34379e56a71a3aa5d71865fe9


x86_64-linux

❌ 1 package failed to build:
  • dstep
✅ 10 packages built:
  • btdu
  • dub-to-nix
  • dubBuildHook
  • dubCheckHook
  • dubSetupHook
  • inochi-creator
  • inochi-session
  • literate
  • luneta
  • serve-d

Error logs: `x86_64-linux`
dstep
----------------
??:? [0x45f5d6]
??:? [0x45f242]
??:? [0x48565e]
??:? [0x4674ef]
??:? [0x42cf55]
??:? [0x42c434]
??:? [0x429505]
??:? [0x428255]
??:? [0x407f1e]
??:? [0x4077c6]
??:? [0x407343]
??:? [0x4671cc]
??:? [0x4670c6]
??:? [0x466f1c]
??:? [0x408d41]
??:? [0x7ffff7c2a4d7]
??:? __libc_start_main [0x7ffff7c2a59a]
??:? [0x404a14]
Error: /build/configure-55383a failed with status: 1

@philiptaron
Copy link
Contributor

Could you please take a look at the dstep failure?

@TomaSajt
Copy link
Contributor Author

Fixed.
Now, just like before this PR, configurePhase will not run the default ./configure logic
(This is similar to buildRustPackage, where the entire configurePhase is just calling the pre and post hooks)

@philiptaron
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 344744
Commit: 2d62506a97b2ba80c378756c78bdebba0ab6be89


x86_64-linux

✅ 11 packages built:
  • btdu
  • dstep
  • dub-to-nix
  • dubBuildHook
  • dubCheckHook
  • dubSetupHook
  • inochi-creator
  • inochi-session
  • literate
  • luneta
  • serve-d

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Aug 29, 2025
@philiptaron philiptaron merged commit ee78ea6 into NixOS:master Aug 29, 2025
30 of 32 checks passed
PetarKirov added a commit to metacraft-labs/nixos-modules that referenced this pull request Dec 5, 2025
… `buildDubPackage`

PR NixOS/nixpkgs#344744 introduced a breaking change
in the way `dubTestFlags` are passed to `dub-check-hook.sh`.
PetarKirov added a commit to metacraft-labs/nixos-modules that referenced this pull request Dec 5, 2025
… `buildDubPackage`

PR NixOS/nixpkgs#344744 introduced a breaking change
in the way `dubTestFlags` are passed to `dub-check-hook.sh`.
@TomaSajt TomaSajt deleted the dub-refactor branch January 20, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: package (new) This PR adds a new package 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. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants