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: rework ASAN build on Actions #32352

Closed
wants to merge 1 commit into from

Conversation

mmarchini
Copy link
Contributor

@mmarchini mmarchini commented Mar 19, 2020

Changed from Debug build to Release build to avoid out of memory issues
during the link step. This should also speed up the build.

Commented out the test runner for now, since there are too many ASAN
warnings there to be useful. We should re-enable the test runner once
our ASAN warnings are fixed.

Simplify the environment: instead of running on a container, runs
directly on the host. With this, ASAN build looks identical to other
builds in GitHub Actions, with the exception of the --enable-asan
flag.

Ref: #32257
Ref: #32324

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Mar 19, 2020
@mmarchini mmarchini marked this pull request as ready for review March 19, 2020 00:02
Changed from Debug build to Release build to avoid out of memory issues
during the link step. This should also speed up the build.

Commented out the test runner for now, since there are too many ASAN
warnings there to be useful. We should re-enable the test runner once
our ASAN warnings are fixed.

Simplify the environment: instead of running on a container, runs
directly on the host. With this, ASAN build looks identical to other
builds in GitHub Actions, with the exception of the `--enable-asan`
flag.

Ref: https://github.com/nodejs/node/issue/32257
Ref: nodejs#32324
@richardlau
Copy link
Member

URL for the first Refs has a typo.

@mmarchini
Copy link
Contributor Author

Well, that's a bummer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants