Skip to content

[packages] add clang-format #2999

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 10 commits into from
Apr 26, 2025
Merged

[packages] add clang-format #2999

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9d31309
[packages] add clang-format
amin1377 Apr 23, 2025
b7c1f1f
[docker] set ubuntu version to 24.04
amin1377 Apr 23, 2025
d56576e
[dockerfile] enable system-wide python package installation for pip
amin1377 Apr 24, 2025
0afdfff
[dockerfile] add comment
amin1377 Apr 24, 2025
dfc89dc
[package] check whehter clang-format-18 package exist
amin1377 Apr 24, 2025
ab3b2a9
Merge branch 'master' into add_formatting_package
amin1377 Apr 24, 2025
af7c044
[package] remove deprecated names-only option
amin1377 Apr 24, 2025
f13addc
[package] remove if condition
amin1377 Apr 24, 2025
1bd97f1
[doc] update quick start on installing packages
amin1377 Apr 24, 2025
7cdf433
[doc] clarify that clang-format is not required to build VPR
amin1377 Apr 25, 2025
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
7 changes: 5 additions & 2 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
FROM ubuntu:22.04
FROM ubuntu:24.04
ARG DEBIAN_FRONTEND=noninteractive
# set out workspace
ENV WORKSPACE=/workspace
RUN mkdir -p ${WORKSPACE}
WORKDIR ${WORKSPACE}
COPY . ${WORKSPACE}
# Required to bypass Python's protection on system-wide package installations in Ubuntu 23.04+.
# This allows pip to install packages globally without using a virtual environment.
ENV PIP_BREAK_SYSTEM_PACKAGES=1
# Install and cleanup is done in one command to minimize the build cache size
RUN apt-get update -qq \
# Extract package names from install_apt_packages.sh
Expand All @@ -29,4 +32,4 @@ RUN apt-get update -qq \
# Build VTR
RUN rm -rf build && make -j$(nproc) && make install
# Container's default launch command
SHELL ["/bin/bash", "-c"]
SHELL ["/bin/bash", "-c"]
4 changes: 4 additions & 0 deletions install_apt_packages.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,7 @@ sudo apt-get install -y \
# Required to build the documentation
sudo apt-get install -y \
sphinx-common

# Required for code formatting
sudo apt-get install -y \
clang-format-18
Comment on lines +43 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

We mention that install_apt_packages.sh works with Ubuntu 18, 20, 22 and 24 in the quick start docs, but I believe versions 18 to 22 do not have clang-format-18 in the default apt repos. You should either change the docs for this and remove support for the older versions, or alternatively add an if/else statement to the script to just not install clang-format-18 for the Ubuntu versions that don't have clang-format-18 and say that make format is only supported for Ubuntu 24.04 or newer in the docs. This way people on older Ubuntus can at least compile and use VTR with the same script, they just can't use make format.

You can use this command to check if clang-format-18 is available (it actually checks the local apt-cache but since we do a apt-get update in the script it should be up to date and reliable):

apt-cache search --names-only 'clang-format-18'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, thanks! I added the condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After discussing with @AmirhosseinPoolad , we decided to update the documentation to inform users that they may encounter issues on Ubuntu versions older than 24.04. We will also include instructions on how to resolve these issues.