Skip to content

Use apt-get to install Clang on CI #6345

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.

Sign up for GitHub

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

Merged
merged 27 commits into from
Aug 30, 2022
Merged

Use apt-get to install Clang on CI #6345

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
7e846c8
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
8d87fd8
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
a3a5e38
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
d34e490
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
8dafd57
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
a028757
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
c52073d
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
f39b33a
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
84c96bb
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
469c1e1
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
9a9b1f0
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
7618b1d
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
397052c
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
c349518
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
b0ecc57
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
87821c4
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
e768fb9
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
f82796b
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 28, 2022
bc521ab
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 29, 2022
a54221a
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 29, 2022
ddeb733
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 29, 2022
31fbbed
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 29, 2022
58a5592
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 29, 2022
3f5b19b
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 29, 2022
6bea064
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 29, 2022
61eb847
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 29, 2022
2790ed0
Use apt-get to install Clang on CI
SiarheiFedartsou Aug 30, 2022
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
45 changes: 28 additions & 17 deletions .github/workflows/osrm-backend.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ jobs:
runs-on: ubuntu-20.04
BUILD_TOOLS: ON
BUILD_TYPE: Debug
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
Copy link
Member Author

Choose a reason for hiding this comment

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

We could just use CCOMPILER/CXXCOMPILER to use pre-installed clang, but the lowest pre-installed version is only 10: https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2004-Readme.md

CUCUMBER_TIMEOUT: 60000

- name: clang-11.0-debug-clang-tidy
Expand All @@ -147,7 +147,7 @@ jobs:
runs-on: ubuntu-20.04
BUILD_TOOLS: ON
BUILD_TYPE: Debug
CLANG_VERSION: 11.0.0
CLANG_VERSION: 11
CUCUMBER_TIMEOUT: 60000
ENABLE_CLANG_TIDY: ON

Expand All @@ -157,7 +157,7 @@ jobs:
runs-on: ubuntu-20.04
BUILD_TOOLS: ON
BUILD_TYPE: Release
CLANG_VERSION: 11.0.0
CLANG_VERSION: 11
Copy link
Member Author

Choose a reason for hiding this comment

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

Do we have any obligations to support certain old compilers btw?

Copy link
Member

Choose a reason for hiding this comment

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

