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

deps: V8: cherry-pick 53e62af #29898

Closed
wants to merge 1 commit into from
Closed

deps: V8: cherry-pick 53e62af #29898

wants to merge 1 commit into from

Conversation

targos
Copy link
Member

@targos targos commented Oct 9, 2019

Original commit message:

[build] Include string in v8.h

Explicitly #include<string> in v8.h, since std::string is referenced
in it. In the C++ STL shipped with Visual Studio 2019, none of the
headers included in v8.h ends up including the C++ string header, which
caused a compile error.

Bug: v8:9793
Change-Id: I84a133dd10dd6dcc7b70287af393e82cf0dc97df
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1834321
Reviewed-by: Adam Klein <[email protected]>
Commit-Queue: Adam Klein <[email protected]>
Cr-Commit-Position: refs/heads/master@{#64074}

Refs: v8/v8@53e62af

@targos
deps: V8: cherry-pick 53e62af
4acf313 
Original commit message:

    [build] Include string in v8.h

    Explicitly #include<string> in v8.h, since std::string is referenced
    in it. In the C++ STL shipped with Visual Studio 2019, none of the
    headers included in v8.h ends up including the C++ string header, which
    caused a compile error.

    Bug: v8:9793
    Change-Id: I84a133dd10dd6dcc7b70287af393e82cf0dc97df
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1834321
    Reviewed-by: Adam Klein <[email protected]>
    Commit-Queue: Adam Klein <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64074}

Refs: v8/v8@53e62af
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. labels Oct 9, 2019
@targos targos added v8 engine Issues and PRs related to the V8 dependency. and removed v8 engine Issues and PRs related to the V8 dependency. labels Oct 9, 2019
@targos targos mentioned this pull request Oct 9, 2019
Closed
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25918/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25924/

@targos
Copy link
Member Author

targos commented Oct 11, 2019

Landed in a350d8b

targos added a commit that referenced this pull request Oct 11, 2019
@targos
deps: V8: cherry-pick 53e62af
a350d8b 
Original commit message:

    [build] Include string in v8.h

    Explicitly #include<string> in v8.h, since std::string is referenced
    in it. In the C++ STL shipped with Visual Studio 2019, none of the
    headers included in v8.h ends up including the C++ string header, which
    caused a compile error.

    Bug: v8:9793
    Change-Id: I84a133dd10dd6dcc7b70287af393e82cf0dc97df
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1834321
    Reviewed-by: Adam Klein <[email protected]>
    Commit-Queue: Adam Klein <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64074}

Refs: v8/v8@53e62af

PR-URL: #29898
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
@targos targos closed this Oct 11, 2019
@targos targos deleted the v8-fix-vs2019 branch October 11, 2019 09:41
@BethGriggs BethGriggs mentioned this pull request Oct 21, 2019
Merged
@targos
Copy link
Member Author

targos commented Nov 8, 2019

Should only land on v12.x if #30109 does.

MylesBorins pushed a commit that referenced this pull request Jan 8, 2020
@targos @MylesBorins
deps: V8: cherry-pick 53e62af
512a0cc 
Original commit message:

    [build] Include string in v8.h

    Explicitly #include<string> in v8.h, since std::string is referenced
    in it. In the C++ STL shipped with Visual Studio 2019, none of the
    headers included in v8.h ends up including the C++ string header, which
    caused a compile error.

    Bug: v8:9793
    Change-Id: I84a133dd10dd6dcc7b70287af393e82cf0dc97df
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1834321
    Reviewed-by: Adam Klein <[email protected]>
    Commit-Queue: Adam Klein <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64074}

Refs: v8/v8@53e62af

PR-URL: #29898
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
@targos targos mentioned this pull request Jan 15, 2020
Closed
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
@targos @BethGriggs
deps: V8: cherry-pick 53e62af
45240a1 
Original commit message:

    [build] Include string in v8.h

    Explicitly #include<string> in v8.h, since std::string is referenced
    in it. In the C++ STL shipped with Visual Studio 2019, none of the
    headers included in v8.h ends up including the C++ string header, which
    caused a compile error.

    Bug: v8:9793
    Change-Id: I84a133dd10dd6dcc7b70287af393e82cf0dc97df
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1834321
    Reviewed-by: Adam Klein <[email protected]>
    Commit-Queue: Adam Klein <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64074}

Refs: v8/v8@53e62af

PR-URL: #29898
Reviewed-By: David Carlier <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Gus Caplan <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@devsnek devsnek devsnek approved these changes

@richardlau richardlau richardlau approved these changes

@devnexen devnexen devnexen approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees
No one assigned
Labels
build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@targos @nodejs-github-bot @cjihrig @devnexen @richardlau @devsnek @BridgeAR