-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
openbugs: init at 3.2.3 #233506
openbugs: init at 3.2.3 #233506
Conversation
pkgs/applications/science/machine-learning/openbugs/default.nix
Outdated
Show resolved
Hide resolved
pkgs/applications/science/machine-learning/openbugs/default.nix
Outdated
Show resolved
Hide resolved
pkgs/applications/science/machine-learning/openbugs/default.nix
Outdated
Show resolved
Hide resolved
de8a99f
to
ba61dbe
Compare
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.
Thank you, lgtm!
While looking at something else, I discovered # A stdenv capable of building 32-bit binaries.
# On x86_64-linux, it uses GCC compiled with multilib support; on i686-linux,
# it's just the plain stdenv.
stdenv_32bit = lowPrio (if stdenv.hostPlatform.is32bit then stdenv else multiStdenv); which probably does what we want here, though I'm not sure if it's preferred. |
You were right, now it works for 32 bits also. I have tried it and it compiles and runs smoothly. I have just made a small tweak in the buildInputs. |
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.
Sorry, I meant that we can swap out stdenv
for stdenv_32bit
entirely, and omit the references to gcc_multi
and nativeBuildInputs
. While we're at it, we should leave a comment explaining why we have to use stdenv_32bit
.
Kind of like https://github.com/nixos/nixpkgs/blob/master/pkgs/tools/misc/loadlibrary/default.nix
It still needs to be viewed by someone with commit privileges (I'm new to nixpkgs too).
Yes, I understood you but I think this way it is cleaner and better as you can see de dependences explicitly. Is there any other advantage using stdenv_32bit? No problem, thank you for your work. |
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.
OK, in that case LGTM for now 👍
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.
This package is really only supported on 32-bit x86, right? The gcc_multi
thing is a bit of a hack. So probably it would make more sense to only support 32-bit in the package, but tell 64-bit Linux to use the 32-bit version?
(But I'm not a multilib expert, so I'm happy to be corrected by somebody who knows more about how we usually handle this stuff — I just note that we don't have any other packages directly using gcc_multi apart from LLVM, but a few packages that use pkgsi686Linux.callPackage.)
pkgs/applications/science/machine-learning/openbugs/default.nix
Outdated
Show resolved
Hide resolved
pkgs/applications/science/machine-learning/openbugs/default.nix
Outdated
Show resolved
Hide resolved
pkgs/applications/science/machine-learning/openbugs/default.nix
Outdated
Show resolved
Hide resolved
@ofborg build openbugs |
Thanks for the contribution! |
Backport failed for Please cherry-pick the changes locally. git fetch origin release-22.11
git worktree add -d .worktree/backport-233506-to-release-22.11 origin/release-22.11
cd .worktree/backport-233506-to-release-22.11
git checkout -b backport-233506-to-release-22.11
ancref=$(git merge-base 05c30cbde364894fa3a774fb62e78b96b9a93f0f cfbff1faffc96af3dc4223fa12f6f4da6077decc)
git cherry-pick -x $ancref..cfbff1faffc96af3dc4223fa12f6f4da6077decc |
Successfully created backport PR for |
Don't worry about the 22.11 backport. It only has a month or so of support left anyway.
|
Awesome! Thank you very much. |
Description of changes
Uploaded a new package calles OpenBUGS and added myself as a maintainer.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)