Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
deps: cherry-pick 6f68f30 from v8 upstream#7802
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
stefanmb commented Jul 20, 2016
Required to complete work in #7487 |
Original commit message: [build] Add force_dynamic_crt option to build a static library with /… …MD on windows Adds option to build a V8 library statically, but with the options on windows that allows it to be subsequently included in another DLL. On Windows this is required for it to correclty link against the correct C++ runtime. Require for our Node.js shared library build. Reference: nodejs#7487 BUG= [email protected], [email protected] Committed: https://crrev.com/9cf88c1c364cf76c1e745aa63196768435e8ef5d Review-Url: https://codereview.chromium.org/2149963002 Cr-Original-Commit-Position: refs/heads/master@{nodejs#37814} Cr-Commit-Position: refs/heads/master@{nodejs#37856}
0917e78 to e93720dComparebnoordhuis commented Jul 20, 2016
bnoordhuis commented Jul 20, 2016
Node CI again too, previous one failed massively due to network errors: https://ci.nodejs.org/job/node-test-pull-request/3347/ |
mhdawson commented Aug 4, 2016
LGTM, will land today. |
Original commit message: [build] Add force_dynamic_crt option to build a static library with /… …MD on windows Adds option to build a V8 library statically, but with the options on windows that allows it to be subsequently included in another DLL. On Windows this is required for it to correclty link against the correct C++ runtime. Require for our Node.js shared library build. Reference: #7487 BUG= [email protected], [email protected] Committed: https://crrev.com/9cf88c1c364cf76c1e745aa63196768435e8ef5d Review-Url: https://codereview.chromium.org/2149963002 Cr-Original-Commit-Position: refs/heads/master@{#37814} Cr-Commit-Position: refs/heads/master@{#37856} PR-URL: #7802 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
mhdawson commented Aug 4, 2016
Landed as cc18937 |
Added "dll" option to vcbuild.bat Insure that Unix SO name is not used on Windows (i.e. produce a .dll file) Insure that Node and its V8 dependency link against the Visual C++ Runtime dynamically. Requires backported V8 patch, see PR 7802. Ref: #7802 PR-URL: #7487 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
cjihrig commented Aug 10, 2016
stefanmb commented Aug 10, 2016
cjihrig commented Aug 10, 2016
OK, thanks. Adding the "don't land on" labels. |
mhdawson commented Aug 11, 2016
I'd like it to land on 6.x for our use and I believe other consumers like electron may want that as well. @sxa555 can you backport #7487. to 6.X so that we have the pre-reqs to land this on 6.x I think we should hold off on 4.x until we have testing etc. in place, but I do think we want in the next LTS to go out. I'll remove the dont-land-on-v6.x, let me know if anybody disagress. |
cjihrig commented Aug 11, 2016
I'm going to reapply the dont-land-on-v6.x label, because this doesn't apply cleanly. We can use a backport PR instead. |
ofrobots commented Aug 11, 2016
I have included this commit in #8054. This won't need independent backporting if that lands on |
sxa commented Aug 12, 2016
@cjihrig I believe the only issue with the application was that the V8 version numbers were different. The modifications themselves apply ok :-) I'm creating a separate PR that will do this along with the main WIndows changes to build as a DLL (although I can take that out if the 5.1 update lands quickly as @ofrobots suggests). |
Added "dll" option to vcbuild.bat Insure that Unix SO name is not used on Windows (i.e. produce a .dll file) Insure that Node and its V8 dependency link against the Visual C++ Runtime dynamically. Requires backported V8 patch, see PR 7802. Ref: nodejs#7802 PR-URL: nodejs#7487 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Original Commit Message: Added "dll" option to vcbuild.bat Insure that Unix SO name is not used on Windows (i.e. produce a .dll file) Insure that Node and its V8 dependency link against the Visual C++ Runtime dynamically. Requires backported V8 patch, see PR 7802. Ref: #7802 PR-URL: #7487 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Michael Dawson <[email protected]> PR-URL: #8084 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
Original commit message: [build] Add force_dynamic_crt option to build a static library with /… …MD on windows Adds option to build a V8 library statically, but with the options on windows that allows it to be subsequently included in another DLL. On Windows this is required for it to correclty link against the correct C++ runtime. Require for our Node.js shared library build. Reference: nodejs#7487 BUG= [email protected], [email protected] Committed: https://crrev.com/9cf88c1c364cf76c1e745aa63196768435e8ef5d Review-Url: https://codereview.chromium.org/2149963002 Cr-Original-Commit-Position: refs/heads/master@{nodejs#37814} Cr-Commit-Position: refs/heads/master@{nodejs#37856} PR-URL: nodejs#7802 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
MylesBorins commented Nov 15, 2016
why did this backport move the change from |
Added "dll" option to vcbuild.bat Ensure that Unix SO name is not used on Windows (i.e. produce a .dll file) Ensure that Node and its V8 dependency link against the Visual C++ Runtime dynamically. Requires backported V8 patch Ref: nodejs#7802 PR-URL: nodejs#7487 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Original commit message: [build] Add force_dynamic_crt option to build a static library with /… …MD on windows Adds option to build a V8 library statically, but with the options on windows that allows it to be subsequently included in another DLL. On Windows this is required for it to correclty link against the correct C++ runtime. Require for our Node.js shared library build. Reference: nodejs#7487 BUG= [email protected], [email protected] Committed: https://crrev.com/9cf88c1c364cf76c1e745aa63196768435e8ef5d Review-Url: https://codereview.chromium.org/2149963002 Cr-Original-Commit-Position: refs/heads/master@{nodejs#37814} Cr-Commit-Position: refs/heads/master@{nodejs#37856} Ref: nodejs#7802 Ref: nodejs#8084 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Original commit message: [build] Add force_dynamic_crt option to build a static library with /… …MD on windows Adds option to build a V8 library statically, but with the options on windows that allows it to be subsequently included in another DLL. On Windows this is required for it to correclty link against the correct C++ runtime. Require for our Node.js shared library build. Reference: #7487 BUG= [email protected], [email protected] Committed: https://crrev.com/9cf88c1c364cf76c1e745aa63196768435e8ef5d Review-Url: https://codereview.chromium.org/2149963002 Cr-Original-Commit-Position: refs/heads/master@{#37814} Cr-Commit-Position: refs/heads/master@{#37856} Ref: #7802 Ref: #8084 PR-URL: #9610 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Ali Ijaz Sheikh <[email protected]>
Original Commit Message: Added "dll" option to vcbuild.bat Insure that Unix SO name is not used on Windows (i.e. produce a .dll file) Insure that Node and its V8 dependency link against the Visual C++ Runtime dynamically. Requires backported V8 patch, see PR 7802. Ref: #7802 PR-URL: #7487 Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Michael Dawson <[email protected]> PR-URL: #9385 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Myles Borins <[email protected]>
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
deps
Description of change
Original commit message:
[build] Add force_dynamic_crt option to build a static library with /…
…MD on windows
Adds option to build a V8 library statically, but with the options on
windows that allows it to be subsequently included in another DLL. On
Windows this is required for it to correclty link against the correct
C++ runtime. Require for our Node.js shared library build.
Reference: #7487
BUG=
R=[email protected], [email protected]
Committed: https://crrev.com/9cf88c1c364cf76c1e745aa63196768435e8ef5d
Review-Url: https://codereview.chromium.org/2149963002
Cr-Original-Commit-Position: refs/heads/master@{#37814}
Cr-Commit-Position: refs/heads/master@{#37856}