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 build on msvc #159

Closed
wants to merge 16 commits into from
Closed

Conversation

gengjiawen
Copy link
Member

@gengjiawen gengjiawen commented Jun 1, 2020

If this works, I will port it to upstream v8.

nodejs-ci and others added 16 commits June 1, 2020 05:55
Original commit message:

    [testrunner] delete ancient junit compatible format support

    Testrunner has ancient support for JUnit compatible XML output.

    This CL removes this old feature.

    [email protected],[email protected],[email protected]
    CC=​[email protected]

    Bug: v8:8728
    Change-Id: I7e1beb011dbaec3aa1a27398a5c52abdd778eaf0
    Reviewed-on: https://chromium-review.googlesource.com/c/1430065
    Reviewed-by: Jakob Gruber <[email protected]>
    Reviewed-by: Michael Starzinger <[email protected]>
    Commit-Queue: Tamer Tas <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#59045}

Refs: v8/v8@bd019bd

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Patch V8 (compiler/js-heap-broker.cc) to remove the use of an optional
property, which is a fairly new C++ feature, since that requires a newer
XCode version than the minimum requirement in BUILDING.md and thus
breaks CI.

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Fixes a compilation issue on some platforms

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
This should be semver-patch since actual invocation is version
conditional.

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
There is a bug in the most recent version of VS2015 that affects v8.h
and therefore prevents compilation of addons.

Refs: https://stackoverflow.com/q/38378693

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
PR-URL: nodejs/node#26685
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>

PR-URL: nodejs/node#32116
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Patch V8 (wasm/wasm-module.cc) to remove const qualifier from type
passed to template call of `OwnedVector::Of`. Xcode 8 can't convert
'OwnedVector<unsigned char>' to 'OwnedVector<const unsigned char>' when
returning from a function (which is likely a bug on Xcode, considering
this worked on the prior version of Xcode as well as newer versions).
This workaround shouldn't affect the application, since the const
qualifier is preserved in the AsmJsOffsetInformation::encoded_offset_.

There's also a V8 test passing a const-qualified type to ::Of, but since
we don't test V8 on Xcode 8, it should be fine to leave it as is.

Signed-off-by: Matheus Marchini <[email protected]>

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Bump minimum version of ICU needed to build node to 67.

Refs: v8/v8@611e412
until 1d2677008511f7198f1b2fa3f981ea0c3464a2f3
until 09f082f2335131608f645a11ac2221fcc0048478
until daae98c33643bcf64009572a0540bc53b978cd30
@targos
Copy link
Member

targos commented Jun 2, 2020

Thanks @gengjiawen. I pushed your commit to canary-base.

@targos targos closed this Jun 2, 2020
@gengjiawen gengjiawen deleted the vs2019-build branch June 2, 2020 08:36
@gengjiawen
Copy link
Member Author

Upstream has been merged, should we remove this commit?

@ryzokuken
Copy link

@gengjiawen I think it would be taken care of once your commit lands in LKGR and I'll remove the empty commit.

@targos
Copy link
Member

targos commented Jun 3, 2020

image

The commit is in LKGR

@ryzokuken
Copy link

@targos then I guess we'll have a rebase error breaking the CI tomorrow and I'll fix this!

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.

7 participants