Skip to content

ci: run Bazel earlier#7732

Merged
vvbandeira merged 4 commits into
The-OpenROAD-Project:masterfrom
vvbandeira:bazel-early
Jul 7, 2025
Merged

ci: run Bazel earlier#7732
vvbandeira merged 4 commits into
The-OpenROAD-Project:masterfrom
vvbandeira:bazel-early

Conversation

@vvbandeira
Copy link
Copy Markdown
Member

@vvbandeira vvbandeira commented Jul 6, 2025

Approx. 10 min TAT for Bazel: ~2-3 minutes for setup and ~7-8 minutes to run tests w/cache enabled.

@vvbandeira vvbandeira marked this pull request as draft July 6, 2025 18:51
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 6, 2025

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 6, 2025

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 6, 2025

clang-tidy review says "All clean, LGTM! 👍"

@vvbandeira
Copy link
Copy Markdown
Member Author

@oharboe
Not ready yet, but Bazel would run in parallel with Docker with this PR. Was this what you had in mind with #7592?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 6, 2025

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 6, 2025

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 6, 2025

clang-tidy review says "All clean, LGTM! 👍"

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 6, 2025

clang-tidy review says "All clean, LGTM! 👍"

1 similar comment
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 6, 2025

clang-tidy review says "All clean, LGTM! 👍"

Signed-off-by: Vitor Bandeira <vvbandeira@precisioninno.com>
Signed-off-by: Vitor Bandeira <vvbandeira@precisioninno.com>
Signed-off-by: Vitor Bandeira <vvbandeira@precisioninno.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 6, 2025

clang-tidy review says "All clean, LGTM! 👍"

Comment thread Jenkinsfile Outdated
Comment thread Jenkinsfile
node {
stage('Setup Bazel Build') {
checkout scm;
sh label: 'Setup Docker Image', script: 'docker build -f docker/Dockerfile.bazel -t openroad/bazel-ci .';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Don't build docker image here, use prebuilt image. This will cut down the time it takes to start a docker build with several minutes. Added as a task to #7568

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In our CI, this takes less than 1 min. The last three builds were 57s, 47s, and 44s. We can use prebuilt, but I don't think it is a high priority. The breakdown of the runtime is ~1 min to provision the VM, ~2 min to checkout the code (this can be improved with shallow clone, I will give it a try as is low effort), ~1 min to build the image and 7+min to run the bazelisk test [...]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I guess we'll whittle away at the things that move the needle and then eventually this sort of thing would come to the top of the list... That these quicker things start to matter in terms of time to failed CI and successful Bazel build is a good problem to have.

Comment thread docker/Dockerfile.bazel

RUN curl -Lo bazelisk https://github.com/bazelbuild/bazelisk/releases/latest/download/bazelisk-linux-amd64 \
&& chmod +x bazelisk \
&& mv bazelisk /usr/local/bin/bazelisk
Copy link
Copy Markdown
Collaborator

@oharboe oharboe Jul 7, 2025

Choose a reason for hiding this comment

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

Refinement for later: download .bazelversion here. #7568

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

How often do you see this being updated? If it is somewhat often, I would say to wait until we have the prebuilt image set up to automatically update; otherwise, it will require manual steps to update, which is not ideal.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Low priority, listed more as an observation here.

Comment thread docker/Dockerfile.bazel
&& apt-get -y install --no-install-recommends \
build-essential \
clang \
containerd.io \
Copy link
Copy Markdown
Collaborator

@oharboe oharboe Jul 7, 2025

Choose a reason for hiding this comment

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

We should be building within seconds and minutes for bazel, so small things will matter...

Refinement for later: simplify docker image. #7568

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

All the current packages are required to run the tests. I iterated over adding one package at a time and checking the error from bazelisk test [...]. Unless we can move the dependencies into Bazel itself, these are the minimal requirements as far as I can tell.

@oharboe oharboe self-requested a review July 7, 2025 07:32
Copy link
Copy Markdown
Collaborator

@oharboe oharboe left a comment

Choose a reason for hiding this comment

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

Merge: excellent starting point for further discussion, improvements and priortirization.

Signed-off-by: Vitor Bandeira <vvbandeira@precisioninno.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 7, 2025

clang-tidy review says "All clean, LGTM! 👍"

@vvbandeira vvbandeira disabled auto-merge July 7, 2025 14:25
@vvbandeira vvbandeira merged commit 9756c46 into The-OpenROAD-Project:master Jul 7, 2025
11 checks passed
@vvbandeira vvbandeira deleted the bazel-early branch July 7, 2025 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants