Add a script in contrib to download a cmake binary #19632

tkelman · 2016-12-17T05:22:53Z

will help on Ubuntu 14.04 and other distros when we upgrade to a
version of LLVM that requires cmake 3.4.3 or newer (#19123)

probably more useful on linux than on mac, since I assume most people on mac are getting cmake from homebrew

will help on Ubuntu 14.04 and other distros when we upgrade to a version of LLVM that requires cmake 3.4.3 or newer

staticfloat · 2016-12-19T08:45:40Z

contrib/download_cmake.sh

+ exit 1;;
+esac
+
+deps/tools/jldownload https://cmake.org/files/v$CMAKE_VERSION_MAJMIN/$FULLNAME.tar.gz


I've added this to the whitelist on cache.julialang.org and verified it works with this PR.

staticfloat · 2016-12-19T08:47:55Z

contrib/download_cmake.sh

+deps/tools/jldownload https://cmake.org/files/v$CMAKE_VERSION_MAJMIN/$FULLNAME.tar.gz
+echo "$CMAKE_SHA256 $FULLNAME.tar.gz" | sha256sum -c -
+tar -xzf $FULLNAME.tar.gz
+echo "CMAKE = $PWD/$CMAKE_EXTRACTED_PATH" >> Make.user


This makes me ever so slightly uneasy; usually we don't touch Make.user at all. I'm not fundamentally against it, but I could see us deciding this is a bad idea in the future. Also, if we do do this, I usually use override for anything in Make.user.

It'd be nice to auto-set CMAKE to our possible installation downloaded paths if they exist, since they are easily calculable, and could still be easily overwritten from Make.user, so we wouldn't lose any flexibility if the user for whatever reason didn't want to use our downloaded CMAKE.

The CMAKE variable is specifically initialized with CMAKE ?= cmake so it doesn't need override.

I'd much rather do it this way then either adding the folder to the path or replacing the CMAKE ?= cmake default with anything far more complicated that has to check the filesystem.

It's not so complicated, it's a simple check for the existence of the cmake binary at one location per platform. I don't think we should have our build system provide automated helper functions that rely on overrides placed in locations that are usually only user-specified. This is not so different from our handling of patchelf, only instead of relying on a USE_SYSTEM_XYZ make variable, we first check for the existence of a file.

In short, I feel that if we are going to provide a helper function to download a tool, we should be able to automatically find that tool once it's been downloaded by our helper script.

Users won't be prompted to run download_cmake.sh unless there is a problem during llvm cmake build time, right? And even if users do download this cmake and it gets used during build time, that shouldn't introduce new problems, should it?

it actually could cause problems if a different cmake gets used to try to rebuild one of the other libraries, libgit2 or mbedtls etc that aren't as picky about version requirements

That's no different from any other kind of build dependency that changes in between builds, though. For bonus points, we could clean every cmake-using dependency when we run download_cmake.sh.

should I take the approval as "good enough for now" ?

Thinking about it a bit more, I guess where your way might make more sense is if we also want to use this on the buildbots. Is that any easier than your current scripts that are compiling it from source? We could drop back to 3.6.3 which is the last version of cmake where they provided 32 bit linux binaries.

staticfloat · 2016-12-19T08:49:43Z

contrib/download_cmake.sh

+esac
+
+deps/tools/jldownload https://cmake.org/files/v$CMAKE_VERSION_MAJMIN/$FULLNAME.tar.gz
+echo "$CMAKE_SHA256 $FULLNAME.tar.gz" | sha256sum -c -


Unfortunately, sha256sum doesn't exist on OSX/BSD, it's accessed via shasum -a 256. See this part of jlchecksum to see what I mean.

I've created a PR against your PR (man I really love git) to illustrate how I would use jlchecksum to do this more easily/portably.

This script isn't all that valuable on mac. It's been discussed elsewhere that we should probably move to using cmake itself for checksumming in a can-rely-on-sha512-everywhere way. So I'm avoiding jlchecksum with the expectation that it may be rewritten in a way that relies on cmake, giving a bootstrapping issue here.

Great, didn't know that cmake can do checksumming by itself, we should definitely move toward that then

staticfloat · 2016-12-19T09:05:00Z

contrib/download_cmake.sh

+# Script to download newest version of cmake on linux (or mac)
+# saves you the trouble of compiling it if you don't have root
+set -e # stop on failure
+cd "$(dirname "$0")"/.. # run in top-level directory


Can we squirrel away both the downloaded tarball and the unpacked directory within deps/ somewhere? I'm not a fan of having it laying around in the top-level directory.

Why not? This isn't being built from source like every single other dependency, it's downloading a binary.

It seems messy to me, but that's an aesthetic choice. This doesn't seem too conceptually different to me than downloading source and compiling the binary to use it during build time, like we do with e.g. patchelf, but this isn't a strong objection. At the very least, we should add .gitignore rules for wherever this does end up.

staticfloat · 2016-12-22T08:06:06Z

Yes. :)

…

