GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — tools: add msvc /P output to .gitignore by gengjiawen · Pull Request #35735 · nodejs/node

Conversation

gengjiawen commented Oct 21, 2020

Copy link

Copy Markdown

Member

This is pretty useful when debug missing symbol on windows.

See https://docs.microsoft.com/en-us/cpp/build/reference/p-preprocess-to-a-file?view=vs-2019

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss reacted with thumbs up emoji

All reactions

👍 1 reaction


          tools: add msvc /P output to .gitignore

0d1cfd5

nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label

Oct 21, 2020

gengjiawen commented Oct 23, 2020

Copy link

Copy Markdown

Member Author

cc @nodejs/platform-windows @targos

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos commented Oct 23, 2020

Copy link

Copy Markdown

Member

How did you add the /P argument?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gengjiawen commented Oct 23, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member Author

How did you add the /P argument?

diff --git a/tools/v8_gypfiles/v8.gyp b/tools/v8_gypfiles/v8.gyp
index fd786d024e..696474c053 100644
--- a/tools/v8_gypfiles/v8.gyp
+++ b/tools/v8_gypfiles/v8.gyp
@@ -203,7 +203,7 @@
   'target_defaults': {
     'msvs_settings': {
       'VCCLCompilerTool': {
-        'AdditionalOptions': ['/utf-8']
+        'AdditionalOptions': ['/utf-8', '/P']
       }
     },
   },

Recently use case is #35433. The clang support maybe require us to patch v8 again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gengjiawen commented Oct 27, 2020

Copy link

Copy Markdown

Member Author

@targos ping

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

targos approved these changes

Oct 27, 2020

View reviewed changes

targos left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@nodejs/build-files

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

gengjiawen added the commit-queue PRs queued for automated landing through the Commit Queue. label

Oct 28, 2020

github-actions Bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

Oct 28, 2020

github-actions Bot commented Oct 28, 2020

Copy link

Copy Markdown

Contributor

Commit Queue failed

- Loading data for nodejs/node/pull/35735
✔  Done loading data for nodejs/node/pull/35735
----------------------------------- PR info ------------------------------------
Title      tools: add msvc /P output to .gitignore (#35735)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     gengjiawen:msvc-ignore -> nodejs:master
Labels     meta
Commits    1
 - tools: add msvc /P output to .gitignore
Committers 1
 - Jiawen Geng 
PR-URL: https://github.com/nodejs/node/pull/35735
Reviewed-By: Michaël Zasso 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/35735
Reviewed-By: Michaël Zasso 
--------------------------------------------------------------------------------
   ✔  Last GitHub Actions successful
   ✖  No Jenkins CI runs detected
   ℹ  This PR was created on Wed, 21 Oct 2020 10:47:38 GMT
   ✔  Approvals: 1
   ✔  - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/35735#pullrequestreview-517615271
   ✖  This PR needs to wait 9 more hours to land (or 0 hours if there is one more approval)
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

Commit Queue action: https://github.com/nodejs/node/actions/runs/332615436

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

github-actions Bot added the commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. label

Oct 28, 2020

gengjiawen added a commit that referenced this pull request

Oct 30, 2020


          tools: add msvc /P output to .gitignore

48ed72d

PR-URL: #35735
Reviewed-By: Michaël Zasso <[email protected]>

gengjiawen commented Oct 30, 2020

Copy link

Copy Markdown

Member Author

Landed in 48ed72d

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gengjiawen closed this

Oct 30, 2020

gengjiawen deleted the msvc-ignore branch

October 30, 2020 01:47

targos pushed a commit that referenced this pull request

Nov 3, 2020


          tools: add msvc /P output to .gitignore

31753ec

PR-URL: #35735
Reviewed-By: Michaël Zasso <[email protected]>

targos mentioned this pull request

Nov 3, 2020

v15.1.0 release proposal #35948

Merged

targos pushed a commit that referenced this pull request

Mar 3, 2021


          tools: add msvc /P output to .gitignore

372fa0f

PR-URL: #35735
Reviewed-By: Michaël Zasso <[email protected]>

MylesBorins pushed a commit that referenced this pull request

Apr 6, 2021


          tools: add msvc /P output to .gitignore

5d29781

PR-URL: #35735
Reviewed-By: Michaël Zasso <[email protected]>

danielleadams mentioned this pull request

May 3, 2021

v14.17.0 proposal #38507

Merged

targos removed the commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. label

Sep 5, 2021

Reviewers

1 more reviewer

targos

targos approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

gengjiawen commented Oct 21, 2020

Uh oh!

gengjiawen commented Oct 23, 2020

Uh oh!

targos commented Oct 23, 2020

Uh oh!

gengjiawen commented Oct 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gengjiawen commented Oct 27, 2020

Uh oh!

targos left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 28, 2020

Uh oh!

gengjiawen commented Oct 30, 2020

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gengjiawen commented Oct 23, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.