Ideally for release builds we want to use a compiler that supports the minimum requirements of the oldest Node platform in our matrix. This is to avoid any ABI/stdlib incompatibilities with the host environments (example: #4567)

This is the toolchain info for Node 12: https://github.com/nodejs/node/blob/v12.x/BUILDING.md

ENABLE_CONAN: ON
ENABLE_SANITIZER: ON

Expand All @@ -167,7 +167,7 @@ jobs:
runs-on: ubuntu-20.04
BUILD_TOOLS: ON
BUILD_TYPE: Release
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
ENABLE_CONAN: ON

- name: gcc-11-release
Expand Down Expand Up @@ -285,7 +285,7 @@ jobs:
node: 12
runs-on: ubuntu-20.04
BUILD_TYPE: Release
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
Expand All @@ -296,7 +296,7 @@ jobs:
node: 12
runs-on: ubuntu-20.04
BUILD_TYPE: Debug
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
Expand All @@ -307,7 +307,7 @@ jobs:
node: 14
runs-on: ubuntu-20.04
BUILD_TYPE: Release
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
Expand All @@ -318,7 +318,7 @@ jobs:
node: 14
runs-on: ubuntu-20.04
BUILD_TYPE: Debug
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
Expand All @@ -330,7 +330,7 @@ jobs:
node: 16
runs-on: ubuntu-20.04
BUILD_TYPE: Release
CLANG_VERSION: 6.0.0
Copy link
Member

Choose a reason for hiding this comment

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

I think all references to CLANG_VERSION can be removed now?

CLANG_VERSION: 6
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
Expand All @@ -341,7 +341,7 @@ jobs:
node: 16
runs-on: ubuntu-20.04
BUILD_TYPE: Debug
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
Expand All @@ -364,7 +364,7 @@ jobs:
node: latest
runs-on: ubuntu-20.04
BUILD_TYPE: Release
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
Expand All @@ -375,7 +375,7 @@ jobs:
node: latest
runs-on: ubuntu-20.04
BUILD_TYPE: Debug
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
Expand All @@ -398,7 +398,7 @@ jobs:
node: "lts/*"
runs-on: ubuntu-20.04
BUILD_TYPE: Release
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
Expand All @@ -409,7 +409,7 @@ jobs:
node: "lts/*"
runs-on: ubuntu-20.04
BUILD_TYPE: Debug
CLANG_VERSION: 6.0.0
CLANG_VERSION: 6
ENABLE_GLIBC_WORKAROUND: ON
ENABLE_CONAN: ON
NODE_PACKAGE_TESTS_ONLY: ON
Expand Down Expand Up @@ -518,8 +518,14 @@ jobs:
if [[ -n ${CLANG_VERSION} ]]; then
echo "CCOMPILER=clang" >> $GITHUB_ENV
echo "CXXCOMPILER=clang++" >> $GITHUB_ENV
${MASON} install clang++ ${CLANG_VERSION}
echo "$(${MASON} prefix clang++ ${CLANG_VERSION})/bin" >> $GITHUB_PATH
sudo apt-get update -y && sudo apt-get install "clang++-${CLANG_VERSION}"
if [[ "${CLANG_VERSION}" = "6" ]]; then
Copy link
Member Author

Choose a reason for hiding this comment

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

Clang 6 is installed as /usr/bin/clang++-6.0 and Clang 11 as /usr/bin/clang++-11, so had to do this.
It is a bit ugly, but couldn't find better way.

Copy link
Member

Choose a reason for hiding this comment

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

Is this the same package as https://packages.ubuntu.com/focal/clang-6.0?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so.

Copy link
Member Author

Choose a reason for hiding this comment

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

Screenshot 2022-08-29 at 17 42 08

Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to set CCOMPILER/CXXCOMPILER to e.g. clang-6.0/clang++6.0 and then just install all compilers at L543? Or is there a step that's requiring the symlink?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will try

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems it works, a way more elegant solution, thanks 👍

sudo update-alternatives --install /usr/bin/clang++ clang++ "/usr/bin/clang++-${CLANG_VERSION}.0" 10000000
sudo update-alternatives --install /usr/bin/clang clang "/usr/bin/clang-${CLANG_VERSION}.0" 10000000
else
sudo update-alternatives --install /usr/bin/clang++ clang++ "/usr/bin/clang++-${CLANG_VERSION}" 10000000
sudo update-alternatives --install /usr/bin/clang clang "/usr/bin/clang-${CLANG_VERSION}" 10000000
fi
# we only enable lto for release builds
# and therefore don't need to us ld.gold or llvm tools for linking
# for debug builds
Expand All @@ -528,7 +534,6 @@ jobs:
echo "$(${MASON} prefix binutils 2.27)/bin" >> $GITHUB_PATH
fi
fi

# Linux dev packages
if [ "${TARGET_ARCH}" != "i686" ] && [ "${ENABLE_CONAN}" != "ON" ]; then
sudo add-apt-repository ppa:ubuntu-toolchain-r/test -y
Expand Down Expand Up @@ -556,6 +561,12 @@ jobs:
tar zxvf onetbb.tgz
sudo cp -a oneapi-tbb-${TBB_VERSION}/lib/. /usr/local/lib/
sudo cp -a oneapi-tbb-${TBB_VERSION}/include/. /usr/local/include/
- name: Check clang version
Copy link
Member Author

Choose a reason for hiding this comment

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

Kind of self-check that actual clang version is the same as expected.

run: |
if [[ -n ${CLANG_VERSION} ]]; then
clang --version | grep "${CLANG_VERSION}"
clang++ --version | grep "${CLANG_VERSION}"
fi

- name: Prepare build
run: |
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
- FIXED: Bug in bicycle profile that caused exceptions if there is a highway=bicycle in the data. [#6296](https://github.com/Project-OSRM/osrm-backend/pull/6296)
- FIXED: Internal refactoring of identifier types used in data facade [#6044](https://github.com/Project-OSRM/osrm-backend/pull/6044)
- Build:
- CHANGED: Use apt-get to install Clang on CI. [#6345](https://github.com/Project-OSRM/osrm-backend/pull/6345)
- CHANGED: Fix TBB in case of Conan + NodeJS build. [#6333](https://github.com/Project-OSRM/osrm-backend/pull/6333)
- CHANGED: Migrate to modern TBB version. [#6300](https://github.com/Project-OSRM/osrm-backend/pull/6300)
- CHANGED: Enable performance-move-const-arg clang-tidy check. [#6319](https://github.com/Project-OSRM/osrm-backend/pull/6319)
Expand Down