On Wed, Dec 21, 2016 at 11:19 PM, Tony Kelman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In contrib/download_cmake.sh <#19632>: > + Darwin-x86_64) + CMAKE_SHA256=$CMAKE_SHA256_DARWIN + CMAKE_EXTRACTED_PATH=$FULLNAME/CMake.app/Contents/bin/cmake;; + Linux-x86_64) + CMAKE_SHA256=$CMAKE_SHA256_LINUX + CMAKE_EXTRACTED_PATH=$FULLNAME/bin/cmake;; + *) + echo "This script only supports x86_64 Mac and Linux. For other platforms," >&2 + echo "get cmake from your package manager or compile it from source." >&2 + exit 1;; +esac + +deps/tools/jldownload https://cmake.org/files/v$CMAKE_VERSION_MAJMIN/$FULLNAME.tar.gz +echo "$CMAKE_SHA256 $FULLNAME.tar.gz" | sha256sum -c - +tar -xzf $FULLNAME.tar.gz +echo "CMAKE = $PWD/$CMAKE_EXTRACTED_PATH" >> Make.user should I take the approval as "good enough for now" ? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#19632>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAH_aO47shiznqAUz2MdpP8sM6lnCiaiks5rKiR0gaJpZM4LPyqp> .

staticfloat · 2016-12-22T09:54:16Z

The from-source build is pretty painless. No sweat. -E

…

On Thu, Dec 22, 2016 at 1:25 AM, Tony Kelman ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In contrib/download_cmake.sh <#19632>: > + Darwin-x86_64) + CMAKE_SHA256=$CMAKE_SHA256_DARWIN + CMAKE_EXTRACTED_PATH=$FULLNAME/CMake.app/Contents/bin/cmake;; + Linux-x86_64) + CMAKE_SHA256=$CMAKE_SHA256_LINUX + CMAKE_EXTRACTED_PATH=$FULLNAME/bin/cmake;; + *) + echo "This script only supports x86_64 Mac and Linux. For other platforms," >&2 + echo "get cmake from your package manager or compile it from source." >&2 + exit 1;; +esac + +deps/tools/jldownload https://cmake.org/files/v$CMAKE_VERSION_MAJMIN/$FULLNAME.tar.gz +echo "$CMAKE_SHA256 $FULLNAME.tar.gz" | sha256sum -c - +tar -xzf $FULLNAME.tar.gz +echo "CMAKE = $PWD/$CMAKE_EXTRACTED_PATH" >> Make.user Thinking about it a bit more, I guess where your way might make more sense is if we also want to use this on the buildbots. Is that any easier than your current scripts that are compiling it from source? We could drop back to 3.6.3 which is the last version of cmake where they provided 32 bit linux binaries. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#19632>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAH_aJY_APk5HcFsN4LO0bWtjrhUjZhEks5rKkH9gaJpZM4LPyqp> .

Add a script in contrib to download a cmake binary

5700d37

will help on Ubuntu 14.04 and other distros when we upgrade to a version of LLVM that requires cmake 3.4.3 or newer

tkelman added the domain:building Build system, or building Julia or its dependencies label Dec 17, 2016

tkelman mentioned this pull request Dec 17, 2016

Upgrade to LLVM 3.9 #19123

Closed

tkelman requested a review from staticfloat December 19, 2016 00:52

staticfloat requested changes Dec 19, 2016

View reviewed changes

staticfloat mentioned this pull request Dec 19, 2016

Use jlchecksum machinery to deal with checksums #19644

Closed

Run in deps/scratch, fix shasum invocation on darwin

5832f81

tkelman force-pushed the tk/download_cmake branch from c77891c to 5832f81 Compare December 20, 2016 05:48

staticfloat approved these changes Dec 20, 2016

View reviewed changes

tkelman merged commit 387e964 into master Dec 22, 2016

tkelman deleted the tk/download_cmake branch December 22, 2016 08:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a script in contrib to download a cmake binary #19632

Add a script in contrib to download a cmake binary #19632

tkelman commented Dec 17, 2016 •

edited

Loading

staticfloat Dec 19, 2016

staticfloat Dec 19, 2016

staticfloat Dec 19, 2016

tkelman Dec 19, 2016

staticfloat Dec 19, 2016

staticfloat Dec 19, 2016

staticfloat Dec 19, 2016

tkelman Dec 19, 2016

staticfloat Dec 19, 2016

tkelman Dec 22, 2016

tkelman Dec 22, 2016

staticfloat Dec 19, 2016

staticfloat Dec 19, 2016

tkelman Dec 19, 2016

staticfloat Dec 19, 2016

staticfloat Dec 19, 2016

tkelman Dec 19, 2016

staticfloat Dec 19, 2016

staticfloat commented Dec 22, 2016 via email

staticfloat commented Dec 22, 2016 via email

Add a script in contrib to download a cmake binary #19632

Add a script in contrib to download a cmake binary #19632

Conversation

tkelman commented Dec 17, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

staticfloat commented Dec 22, 2016 via email

staticfloat commented Dec 22, 2016 via email

tkelman commented Dec 17, 2016 •

edited

Loading