-
Notifications
You must be signed in to change notification settings - Fork 704
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
{compiler}[GCCcore/4.9.3] LLVM v3.8.1 #3474
{compiler}[GCCcore/4.9.3] LLVM v3.8.1 #3474
Conversation
Test report by @boegel |
@ocaisa Python 2.7 is required for LLVM, hence the failed test report, see also https://github.com/hpcugent/easybuild-easyconfigs/pull/3407/files#diff-a09113334b8d0515546f9225aab5fcf2 |
Test report by @boegel |
@boegel Looks like Travis doesn't support |
@ocaisa this has nothing to do with Travis, it's a check in so, the robot is not aware that dependencies may get installed as hidden modules, it's a bug... |
@ocaisa also, I'm not sure if we should include |
Ok, unhiding it but leaving it in there so people are aware that that's what I think they should do |
osdependencies = [('openssl-devel', 'libssl-dev')] | ||
|
||
# We hide this by default since users should not use it in production | ||
#hidden = True |
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.
@ocaisa can you clarify why this should not be used in production in the comment?
If it's clear, I guess it makes sense to leave this in.
Also, please use --pr-commit-msg
with --update-pr
to provide meaningful commit messages; I'll look into enforcing that (only --new-pr
defaults to somewhat decent auto-derived commit messages)
configopts += "-DLLVM_INSTALL_UTILS=ON" | ||
|
||
separate_build_dir = True | ||
|
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.
@ocaisa please include a custom sanity_check_paths
like we have in https://github.com/hpcugent/easybuild-easyconfigs/blob/master/easybuild/easyconfigs/l/LLVM/LLVM-3.8.0-intel-2016a.eb
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.
Cloned
Test report by @boegel |
Test report by @boegel |
|
Test report by @boegel |
@ocaisa is the problem with Python bare on SL6 caused by a missing build dep on |
...since python may be called on its own
Test report by @boegel |
@ocaisa still failing on the missing |
since we have a bin directory
I can't reproduce this, is it possible to get the full log? I probably have a poor understanding of this but should binutils be a dep or a builddep in this case? Do I need bintuils at runtime for executables? |
Ah, I can reproduce this. I was using the default sanity check. Ok, let me take a look. |
@ocaisa binutils should be a build dep when |
@@ -28,7 +28,7 @@ dependencies = [ | |||
('ncurses', '6.0'), | |||
] | |||
|
|||
configopts = '-DBUILD_SHARED_LIBS=ON' | |||
configopts = '-DBUILD_SHARED_LIBS=ON ' |
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.
oh nice catch!
Should build now, testing... |
Built successfully, |
Test report by @boegel |
Test report by @boegel |
lgtm |
Going in, thanks @ocaisa! |
(created using
eb --new-pr
)