-
-
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
julia_13: fix julia_13 missing Lib of lapack. #89270
Conversation
Tests still fail on OfBorg. I'm a little leery of disabling tests in a PR that doesn't include that in the title. |
cc @twhitehead who was working on the Julia tests in #79174 for thoughts on this disabling of tests. |
yeh, we had a discussing in julia_13 PR. #79174 (comment) about disable the test file. In expected testing that is
but, Until this time the testing problem still in here. that is why the Julia didn't have binary cache in hydra CI.
|
@drewrisinger my own hydra CI test for julia_13 at here. http://221.4.35.244:8300/build/6#tabs-summary |
The OfBorg evaluation of this PR with tests fails: https://github.com/NixOS/nixpkgs/pull/89270/checks?check_run_id=725656788. Looks OfBorg build was against julia, not julia_13. Is that the one you meant to build/fix? |
looks like title should be changed to |
]; | ||
|
||
enableParallelBuilding = true; | ||
|
||
doCheck = !stdenv.isDarwin; | ||
checkTarget = "testall"; | ||
checkTarget = ""; |
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.
From your hydra log, it looks like this line just disables all tests. If that's what you want to do, you should change this to doCheck = false
, vs obfuscating this by setting checkTarget = ""
.
I'm not comfortable approving anything that disables ALL tests, especially for something as big as a programming language.
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.
thanks told me the reason. I will reset hard to HEAD~1. Only adding dependence in this PR.
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.
last build on hydra test
@GrahamcOfBorg eval |
@drewrisinger sorry for the delay in following up with the prior one. I've followed opened up the following upstream bug reports with respect to a variety of issues that are tripping up in nix
I also discovered you need to set HOME and USER to something reasonable or a bunch of tests break preCheck = ''
HOME=$NIX_BUILD_TOP
USER=$(whoami)
JULIA_CPU_THREADS=''${NIX_BUILD_CORES:-1}
'' |
@twhitehead thanks for the response, I think fixing Julia tests is now out of scope for this PR, though I think it's still very important. |
The underlying issue is that the build runs multiple tests in a single julia instance and not all tests leave behind a clean state (i.e., some of the earlier tests break things for later tests). Why this is showing up under nix and not most other systems is because building without a network connection causes all the test to run serially in one julia process (where all bad test interactions surface) compared to the standard way of running tests in parallel across multiple julia process (where bad test interactions only surface if they happen to occur in the same julia process). So, yeah, the Julia test process as it stands is reasonably broken. At least now upstream has a reproducible way to trigger it (test with only one process) and will hopefully get stuff cleaned up. |
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.
Diff LGTM.
With nix-review pr 89270
, doesn't build. Seems to be b/c of upstream test failures. However, this is a strict improvement and it would likely build if tests disabled. See comments in thread for status of Julia testing in Nix @twhitehead.
Fixes fallout from #83888
@twhitehead. @cstich I bumped up julia_13 to julia1.4.2. also, I did a build test and patched you have mentioned code on hydra CI. there are my test logs.http://221.4.35.244:8300/build/36/nixlog/1 |
Motivation for this change
lapck
. Regarding comments issue in julia_13: The lapackProvider couldn't link liblapack.so #89155checkTarget
tonone
. let Julia passing the build process.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)