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

Build fix #597

Merged
merged 1 commit into from
Oct 9, 2023
Merged

Build fix #597

merged 1 commit into from
Oct 9, 2023

Conversation

onelxj
Copy link
Contributor

@onelxj onelxj commented Jun 23, 2023

  • Memory usage by Docker (via Dazel) was causing crashes on our limited-memory GitHub Actions runners.
  • Dazel is no longer useful now that we mostly use Codespaces, so we can remove it.
  • Fix asan tool.
  • Moved to larger GitHub runners.
    Turn on address sanitizer for ubuntu runner #599

@aviator-app
Copy link

aviator-app bot commented Jun 23, 2023

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.


See the real-time status of this PR on the Aviator webapp.

@onelxj onelxj force-pushed the build branch 16 times, most recently from 3ad34ee to ea41837 Compare June 27, 2023 13:35
@onelxj onelxj requested review from benh and rjhuijsman June 27, 2023 16:13
@onelxj onelxj force-pushed the build branch 5 times, most recently from a9d0d96 to 1763b0d Compare July 4, 2023 00:54
@onelxj
Copy link
Contributor Author

onelxj commented Jul 4, 2023

Since we are using codespaces for develop, all docker (dazel) things can be removed, as well as run build using github runner config, but with config=asan off

@onelxj onelxj force-pushed the build branch 3 times, most recently from 378646c to 437423d Compare July 4, 2023 13:17
@onelxj onelxj force-pushed the build branch 13 times, most recently from 3455888 to 5af723f Compare July 5, 2023 13:03
@onelxj onelxj requested a review from rjhuijsman July 5, 2023 13:42
Copy link
Contributor

@rjhuijsman rjhuijsman left a comment

Choose a reason for hiding this comment

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

I see some more unexpected-to-me changes have popped up, so I'd like some clarification on those before I approve.

For posterity, it would be good to record the "why" of this PR in the PR description:

  • Memory usage by Docker (via Dazel) was causing crashes on our limited-memory GitHub Actions runners.
  • Dazel is no longer useful now that we mostly use Codespaces, so we can remove it.

.github/workflows/build_and_test.yml Outdated Show resolved Hide resolved
test/grpc/streaming/streaming.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@rjhuijsman rjhuijsman left a comment

Choose a reason for hiding this comment

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

Thanks! 👍🏻

Final thing: let's also link #599 in the PR description.

@onelxj onelxj force-pushed the build branch 4 times, most recently from 754f231 to 846077b Compare July 6, 2023 18:18
@benh
Copy link
Member

benh commented Aug 31, 2023

@onelxj what's the status of this? Does this fix the build?

@onelxj onelxj force-pushed the build branch 3 times, most recently from 1c4ff7f to 95bc2b2 Compare September 1, 2023 14:14
Copy link
Member

@benh benh left a comment

Choose a reason for hiding this comment

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

Let's try adding this to .bazelrc:

# By default run both (target) compilation mode and host compilation
# mode in 'fastbuild' so that any "tools" that get build (which are
# built for the host) don't require building things a second time.
build --compilation_mode=fastbuild
build --host_compilation_mode=fastbuild

@onelxj onelxj merged commit 961d2cd into 3rdparty:main Oct 9, 2023
4 checks passed
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.

3 participants