-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
mrtrix3: init at 0.3.16 #33892
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.
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
mrtrix3: init at 0.3.16 #33892
Conversation
Needed for sandbox build.
Not sure of an appropriate reviewer, perhaps @ttuegel as I am unsure I have correctly assigned qt dependencies as buildInputs vs. nativeBuildInputs. |
pkgs/top-level/all-packages.nix
Outdated
@@ -18870,6 +18870,8 @@ with pkgs; | |||
|
|||
mrbayes = callPackage ../applications/science/biology/mrbayes { }; | |||
|
|||
mrtrix3 = callPackage ../applications/science/biology/mrtrix { }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use qt56.callPackage
here instead of callPackage
.
}; | ||
|
||
nativeBuildInputs = [ | ||
qt56.qmake |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove the qt56.
prefix from qmake
, qtscript
, and qtsvg
.
|
||
nativeBuildInputs = [ | ||
qt56.qmake | ||
qt56.qtscript |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
qmake
is a nativeBuildInput
because it is a build tool, but qtscript
is a runtime dependency that should go in buildInputs
.
Thanks for the review. I have made all suggested changes. I also created a new category: science/imaging. If accepted, I will make a new PR to move science/biology/ants to science/imaging/ants also, as it is a more appropriate classification. I also have some other packages to contribute upstream that would belong there. |
Tried to merge — a strange build problem about |
|
Looks better to me, thanks @bcdarwin |
Motivation for this change
Add MRTrix3
Things done
build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)