-
Notifications
You must be signed in to change notification settings - Fork 37
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
add gcc and binutils packages #60
add gcc and binutils packages #60
Conversation
…oaded are too big for their service
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.
Looks great! Thanks for putting this together :) A few pretty minor comments
I think @stuhood was saying we're going to stop checking in binaries at all, which hopefully should make your pushing large files problem go away...
build-binutils-2.30.sh
Outdated
mkdir -p "$BINUTILS_BUILD_TMP_DIR" | ||
pushd "$BINUTILS_BUILD_TMP_DIR" # root -> $BINUTILS_BUILD_TMP_DIR | ||
|
||
curl -L -v -O "$BINUTILS_RELEASE_URL" |
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.
Throw in a --fail
, and throughout the PR (which I really, really wish was the default)
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.
Done!
build-clang-6.0.0.sh
Outdated
@@ -0,0 +1,160 @@ | |||
#!/bin/bash |
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.
It would be nice to re-use the same script for 5.0.1 and 6.0.0, given the differences seem to be:
- Actual version numbers
- A 'final' being present or absent in a string
- Extracting two extra directories
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 agree, just wasn't sure if that was something people would find useful. This works for clang 5.0.1 and 6.0.0 now using e.g. ./build-clang.sh osx 5.0.1
(or it worked when I last tried running it, will focus on correctness later today).
build-gcc-7.3.0.sh
Outdated
--build="x86_64-apple-darwin${_osx_uname_release}" | ||
--host="x86_64-apple-darwin${_osx_uname_release}" | ||
--target="x86_64-apple-darwin${_osx_uname_release}" | ||
--with-system-zlib |
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.
Is statically linking zlib an option? Fewer system deps are better imo...
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.
Agreed, that was ripped from the homebrew recipe without further thought. I have since tried investigating how difficult it would be to build all of these from the CentOS docker image, which changed these again (this file is now build-gcc.sh
).
build-gcc-7.3.0.sh
Outdated
## Some functions, despite bash making it unnecessarily hard to use them. | ||
|
||
# Check if a directory exists, and error out or return its absolute | ||
# path. Passing -f allows testing a file as well. |
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.
-e
just tests "exists" - is -d
vs -f
actually a distinction you care about?
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.
Nope! And random meaningless arguments in shell scripts get confusing pretty fast, so a great point to note. See get_existing_absolute_path
in utils.bash
for what this function ended up turning into.
Changed everything a lot, now we have Each new script (and functions within those scripts) echoes some single distinct "product" to stdout upon completion, and tries to work within its own subdirectory whenever possible. This idiom may not be necessary but seemed to be make this type of script easier to write and follow. The idea with this is 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.
This is genuinely some of the best crafted bash I've ever seen. Kudos!
I haven't run this, but I'm happy with it assuming it runs. Handing over to @stuhood for how things should look in a no-binaries-committed world :)
|
||
## Interpret arguments and execute build. | ||
|
||
# $1 = |
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.
Delete?
local -r path_arg="$1" | ||
# -f is an "illegal option" for OSX readlink, so this may have e.g. `/../` | ||
# within it. | ||
local -r abs_path="$(pwd)/${path_arg}" |
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.
Nit: Check that path_arg is relative - if it's absolute, we don't want to double-absolutify it :)
|
||
function var_is_set { | ||
local -r var_name="$1" | ||
[[ "${!var_name+x}" != '' ]] |
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.
What's the +x
here?
(Also, this is perfectly fine and readable, but -z
(empty string) and -n
(non-empty) are handy tests to know about :))
gcc (like clang) is being provided for both OSX and Linux. binutils does not support OSX (and I've tried to change that, believe me), so the linker will stay an unsolved problem for now. The scripts for all 3 of these tools will mkdir -p the appropriate output directories and copy the packaged archive over to
build-support/bin/${tool}/${arch}/${version}/${tool}.tar.gz
(and also, all are being provided as tgz archives -- BinaryTool is great!).The packages produced by these scripts have been used to successfully compile and link code in an extremely bare Linux environment. The same level of testing has not been done with OSX, but I have done my best to make as much of the script as cross-platform as possible so the only feature gaps are the ones in the software we're depending on.
Be forewarned that gcc can take literal days to build unless you dial up the parallelism (using
MAKE_JOBS
), but that you can't do that on OSX due to race conditions (unclear why, but it has always failed when I've tried).Let me know if there's a ton more work to be done here or if these look alright. Obviously these packages are big, and they are also hard to slim down -- that's not saying it's impossible.
Also, here is my pants PR which consumes these subsystems. Please let me know if there are any challenges to getting either of these PRs merged that I haven't considered. Thanks!
I know the history is all screwy, github stopped letting me upload files > 100MB to this repo (even though I'd successfully uploaded them before e.g. clang?) and I'm not sure why. I can spend some effort making the version history cleaner to merge if necessary, especially for a repo with huge binaries where everything can be slow sometimes.