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.

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

Jump to bottom

build: set ASAN workaround #43085

Closed
wants to merge 1 commit into from
Closed

build: set ASAN workaround #43085

wants to merge 1 commit into from

Conversation

richardlau
Copy link
Member

@richardlau richardlau commented May 13, 2022 •
edited
Loading

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label May 13, 2022
@richardlau richardlau marked this pull request as draft May 13, 2022 15:23
@richardlau
Copy link
Member Author

Oops, meant to open this in my fork. Shot-in-the-dark testing if the workaround in google/sanitizers#1322 makes any difference.

@gengjiawen
Copy link
Member

Looks like this works :)

@richardlau
Copy link
Member Author

Looks like this works :)

I've been rerunning the Test ASan workflow just in case the test is flaky. So far six runs for this PR haven't reproduced #43082 (the first attempt failed with #39655). I'm currently running a seventh.

Optimistically marking this as ready for review.

@richardlau richardlau marked this pull request as ready for review May 14, 2022 17:09
@gengjiawen gengjiawen added the commit-queue Add this label to land a pull request using GitHub Actions. label May 15, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels May 15, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/43085
✔  Done loading data for nodejs/node/pull/43085
----------------------------------- PR info ------------------------------------
Title      build: set ASAN workaround (#43085)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     richardlau:asan -> nodejs:master
Labels     meta
Commits    1
 - build: set ASAN workaround
Committers 1
 - Richard Lau 
PR-URL: https://github.com/nodejs/node/pull/43085
Refs: https://github.com/google/sanitizers/issues/1322
Refs: https://github.com/nodejs/node/issues/43082
Reviewed-By: Jiawen Geng 
Reviewed-By: LiviaMedeiros 
Reviewed-By: Rich Trott 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43085
Refs: https://github.com/google/sanitizers/issues/1322
Refs: https://github.com/nodejs/node/issues/43082
Reviewed-By: Jiawen Geng 
Reviewed-By: LiviaMedeiros 
Reviewed-By: Rich Trott 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 13 May 2022 15:20:31 GMT
   ✔  Approvals: 4
   ✔  - Jiawen Geng (@gengjiawen): https://github.com/nodejs/node/pull/43085#pullrequestreview-972969968
   ✔  - LiviaMedeiros (@LiviaMedeiros): https://github.com/nodejs/node/pull/43085#pullrequestreview-973087180
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/43085#pullrequestreview-973088632
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/43085#pullrequestreview-973094000
   ✖  This PR needs to wait 11 more hours to land
   ✖  Last GitHub CI failed
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/2326053002

@LiviaMedeiros LiviaMedeiros added the fast-track PRs that do not need to wait for 48 hours to land. label May 15, 2022
@github-actions
Copy link
Contributor

Fast-track has been requested by @LiviaMedeiros. Please 👍 to approve.

@Trott Trott added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels May 15, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels May 15, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/43085
✔  Done loading data for nodejs/node/pull/43085
----------------------------------- PR info ------------------------------------
Title      build: set ASAN workaround (#43085)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     richardlau:asan -> nodejs:master
Labels     meta, fast-track
Commits    1
 - build: set ASAN workaround
Committers 1
 - Richard Lau 
PR-URL: https://github.com/nodejs/node/pull/43085
Refs: https://github.com/google/sanitizers/issues/1322
Refs: https://github.com/nodejs/node/issues/43082
Reviewed-By: Jiawen Geng 
Reviewed-By: LiviaMedeiros 
Reviewed-By: Rich Trott 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43085
Refs: https://github.com/google/sanitizers/issues/1322
Refs: https://github.com/nodejs/node/issues/43082
Reviewed-By: Jiawen Geng 
Reviewed-By: LiviaMedeiros 
Reviewed-By: Rich Trott 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 13 May 2022 15:20:31 GMT
   ✔  Approvals: 4
   ✔  - Jiawen Geng (@gengjiawen): https://github.com/nodejs/node/pull/43085#pullrequestreview-972969968
   ✔  - LiviaMedeiros (@LiviaMedeiros): https://github.com/nodejs/node/pull/43085#pullrequestreview-973087180
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/43085#pullrequestreview-973088632
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/43085#pullrequestreview-973094000
   ℹ  This PR is being fast-tracked
   ✖  Last GitHub CI failed
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/2326134851

@Trott
Copy link
Member

Trott commented May 15, 2022

Landed in 347000c

Trott pushed a commit that referenced this pull request May 15, 2022
@richardlau @Trott
build: set ASAN workaround
347000c 
PR-URL: #43085
Refs: google/sanitizers#1322
Refs: #43082
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@Trott Trott closed this May 15, 2022
@Trott
Copy link
Member

Trott commented May 15, 2022

@gengjiawen Should we undo 8c800d7?

@gengjiawen
Copy link
Member

@gengjiawen Should we undo 8c800d7?

Nope, they are different issues #43092

@LiviaMedeiros LiviaMedeiros mentioned this pull request May 15, 2022
Open
@Trott
Copy link
Member

Trott commented May 15, 2022

I've been rerunning the Test ASan workflow just in case the test is flaky. So far six runs for this PR haven't reproduced #43082 (the first attempt failed with #39655). I'm currently running a seventh.

This landed on the master branch and then ASan promptly failed, but it was for different reasons, I think. It's now back to its usual timeouts and not the uid-gid problem. So that's good, I think/hope.

BethGriggs pushed a commit that referenced this pull request May 16, 2022
@richardlau @BethGriggs
build: set ASAN workaround
cec678a 
PR-URL: #43085
Refs: google/sanitizers#1322
Refs: #43082
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request May 16, 2022
Merged
@richardlau richardlau deleted the asan branch May 16, 2022 12:17
@sepehrst sepehrst mentioned this pull request May 16, 2022
Closed
juanarbol pushed a commit that referenced this pull request May 31, 2022
@richardlau @juanarbol
build: set ASAN workaround
b7d5869 
PR-URL: #43085
Refs: google/sanitizers#1322
Refs: #43082
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@juanarbol juanarbol mentioned this pull request May 31, 2022
Closed
danielleadams pushed a commit that referenced this pull request Jun 27, 2022
@richardlau @danielleadams
build: set ASAN workaround
af6a3f8 
PR-URL: #43085
Refs: google/sanitizers#1322
Refs: #43082
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Jul 12, 2022
@richardlau @targos
build: set ASAN workaround
0bdb5f2 
PR-URL: #43085
Refs: google/sanitizers#1322
Refs: #43082
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
targos pushed a commit that referenced this pull request Jul 31, 2022
@richardlau @targos
build: set ASAN workaround
cf68280 
PR-URL: #43085
Refs: google/sanitizers#1322
Refs: #43082
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@targos targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@richardlau @guangwong
build: set ASAN workaround
2b69987 
PR-URL: nodejs/node#43085
Refs: google/sanitizers#1322
Refs: nodejs/node#43082
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: LiviaMedeiros <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LiviaMedeiros LiviaMedeiros LiviaMedeiros approved these changes

@gengjiawen gengjiawen gengjiawen approved these changes

@lpinca lpinca lpinca approved these changes

@Trott Trott Trott approved these changes

Assignees
No one assigned
Labels
commit-queue-failed An error occurred while landing this pull request using GitHub Actions. fast-track PRs that do not need to wait for 48 hours to land. meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@richardlau @nodejs-github-bot @gengjiawen @Trott @lpinca @LiviaMedeiros