Skip to content
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

[New] Use Clang as C/C++ compiler if we detected it, close #902 #1300

Merged
merged 3 commits into from
Nov 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ before_install:
- $SHELL --version 2> /dev/null || dpkg -s $SHELL 2> /dev/null || which $SHELL
- curl --version
- wget --version
- clang --version
- clang++ --version
- if [ -n "${SHELLCHECK-}" ]; then sudo apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys 575159689BEFB442 && echo 'deb http://download.fpcomplete.com/ubuntu precise main' | sudo tee /etc/apt/sources.list.d/fpco.list && sudo apt-get update && sudo apt-get install stack bc -y && stack setup && stack install ShellCheck && shellcheck --version ; fi
- if [ -z "${SHELLCHECK-}" ]; then wget -O - http://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add - && echo -e "deb http://apt.llvm.org/precise/ llvm-toolchain-precise main\ndeb http://apt.llvm.org/precise/ llvm-toolchain-precise-3.8 main" | sudo tee /etc/apt/sources.list.d/clang.list && sudo apt-get update && sudo apt-get install clang-3.8 lldb-3.8 -y --force-yes && sudo ln -sf /usr/bin/clang-3.8 /usr/bin/clang && sudo ln -sf /usr/bin/clang++-3.8 /usr/bin/clang++ && clang --version ; fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these not things that can be installed by travis in the "addons: apt" section above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang in Ubuntu 12.04 is too old ( < v3.5)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this installation process be cached, like the shellcheck build is?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, do we have cache on the apt packages?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this only cache the ~/.stack, not the package in system level, am I right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shellcheck build is very very slow, and once the first one was built, it didn't need to be rebuilt on successive builds.

I want to ensure that we don't need to build clang more than the once.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We install clang from binary, no build needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah ok, that's good. is caching the download worth it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, as the time will be mainly used to solve the dependencies, not on the package downloading.

install:
- (mkdir /tmp/urchin && cd /tmp/urchin && curl -s "$(curl -s https://registry.npmjs.com/urchin | grep -Eo '"tarball":\s*"[^"]+"' | tail -n 1 | awk -F\" '{ print $4 }')" -O && tar -x -f urchin*)
- chmod +x /tmp/urchin/package/urchin
Expand Down
10 changes: 9 additions & 1 deletion nvm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ nvm_rc_version() {
fi
}

nvm_clang_version() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add some unit tests for this

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ljharb where should I put the test script? Thanks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test/fast/Unit Tests

clang --version | command awk '{ if ($2 == "version") print $3; else if ($3 == "version") print $4 }' | command sed 's/-.*$//g'
}

nvm_version_greater() {
command awk 'BEGIN {
if (ARGV[1] == "" || ARGV[2] == "") exit(1)
Expand Down Expand Up @@ -1854,6 +1858,10 @@ nvm_install_source() {
elif [ "${NVM_OS}" = 'aix' ]; then
make='gmake'
fi
if nvm_has "clang++" && nvm_has "clang" && nvm_version_greater_than_or_equal_to nvm_clang_version 3.5 ; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are both clang and clang++ needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm_echo "Clang v3.5+ detected! Use Clang as c/c++ compiler!"
MAKE_CXX='CC=clang CXX=clang++'
fi

local tar_compression_flag
tar_compression_flag='z'
Expand Down Expand Up @@ -3141,7 +3149,7 @@ nvm() {
nvm_is_iojs_version nvm_is_alias \
nvm_ls_remote nvm_ls_remote_iojs nvm_ls_remote_index_tab \
nvm_ls nvm_remote_version nvm_remote_versions \
nvm_install_binary \
nvm_install_binary nvm_clang_version \
nvm_get_mirror nvm_get_download_slug nvm_download_artifact \
nvm_install_source nvm_check_file_permissions \
nvm_print_versions nvm_compute_checksum nvm_checksum \
Expand Down
62 changes: 62 additions & 0 deletions test/fast/Unit tests/nvm_clang_version
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
#!/bin/sh

cleanup () {
unset -f die
}

die () { echo -e "$@" ; cleanup ; exit 1; }

if type "clang" > /dev/null 2>&1 ; then
clang_exec="$(type "clang")"
sudo rm -rf "${clang_exec}"
fi
if type "clang++" > /dev/null 2>&1 ; then
clangxx_exec="$(type "clang++")"
sudo rm -rf "${clangxx_exec}"
fi

NVM_ENV=testing \. ../../../nvm.sh

clang() {
if [ "$1" = "--version" ]; then
echo "${VERSION_MESSAGE}"
fi
}

assert_version_is() {
if [ "${1}" != "${2}" ]; then
die "Expected ${2}, got ${1}, origin version message:\n${VERSION_MESSAGE}"
return 1
fi
}

CLANG_VERSION_ON_DEBIAN_JESSIE="Debian clang version 3.5.0-10 (tags/RELEASE_350/final) (based on LLVM 3.5.0)
Target: x86_64-pc-linux-gnu
Thread model: posix"

CLANG_VERSION_ON_UBUNTU_TRUSTY="Ubuntu clang version 3.4-1ubuntu3 (tags/RELEASE_34/final) (based on LLVM 3.4)
Target: x86_64-pc-linux-gnu
Thread model: posix"

CLANG_VERSION_ON_ARCHLINUX="clang version 3.9.0 (tags/RELEASE_390/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/sbin"

CLANG_VERSION_ON_FREEBSD="FreeBSD clang version 3.4.1 (tags/RELEASE_34/dot1-final 208032) 20140512
Target: x86_64-unknown-freebsd10.3
Thread model: posix"

VERSION_MESSAGE="${CLANG_VERSION_ON_DEBIAN_JESSIE}"
assert_version_is "$(nvm_clang_version)" "3.5.0"

VERSION_MESSAGE="${CLANG_VERSION_ON_UBUNTU_TRUSTY}"
assert_version_is "$(nvm_clang_version)" "3.4"

VERSION_MESSAGE="${CLANG_VERSION_ON_ARCHLINUX}"
assert_version_is "$(nvm_clang_version)" "3.9.0"

VERSION_MESSAGE="${CLANG_VERSION_ON_FREEBSD}"
assert_version_is "$(nvm_clang_version)" "3.4.1"

cleanup