-
Notifications
You must be signed in to change notification settings - Fork 6.8k
julia: migrate build process to cmake #16125
base: master
Are you sure you want to change the base?
Conversation
The original build process triggered by `Pkg.add` is done by GNU make.
The original build process triggered by `Pkg.add` is done by GNU make.
3ef9cf0
to
b28b847
Compare
@@ -89,3 +90,21 @@ MARK_AS_ADVANCED( | |||
OpenBLAS | |||
) | |||
|
|||
# Check ILP64 data model for the case of Julia self-shipped `libopenblas64_.so` |
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.
Could anyone review this cmake script?
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.
I don't know much, but maybe you can simplify a bit using CHECK_SOURCE_COMPILES and similar. https://gitlab.kitware.com/cmake/community/wikis/doc/tutorials/How-To-Write-Platform-Checks
Overall looks ok-ish to me.
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.
ah, looks greate, I will try
-
CHECK_SYMBOL_EXISTS
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.
Any updates?
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'm swamped... I will try it after 1/10.
… into ib/jl-cmake-build
I think this PR is ready for review. |
@aaronmarkham This PR improves the integration of Julia's self-shipped I want to add a new build pipeline in the job |
I'm not sure. Let me socialize this a bit and someone can get back to you, but maybe not until after the holiday... |
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.
Have few suggestions/questions. Apologies if any of those are wrong/unintelligent. I don't quite understand cmake/julia but curious nonetheless.
# Otherwise we won't rebuild on an update. | ||
run(`rm -f $_libdir/libmxnet.$(Libdl.dlext)`) | ||
rm(_blddir, recursive=true, force=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.
Previously it deleted the file libmxnet.* in the libdir
Now we are deleting the entire build directory. What changed with that?
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.
well, the final result is the same -- making a fresh build, not increamental build.
This build script is simliar to Python's setup.py.
It's for creating fresh build for user.
julia/deps/build.jl
Outdated
@@ -137,7 +138,7 @@ if !libmxnet_detected | |||
|
|||
run(download_cmd(package_url, "mxnet.7z")) | |||
# this command will create the dir "usr\\lib" | |||
run(`$exe7z e mxnet.7z *\\build\\* *\\lib\\* -y -ousr\\lib`) | |||
run(`$exe7z e mxnet.7z "*\\build\\*" "*\\lib\\*" -y -ousr\\lib`) # TODO check it works on windows or not |
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.
Does this imply we "might" break cmake build for windows? Can't we check this on windows before merging 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.
ah, I checked it on Window, but forgot to remove the comment.
The previous code just poped some warning about quoting.
Co-Authored-By: Chaitanya Prakash Bapat <chai.bapat@gmail.com>
The original build process triggered by
Pkg.build
isdone by GNU make.
TODO
libopenblas
, andINTERFACE64
issue