Import gyp patches from nodejs/node - #1518

Closed
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom
maclover7:jm-mv-node-patches
Closed

Import gyp patches from nodejs/node#1518
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom
maclover7:jm-mv-node-patches

Conversation

@maclover7

@maclover7maclover7 commented Jul 31, 2018•
edited
Loading

Copy link
Copy Markdown
Contributor
Checklist
Description of change

This begins the process of removing tools/gyp from nodejs/node. Once this lands, a new release is published, and is upstreamed into npm, then tools/gyp can be removed from nodejs/node. This would result in only a single version of GYP needing to maintained by the Node.js Foundation, which would live inside nodejs/node-gyp.

I'm planning on opening a separate PR to get all of these on nodejs/node-gyp@master once this PR lands.

GYP patches from nodejs/node

cc @nodejs/node-gyp @nodejs/gyp

Stewart Addisonand others added 7 commits July 31, 2018 11:16
Stewart Addison@maclover7
gyp: backport GYP fix to fix AIX shared suffix
195e379
Required to support the shared library builds on AIX - this sets the
shared library suffix within GYP to .a instead of .so on AIX
My patch: https://codereview.chromium.org/2492233002/ was landed as
as part of this one which fixed some other (not required, but
included for completeness of the backport) changes:
PR-URL: https://github.com/nodejs/node/pull9675
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Ref: https://codereview.chromium.org/2511733005/
@addaleax@maclover7
gyp: float gyp patch for long filenames
bd6929e
Pulling in https://codereview.chromium.org/2019133002/ in its current
state, as gyp seems to be largely abandoned as a project.
Original commit message:
Hash intermediate file name to avoid ENAMETOOLONG
Hash the intermediate Makefile target used for multi-output rules
so that it still works when the involved file names are very long.
Since the intermediate file's name is effectively arbitrary, this
does not come with notable behavioural changes.
The `import hashlib` boilerplate is taken directly
from `xcodeproj_file.py`.
Concretely, this makes the V8 inspector build currently fail when long
pathnames are involved, notably when using ecryptfs which has a lower
file name length limit.
Fixes: nodejs/node#7959
Ref: nodejs/node#7510
PR-URL: nodejs/node#7963
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
@bnoordhuis@maclover7
gyp: add compile_commands.json gyp generator
2dcb7a5
this is a re-base of the gyp part of
3c46bb9931ecea71167342322e09121ee48cde8e
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@danbev@maclover7
gyp: enable cctest to use objects (gyp part)
0bf105d
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@sam-github@maclover7
gyp: implement LD/LDXX for ninja and FIPS
a53dd31
The ability to set the link rule is used for FIPS, and needs to set
both the `ld =` and `ldxx =` variables in the ninja build file to link
c++ (node) and c (openssl-cli, etc.) executables.
URL: nodejs/node#14227
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Masashi Hirano@maclover7
tools: fix "the the" typos in comments
628acf6
PR-URL: nodejs/node#20716Fixes: nodejs/node#20682
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ryzokuken@maclover7
tools: patch gyp to avoid xcrun errors
b2c4f7b
Previously running ./configure with only the Xcode Command Line Tools
installed would give:
xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
Fixes: nodejs/node#12531
PR-URL: nodejs/node#21520
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@maclover7

This comment has been minimized.

Sign in to view
@danbev@maclover7
gyp: fix ninja build failure (GYP patch)
a2425a3
Currently the files specified in libraries in node.gyp `cctest` target are
getting a '.lib' extension on windows when generated with ninja.
This commit adds a check to see if a file has a '.obj' extension and in
that case no '.lib' extension will be added.
Also, the LIBS specified in the 'libraries' section are not
being included in the --start-group --end-group section which
means that these libraries will not be searched causing issue
with linkers where the order matters.
PR-URL: nodejs/node#12484Fixes: nodejs/node#12448
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@maclover7

maclover7 commented Jul 31, 2018

Copy link
Copy Markdown
ContributorAuthor

CI (https://ci.nodejs.org/job/nodegyp-test-pull-request/75/) is green besides for v0.10, which should be fixed by #1492

@maclover7maclover7 mentioned this pull request Jul 31, 2018
Closed
@rvagg

rvagg commented Aug 8, 2018

Copy link
Copy Markdown
Member

@maclover7 the biggest changes I see in here impact AIX and macOS, neither of which appear to be in nodegyp-test-pull-request. xcode_emulation.py is my main concern (compile_commands_json) is purely for Node's configure consumption). Can we get macOS into that Jenkins job?

@maclover7

maclover7 commented Aug 8, 2018•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@rvagg Created a clone of the usual job and added macOS 10.11 -- looks like macOS is failing with Node.js v0.10.48: https://ci.nodejs.org/job/maclover7-nodegyp-test-pull-request/4/

@rvagg

rvagg commented Aug 8, 2018

Copy link
Copy Markdown
Member

That v0.10 error got fixed in #1492 and improved in #1521, also I was on my mac messing around with this yesterday and couldn't find a problem so my comfort level has improved. Unfortunately it's edge-cases that are going to bite us since we have such a poor test suite here.

@rvagg

rvagg commented Aug 9, 2018

Copy link
Copy Markdown
Member

merged & published in v3.8.0

@rvaggrvagg closed this Aug 9, 2018
@maclover7
maclover7 deleted the jm-mv-node-patches branch August 9, 2018 00:56
@richardlaurichardlau mentioned this pull request Jul 5, 2019
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@maclover7@rvagg@addaleax@bnoordhuis@danbev@sam-github@ryzokuken
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Import gyp patches from nodejs/node - #1518
Closed
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Closed
Import gyp patches from nodejs/node#1518
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Conversation
@maclover7
@maclover7maclover7
commented
Jul 31, 2018•

edited

Loading

Copy link


Copy Markdown

Contributor
Checklist
Description of change
This begins the process of removing tools/gyp from nodejs/node. Once this lands, a new release is published, and is upstreamed into npm, then tools/gyp can be removed from nodejs/node. This would result in only a single version of GYP needing to maintained by the Node.js Foundation, which would live inside nodejs/node-gyp.
I'm planning on opening a separate PR to get all of these on nodejs/node-gyp@master once this PR lands.
GYP patches from nodejs/node
cc @nodejs/node-gyp @nodejs/gyp
Stewart Addisonand others
added 7 commits
July 31, 2018 11:16
Stewart Addison@maclover7
gyp: backport GYP fix to fix AIX shared suffix
195e379
Required to support the shared library builds on AIX - this sets the
shared library suffix within GYP to .a instead of .so on AIX
My patch: https://codereview.chromium.org/2492233002/ was landed as
as part of this one which fixed some other (not required, but
included for completeness of the backport) changes:
PR-URL: https://github.com/nodejs/node/pull9675
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Ref: https://codereview.chromium.org/2511733005/
@addaleax@maclover7
gyp: float gyp patch for long filenames
bd6929e
Pulling in https://codereview.chromium.org/2019133002/ in its current
state, as gyp seems to be largely abandoned as a project.
Original commit message:
Hash intermediate file name to avoid ENAMETOOLONG
Hash the intermediate Makefile target used for multi-output rules
so that it still works when the involved file names are very long.
Since the intermediate file's name is effectively arbitrary, this
does not come with notable behavioural changes.
The `import hashlib` boilerplate is taken directly
from `xcodeproj_file.py`.
Concretely, this makes the V8 inspector build currently fail when long
pathnames are involved, notably when using ecryptfs which has a lower
file name length limit.
Fixes: nodejs/node#7959
Ref: nodejs/node#7510
PR-URL: nodejs/node#7963
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
@bnoordhuis@maclover7
gyp: add compile_commands.json gyp generator
2dcb7a5
this is a re-base of the gyp part of
3c46bb9931ecea71167342322e09121ee48cde8e
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@danbev@maclover7
gyp: enable cctest to use objects (gyp part)
0bf105d
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@sam-github@maclover7
gyp: implement LD/LDXX for ninja and FIPS
a53dd31
The ability to set the link rule is used for FIPS, and needs to set
both the `ld =` and `ldxx =` variables in the ninja build file to link
c++ (node) and c (openssl-cli, etc.) executables.
URL: nodejs/node#14227
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Masashi Hirano@maclover7
tools: fix "the the" typos in comments
628acf6
PR-URL: nodejs/node#20716Fixes: nodejs/node#20682
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ryzokuken@maclover7
tools: patch gyp to avoid xcrun errors
b2c4f7b
Previously running ./configure with only the Xcode Command Line Tools
installed would give:
xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
Fixes: nodejs/node#12531
PR-URL: nodejs/node#21520
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@maclover7

This comment has been minimized.

Sign in to view
@danbev@maclover7
gyp: fix ninja build failure (GYP patch)
a2425a3
Currently the files specified in libraries in node.gyp `cctest` target are
getting a '.lib' extension on windows when generated with ninja.
This commit adds a check to see if a file has a '.obj' extension and in
that case no '.lib' extension will be added.
Also, the LIBS specified in the 'libraries' section are not
being included in the --start-group --end-group section which
means that these libraries will not be searched causing issue
with linkers where the order matters.
PR-URL: nodejs/node#12484Fixes: nodejs/node#12448
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@maclover7
maclover7
commented
Jul 31, 2018

Copy link


Copy Markdown

ContributorAuthor
CI (https://ci.nodejs.org/job/nodegyp-test-pull-request/75/) is green besides for v0.10, which should be fixed by #1492
@maclover7maclover7
mentioned this pull request
Jul 31, 2018

Closed

@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
@maclover7 the biggest changes I see in here impact AIX and macOS, neither of which appear to be in nodegyp-test-pull-request. xcode_emulation.py is my main concern (compile_commands_json) is purely for Node's configure consumption). Can we get macOS into that Jenkins job?
@maclover7
maclover7
commented
Aug 8, 2018•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
@rvagg Created a clone of the usual job and added macOS 10.11 -- looks like macOS is failing with Node.js v0.10.48: https://ci.nodejs.org/job/maclover7-nodegyp-test-pull-request/4/
@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
That v0.10 error got fixed in #1492 and improved in #1521, also I was on my mac messing around with this yesterday and couldn't find a problem so my comfort level has improved. Unfortunately it's edge-cases that are going to bite us since we have such a poor test suite here.
@rvagg
rvagg
commented
Aug 9, 2018

Copy link


Copy Markdown

Member
merged & published in v3.8.0
@rvaggrvagg
closed this
Aug 9, 2018
@maclover7
maclover7
deleted the

jm-mv-node-patches

branch
August 9, 2018 00:56
@richardlaurichardlau
mentioned this pull request
Jul 5, 2019

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@maclover7@rvagg@addaleax@bnoordhuis@danbev@sam-github@ryzokuken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Import gyp patches from nodejs/node - #1518
Closed
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Closed
Import gyp patches from nodejs/node#1518
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Conversation
@maclover7
@maclover7maclover7
commented
Jul 31, 2018•

edited

Loading

Copy link


Copy Markdown

Contributor
Checklist
Description of change
This begins the process of removing tools/gyp from nodejs/node. Once this lands, a new release is published, and is upstreamed into npm, then tools/gyp can be removed from nodejs/node. This would result in only a single version of GYP needing to maintained by the Node.js Foundation, which would live inside nodejs/node-gyp.
I'm planning on opening a separate PR to get all of these on nodejs/node-gyp@master once this PR lands.
GYP patches from nodejs/node
cc @nodejs/node-gyp @nodejs/gyp
Stewart Addisonand others
added 7 commits
July 31, 2018 11:16
Stewart Addison@maclover7
gyp: backport GYP fix to fix AIX shared suffix
195e379
Required to support the shared library builds on AIX - this sets the
shared library suffix within GYP to .a instead of .so on AIX
My patch: https://codereview.chromium.org/2492233002/ was landed as
as part of this one which fixed some other (not required, but
included for completeness of the backport) changes:
PR-URL: https://github.com/nodejs/node/pull9675
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Ref: https://codereview.chromium.org/2511733005/
@addaleax@maclover7
gyp: float gyp patch for long filenames
bd6929e
Pulling in https://codereview.chromium.org/2019133002/ in its current
state, as gyp seems to be largely abandoned as a project.
Original commit message:
Hash intermediate file name to avoid ENAMETOOLONG
Hash the intermediate Makefile target used for multi-output rules
so that it still works when the involved file names are very long.
Since the intermediate file's name is effectively arbitrary, this
does not come with notable behavioural changes.
The `import hashlib` boilerplate is taken directly
from `xcodeproj_file.py`.
Concretely, this makes the V8 inspector build currently fail when long
pathnames are involved, notably when using ecryptfs which has a lower
file name length limit.
Fixes: nodejs/node#7959
Ref: nodejs/node#7510
PR-URL: nodejs/node#7963
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
@bnoordhuis@maclover7
gyp: add compile_commands.json gyp generator
2dcb7a5
this is a re-base of the gyp part of
3c46bb9931ecea71167342322e09121ee48cde8e
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@danbev@maclover7
gyp: enable cctest to use objects (gyp part)
0bf105d
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@sam-github@maclover7
gyp: implement LD/LDXX for ninja and FIPS
a53dd31
The ability to set the link rule is used for FIPS, and needs to set
both the `ld =` and `ldxx =` variables in the ninja build file to link
c++ (node) and c (openssl-cli, etc.) executables.
URL: nodejs/node#14227
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Masashi Hirano@maclover7
tools: fix "the the" typos in comments
628acf6
PR-URL: nodejs/node#20716Fixes: nodejs/node#20682
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ryzokuken@maclover7
tools: patch gyp to avoid xcrun errors
b2c4f7b
Previously running ./configure with only the Xcode Command Line Tools
installed would give:
xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
Fixes: nodejs/node#12531
PR-URL: nodejs/node#21520
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@maclover7

This comment has been minimized.

Sign in to view
@danbev@maclover7
gyp: fix ninja build failure (GYP patch)
a2425a3
Currently the files specified in libraries in node.gyp `cctest` target are
getting a '.lib' extension on windows when generated with ninja.
This commit adds a check to see if a file has a '.obj' extension and in
that case no '.lib' extension will be added.
Also, the LIBS specified in the 'libraries' section are not
being included in the --start-group --end-group section which
means that these libraries will not be searched causing issue
with linkers where the order matters.
PR-URL: nodejs/node#12484Fixes: nodejs/node#12448
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@maclover7
maclover7
commented
Jul 31, 2018

Copy link


Copy Markdown

ContributorAuthor
CI (https://ci.nodejs.org/job/nodegyp-test-pull-request/75/) is green besides for v0.10, which should be fixed by #1492
@maclover7maclover7
mentioned this pull request
Jul 31, 2018

Closed

@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
@maclover7 the biggest changes I see in here impact AIX and macOS, neither of which appear to be in nodegyp-test-pull-request. xcode_emulation.py is my main concern (compile_commands_json) is purely for Node's configure consumption). Can we get macOS into that Jenkins job?
@maclover7
maclover7
commented
Aug 8, 2018•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
@rvagg Created a clone of the usual job and added macOS 10.11 -- looks like macOS is failing with Node.js v0.10.48: https://ci.nodejs.org/job/maclover7-nodegyp-test-pull-request/4/
@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
That v0.10 error got fixed in #1492 and improved in #1521, also I was on my mac messing around with this yesterday and couldn't find a problem so my comfort level has improved. Unfortunately it's edge-cases that are going to bite us since we have such a poor test suite here.
@rvagg
rvagg
commented
Aug 9, 2018

Copy link


Copy Markdown

Member
merged & published in v3.8.0
@rvaggrvagg
closed this
Aug 9, 2018
@maclover7
maclover7
deleted the

jm-mv-node-patches

branch
August 9, 2018 00:56
@richardlaurichardlau
mentioned this pull request
Jul 5, 2019

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@maclover7@rvagg@addaleax@bnoordhuis@danbev@sam-github@ryzokuken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Import gyp patches from nodejs/node - #1518
Closed
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Closed
Import gyp patches from nodejs/node#1518
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Conversation
@maclover7
@maclover7maclover7
commented
Jul 31, 2018•

edited

Loading

Copy link


Copy Markdown

Contributor
Checklist
Description of change
This begins the process of removing tools/gyp from nodejs/node. Once this lands, a new release is published, and is upstreamed into npm, then tools/gyp can be removed from nodejs/node. This would result in only a single version of GYP needing to maintained by the Node.js Foundation, which would live inside nodejs/node-gyp.
I'm planning on opening a separate PR to get all of these on nodejs/node-gyp@master once this PR lands.
GYP patches from nodejs/node
cc @nodejs/node-gyp @nodejs/gyp
Stewart Addisonand others
added 7 commits
July 31, 2018 11:16
Stewart Addison@maclover7
gyp: backport GYP fix to fix AIX shared suffix
195e379
Required to support the shared library builds on AIX - this sets the
shared library suffix within GYP to .a instead of .so on AIX
My patch: https://codereview.chromium.org/2492233002/ was landed as
as part of this one which fixed some other (not required, but
included for completeness of the backport) changes:
PR-URL: https://github.com/nodejs/node/pull9675
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Ref: https://codereview.chromium.org/2511733005/
@addaleax@maclover7
gyp: float gyp patch for long filenames
bd6929e
Pulling in https://codereview.chromium.org/2019133002/ in its current
state, as gyp seems to be largely abandoned as a project.
Original commit message:
Hash intermediate file name to avoid ENAMETOOLONG
Hash the intermediate Makefile target used for multi-output rules
so that it still works when the involved file names are very long.
Since the intermediate file's name is effectively arbitrary, this
does not come with notable behavioural changes.
The `import hashlib` boilerplate is taken directly
from `xcodeproj_file.py`.
Concretely, this makes the V8 inspector build currently fail when long
pathnames are involved, notably when using ecryptfs which has a lower
file name length limit.
Fixes: nodejs/node#7959
Ref: nodejs/node#7510
PR-URL: nodejs/node#7963
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
@bnoordhuis@maclover7
gyp: add compile_commands.json gyp generator
2dcb7a5
this is a re-base of the gyp part of
3c46bb9931ecea71167342322e09121ee48cde8e
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@danbev@maclover7
gyp: enable cctest to use objects (gyp part)
0bf105d
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@sam-github@maclover7
gyp: implement LD/LDXX for ninja and FIPS
a53dd31
The ability to set the link rule is used for FIPS, and needs to set
both the `ld =` and `ldxx =` variables in the ninja build file to link
c++ (node) and c (openssl-cli, etc.) executables.
URL: nodejs/node#14227
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Masashi Hirano@maclover7
tools: fix "the the" typos in comments
628acf6
PR-URL: nodejs/node#20716Fixes: nodejs/node#20682
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ryzokuken@maclover7
tools: patch gyp to avoid xcrun errors
b2c4f7b
Previously running ./configure with only the Xcode Command Line Tools
installed would give:
xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
Fixes: nodejs/node#12531
PR-URL: nodejs/node#21520
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@maclover7

This comment has been minimized.

Sign in to view
@danbev@maclover7
gyp: fix ninja build failure (GYP patch)
a2425a3
Currently the files specified in libraries in node.gyp `cctest` target are
getting a '.lib' extension on windows when generated with ninja.
This commit adds a check to see if a file has a '.obj' extension and in
that case no '.lib' extension will be added.
Also, the LIBS specified in the 'libraries' section are not
being included in the --start-group --end-group section which
means that these libraries will not be searched causing issue
with linkers where the order matters.
PR-URL: nodejs/node#12484Fixes: nodejs/node#12448
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@maclover7
maclover7
commented
Jul 31, 2018

Copy link


Copy Markdown

ContributorAuthor
CI (https://ci.nodejs.org/job/nodegyp-test-pull-request/75/) is green besides for v0.10, which should be fixed by #1492
@maclover7maclover7
mentioned this pull request
Jul 31, 2018

Closed

@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
@maclover7 the biggest changes I see in here impact AIX and macOS, neither of which appear to be in nodegyp-test-pull-request. xcode_emulation.py is my main concern (compile_commands_json) is purely for Node's configure consumption). Can we get macOS into that Jenkins job?
@maclover7
maclover7
commented
Aug 8, 2018•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
@rvagg Created a clone of the usual job and added macOS 10.11 -- looks like macOS is failing with Node.js v0.10.48: https://ci.nodejs.org/job/maclover7-nodegyp-test-pull-request/4/
@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
That v0.10 error got fixed in #1492 and improved in #1521, also I was on my mac messing around with this yesterday and couldn't find a problem so my comfort level has improved. Unfortunately it's edge-cases that are going to bite us since we have such a poor test suite here.
@rvagg
rvagg
commented
Aug 9, 2018

Copy link


Copy Markdown

Member
merged & published in v3.8.0
@rvaggrvagg
closed this
Aug 9, 2018
@maclover7
maclover7
deleted the

jm-mv-node-patches

branch
August 9, 2018 00:56
@richardlaurichardlau
mentioned this pull request
Jul 5, 2019

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@maclover7@rvagg@addaleax@bnoordhuis@danbev@sam-github@ryzokuken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Import gyp patches from nodejs/node - #1518
Closed
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Closed
Import gyp patches from nodejs/node#1518
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Conversation
@maclover7
@maclover7maclover7
commented
Jul 31, 2018•

edited

Loading

Copy link


Copy Markdown

Contributor
Checklist
Description of change
This begins the process of removing tools/gyp from nodejs/node. Once this lands, a new release is published, and is upstreamed into npm, then tools/gyp can be removed from nodejs/node. This would result in only a single version of GYP needing to maintained by the Node.js Foundation, which would live inside nodejs/node-gyp.
I'm planning on opening a separate PR to get all of these on nodejs/node-gyp@master once this PR lands.
GYP patches from nodejs/node
cc @nodejs/node-gyp @nodejs/gyp
Stewart Addisonand others
added 7 commits
July 31, 2018 11:16
Stewart Addison@maclover7
gyp: backport GYP fix to fix AIX shared suffix
195e379
Required to support the shared library builds on AIX - this sets the
shared library suffix within GYP to .a instead of .so on AIX
My patch: https://codereview.chromium.org/2492233002/ was landed as
as part of this one which fixed some other (not required, but
included for completeness of the backport) changes:
PR-URL: https://github.com/nodejs/node/pull9675
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Ref: https://codereview.chromium.org/2511733005/
@addaleax@maclover7
gyp: float gyp patch for long filenames
bd6929e
Pulling in https://codereview.chromium.org/2019133002/ in its current
state, as gyp seems to be largely abandoned as a project.
Original commit message:
Hash intermediate file name to avoid ENAMETOOLONG
Hash the intermediate Makefile target used for multi-output rules
so that it still works when the involved file names are very long.
Since the intermediate file's name is effectively arbitrary, this
does not come with notable behavioural changes.
The `import hashlib` boilerplate is taken directly
from `xcodeproj_file.py`.
Concretely, this makes the V8 inspector build currently fail when long
pathnames are involved, notably when using ecryptfs which has a lower
file name length limit.
Fixes: nodejs/node#7959
Ref: nodejs/node#7510
PR-URL: nodejs/node#7963
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
@bnoordhuis@maclover7
gyp: add compile_commands.json gyp generator
2dcb7a5
this is a re-base of the gyp part of
3c46bb9931ecea71167342322e09121ee48cde8e
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@danbev@maclover7
gyp: enable cctest to use objects (gyp part)
0bf105d
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@sam-github@maclover7
gyp: implement LD/LDXX for ninja and FIPS
a53dd31
The ability to set the link rule is used for FIPS, and needs to set
both the `ld =` and `ldxx =` variables in the ninja build file to link
c++ (node) and c (openssl-cli, etc.) executables.
URL: nodejs/node#14227
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Masashi Hirano@maclover7
tools: fix "the the" typos in comments
628acf6
PR-URL: nodejs/node#20716Fixes: nodejs/node#20682
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ryzokuken@maclover7
tools: patch gyp to avoid xcrun errors
b2c4f7b
Previously running ./configure with only the Xcode Command Line Tools
installed would give:
xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
Fixes: nodejs/node#12531
PR-URL: nodejs/node#21520
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@maclover7

This comment has been minimized.

Sign in to view
@danbev@maclover7
gyp: fix ninja build failure (GYP patch)
a2425a3
Currently the files specified in libraries in node.gyp `cctest` target are
getting a '.lib' extension on windows when generated with ninja.
This commit adds a check to see if a file has a '.obj' extension and in
that case no '.lib' extension will be added.
Also, the LIBS specified in the 'libraries' section are not
being included in the --start-group --end-group section which
means that these libraries will not be searched causing issue
with linkers where the order matters.
PR-URL: nodejs/node#12484Fixes: nodejs/node#12448
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@maclover7
maclover7
commented
Jul 31, 2018

Copy link


Copy Markdown

ContributorAuthor
CI (https://ci.nodejs.org/job/nodegyp-test-pull-request/75/) is green besides for v0.10, which should be fixed by #1492
@maclover7maclover7
mentioned this pull request
Jul 31, 2018

Closed

@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
@maclover7 the biggest changes I see in here impact AIX and macOS, neither of which appear to be in nodegyp-test-pull-request. xcode_emulation.py is my main concern (compile_commands_json) is purely for Node's configure consumption). Can we get macOS into that Jenkins job?
@maclover7
maclover7
commented
Aug 8, 2018•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
@rvagg Created a clone of the usual job and added macOS 10.11 -- looks like macOS is failing with Node.js v0.10.48: https://ci.nodejs.org/job/maclover7-nodegyp-test-pull-request/4/
@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
That v0.10 error got fixed in #1492 and improved in #1521, also I was on my mac messing around with this yesterday and couldn't find a problem so my comfort level has improved. Unfortunately it's edge-cases that are going to bite us since we have such a poor test suite here.
@rvagg
rvagg
commented
Aug 9, 2018

Copy link


Copy Markdown

Member
merged & published in v3.8.0
@rvaggrvagg
closed this
Aug 9, 2018
@maclover7
maclover7
deleted the

jm-mv-node-patches

branch
August 9, 2018 00:56
@richardlaurichardlau
mentioned this pull request
Jul 5, 2019

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@maclover7@rvagg@addaleax@bnoordhuis@danbev@sam-github@ryzokuken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Import gyp patches from nodejs/node - #1518
Closed
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Closed
Import gyp patches from nodejs/node#1518
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Conversation
@maclover7
@maclover7maclover7
commented
Jul 31, 2018•

edited

Loading

Copy link


Copy Markdown

Contributor
Checklist
Description of change
This begins the process of removing tools/gyp from nodejs/node. Once this lands, a new release is published, and is upstreamed into npm, then tools/gyp can be removed from nodejs/node. This would result in only a single version of GYP needing to maintained by the Node.js Foundation, which would live inside nodejs/node-gyp.
I'm planning on opening a separate PR to get all of these on nodejs/node-gyp@master once this PR lands.
GYP patches from nodejs/node
cc @nodejs/node-gyp @nodejs/gyp
Stewart Addisonand others
added 7 commits
July 31, 2018 11:16
Stewart Addison@maclover7
gyp: backport GYP fix to fix AIX shared suffix
195e379
Required to support the shared library builds on AIX - this sets the
shared library suffix within GYP to .a instead of .so on AIX
My patch: https://codereview.chromium.org/2492233002/ was landed as
as part of this one which fixed some other (not required, but
included for completeness of the backport) changes:
PR-URL: https://github.com/nodejs/node/pull9675
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Ref: https://codereview.chromium.org/2511733005/
@addaleax@maclover7
gyp: float gyp patch for long filenames
bd6929e
Pulling in https://codereview.chromium.org/2019133002/ in its current
state, as gyp seems to be largely abandoned as a project.
Original commit message:
Hash intermediate file name to avoid ENAMETOOLONG
Hash the intermediate Makefile target used for multi-output rules
so that it still works when the involved file names are very long.
Since the intermediate file's name is effectively arbitrary, this
does not come with notable behavioural changes.
The `import hashlib` boilerplate is taken directly
from `xcodeproj_file.py`.
Concretely, this makes the V8 inspector build currently fail when long
pathnames are involved, notably when using ecryptfs which has a lower
file name length limit.
Fixes: nodejs/node#7959
Ref: nodejs/node#7510
PR-URL: nodejs/node#7963
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
@bnoordhuis@maclover7
gyp: add compile_commands.json gyp generator
2dcb7a5
this is a re-base of the gyp part of
3c46bb9931ecea71167342322e09121ee48cde8e
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@danbev@maclover7
gyp: enable cctest to use objects (gyp part)
0bf105d
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@sam-github@maclover7
gyp: implement LD/LDXX for ninja and FIPS
a53dd31
The ability to set the link rule is used for FIPS, and needs to set
both the `ld =` and `ldxx =` variables in the ninja build file to link
c++ (node) and c (openssl-cli, etc.) executables.
URL: nodejs/node#14227
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Masashi Hirano@maclover7
tools: fix "the the" typos in comments
628acf6
PR-URL: nodejs/node#20716Fixes: nodejs/node#20682
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ryzokuken@maclover7
tools: patch gyp to avoid xcrun errors
b2c4f7b
Previously running ./configure with only the Xcode Command Line Tools
installed would give:
xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
Fixes: nodejs/node#12531
PR-URL: nodejs/node#21520
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@maclover7

This comment has been minimized.

Sign in to view
@danbev@maclover7
gyp: fix ninja build failure (GYP patch)
a2425a3
Currently the files specified in libraries in node.gyp `cctest` target are
getting a '.lib' extension on windows when generated with ninja.
This commit adds a check to see if a file has a '.obj' extension and in
that case no '.lib' extension will be added.
Also, the LIBS specified in the 'libraries' section are not
being included in the --start-group --end-group section which
means that these libraries will not be searched causing issue
with linkers where the order matters.
PR-URL: nodejs/node#12484Fixes: nodejs/node#12448
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@maclover7
maclover7
commented
Jul 31, 2018

Copy link


Copy Markdown

ContributorAuthor
CI (https://ci.nodejs.org/job/nodegyp-test-pull-request/75/) is green besides for v0.10, which should be fixed by #1492
@maclover7maclover7
mentioned this pull request
Jul 31, 2018

Closed

@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
@maclover7 the biggest changes I see in here impact AIX and macOS, neither of which appear to be in nodegyp-test-pull-request. xcode_emulation.py is my main concern (compile_commands_json) is purely for Node's configure consumption). Can we get macOS into that Jenkins job?
@maclover7
maclover7
commented
Aug 8, 2018•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
@rvagg Created a clone of the usual job and added macOS 10.11 -- looks like macOS is failing with Node.js v0.10.48: https://ci.nodejs.org/job/maclover7-nodegyp-test-pull-request/4/
@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
That v0.10 error got fixed in #1492 and improved in #1521, also I was on my mac messing around with this yesterday and couldn't find a problem so my comfort level has improved. Unfortunately it's edge-cases that are going to bite us since we have such a poor test suite here.
@rvagg
rvagg
commented
Aug 9, 2018

Copy link


Copy Markdown

Member
merged & published in v3.8.0
@rvaggrvagg
closed this
Aug 9, 2018
@maclover7
maclover7
deleted the

jm-mv-node-patches

branch
August 9, 2018 00:56
@richardlaurichardlau
mentioned this pull request
Jul 5, 2019

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@maclover7@rvagg@addaleax@bnoordhuis@danbev@sam-github@ryzokuken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Import gyp patches from nodejs/node - #1518
Closed
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Closed
Import gyp patches from nodejs/node#1518
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Conversation
@maclover7
@maclover7maclover7
commented
Jul 31, 2018•

edited

Loading

Copy link


Copy Markdown

Contributor
Checklist
Description of change
This begins the process of removing tools/gyp from nodejs/node. Once this lands, a new release is published, and is upstreamed into npm, then tools/gyp can be removed from nodejs/node. This would result in only a single version of GYP needing to maintained by the Node.js Foundation, which would live inside nodejs/node-gyp.
I'm planning on opening a separate PR to get all of these on nodejs/node-gyp@master once this PR lands.
GYP patches from nodejs/node
cc @nodejs/node-gyp @nodejs/gyp
Stewart Addisonand others
added 7 commits
July 31, 2018 11:16
Stewart Addison@maclover7
gyp: backport GYP fix to fix AIX shared suffix
195e379
Required to support the shared library builds on AIX - this sets the
shared library suffix within GYP to .a instead of .so on AIX
My patch: https://codereview.chromium.org/2492233002/ was landed as
as part of this one which fixed some other (not required, but
included for completeness of the backport) changes:
PR-URL: https://github.com/nodejs/node/pull9675
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Ref: https://codereview.chromium.org/2511733005/
@addaleax@maclover7
gyp: float gyp patch for long filenames
bd6929e
Pulling in https://codereview.chromium.org/2019133002/ in its current
state, as gyp seems to be largely abandoned as a project.
Original commit message:
Hash intermediate file name to avoid ENAMETOOLONG
Hash the intermediate Makefile target used for multi-output rules
so that it still works when the involved file names are very long.
Since the intermediate file's name is effectively arbitrary, this
does not come with notable behavioural changes.
The `import hashlib` boilerplate is taken directly
from `xcodeproj_file.py`.
Concretely, this makes the V8 inspector build currently fail when long
pathnames are involved, notably when using ecryptfs which has a lower
file name length limit.
Fixes: nodejs/node#7959
Ref: nodejs/node#7510
PR-URL: nodejs/node#7963
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
@bnoordhuis@maclover7
gyp: add compile_commands.json gyp generator
2dcb7a5
this is a re-base of the gyp part of
3c46bb9931ecea71167342322e09121ee48cde8e
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@danbev@maclover7
gyp: enable cctest to use objects (gyp part)
0bf105d
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@sam-github@maclover7
gyp: implement LD/LDXX for ninja and FIPS
a53dd31
The ability to set the link rule is used for FIPS, and needs to set
both the `ld =` and `ldxx =` variables in the ninja build file to link
c++ (node) and c (openssl-cli, etc.) executables.
URL: nodejs/node#14227
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Masashi Hirano@maclover7
tools: fix "the the" typos in comments
628acf6
PR-URL: nodejs/node#20716Fixes: nodejs/node#20682
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ryzokuken@maclover7
tools: patch gyp to avoid xcrun errors
b2c4f7b
Previously running ./configure with only the Xcode Command Line Tools
installed would give:
xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
Fixes: nodejs/node#12531
PR-URL: nodejs/node#21520
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@maclover7

This comment has been minimized.

Sign in to view
@danbev@maclover7
gyp: fix ninja build failure (GYP patch)
a2425a3
Currently the files specified in libraries in node.gyp `cctest` target are
getting a '.lib' extension on windows when generated with ninja.
This commit adds a check to see if a file has a '.obj' extension and in
that case no '.lib' extension will be added.
Also, the LIBS specified in the 'libraries' section are not
being included in the --start-group --end-group section which
means that these libraries will not be searched causing issue
with linkers where the order matters.
PR-URL: nodejs/node#12484Fixes: nodejs/node#12448
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@maclover7
maclover7
commented
Jul 31, 2018

Copy link


Copy Markdown

ContributorAuthor
CI (https://ci.nodejs.org/job/nodegyp-test-pull-request/75/) is green besides for v0.10, which should be fixed by #1492
@maclover7maclover7
mentioned this pull request
Jul 31, 2018

Closed

@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
@maclover7 the biggest changes I see in here impact AIX and macOS, neither of which appear to be in nodegyp-test-pull-request. xcode_emulation.py is my main concern (compile_commands_json) is purely for Node's configure consumption). Can we get macOS into that Jenkins job?
@maclover7
maclover7
commented
Aug 8, 2018•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
@rvagg Created a clone of the usual job and added macOS 10.11 -- looks like macOS is failing with Node.js v0.10.48: https://ci.nodejs.org/job/maclover7-nodegyp-test-pull-request/4/
@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
That v0.10 error got fixed in #1492 and improved in #1521, also I was on my mac messing around with this yesterday and couldn't find a problem so my comfort level has improved. Unfortunately it's edge-cases that are going to bite us since we have such a poor test suite here.
@rvagg
rvagg
commented
Aug 9, 2018

Copy link


Copy Markdown

Member
merged & published in v3.8.0
@rvaggrvagg
closed this
Aug 9, 2018
@maclover7
maclover7
deleted the

jm-mv-node-patches

branch
August 9, 2018 00:56
@richardlaurichardlau
mentioned this pull request
Jul 5, 2019

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@maclover7@rvagg@addaleax@bnoordhuis@danbev@sam-github@ryzokuken

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Import gyp patches from nodejs/node - #1518
Closed
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Closed
Import gyp patches from nodejs/node#1518
maclover7 wants to merge 8 commits into
nodejs:v3.xfrom 
maclover7:jm-mv-node-patches
Conversation
@maclover7
@maclover7maclover7
commented
Jul 31, 2018•

edited

Loading

Copy link


Copy Markdown

Contributor
Checklist
Description of change
This begins the process of removing tools/gyp from nodejs/node. Once this lands, a new release is published, and is upstreamed into npm, then tools/gyp can be removed from nodejs/node. This would result in only a single version of GYP needing to maintained by the Node.js Foundation, which would live inside nodejs/node-gyp.
I'm planning on opening a separate PR to get all of these on nodejs/node-gyp@master once this PR lands.
GYP patches from nodejs/node
cc @nodejs/node-gyp @nodejs/gyp
Stewart Addisonand others
added 7 commits
July 31, 2018 11:16
Stewart Addison@maclover7
gyp: backport GYP fix to fix AIX shared suffix
195e379
Required to support the shared library builds on AIX - this sets the
shared library suffix within GYP to .a instead of .so on AIX
My patch: https://codereview.chromium.org/2492233002/ was landed as
as part of this one which fixed some other (not required, but
included for completeness of the backport) changes:
PR-URL: https://github.com/nodejs/node/pull9675
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Ref: https://codereview.chromium.org/2511733005/
@addaleax@maclover7
gyp: float gyp patch for long filenames
bd6929e
Pulling in https://codereview.chromium.org/2019133002/ in its current
state, as gyp seems to be largely abandoned as a project.
Original commit message:
Hash intermediate file name to avoid ENAMETOOLONG
Hash the intermediate Makefile target used for multi-output rules
so that it still works when the involved file names are very long.
Since the intermediate file's name is effectively arbitrary, this
does not come with notable behavioural changes.
The `import hashlib` boilerplate is taken directly
from `xcodeproj_file.py`.
Concretely, this makes the V8 inspector build currently fail when long
pathnames are involved, notably when using ecryptfs which has a lower
file name length limit.
Fixes: nodejs/node#7959
Ref: nodejs/node#7510
PR-URL: nodejs/node#7963
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
@bnoordhuis@maclover7
gyp: add compile_commands.json gyp generator
2dcb7a5
this is a re-base of the gyp part of
3c46bb9931ecea71167342322e09121ee48cde8e
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-Review-By: James M Snell <jasnell@gmail.com>
Ref: nodejs/node#7986
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@danbev@maclover7
gyp: enable cctest to use objects (gyp part)
0bf105d
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161
Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#12450
Reviewed-By: João Reis <reis@janeasystems.com>
@sam-github@maclover7
gyp: implement LD/LDXX for ninja and FIPS
a53dd31
The ability to set the link rule is used for FIPS, and needs to set
both the `ld =` and `ldxx =` variables in the ninja build file to link
c++ (node) and c (openssl-cli, etc.) executables.
URL: nodejs/node#14227
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Masashi Hirano@maclover7
tools: fix "the the" typos in comments
628acf6
PR-URL: nodejs/node#20716Fixes: nodejs/node#20682
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@ryzokuken@maclover7
tools: patch gyp to avoid xcrun errors
b2c4f7b
Previously running ./configure with only the Xcode Command Line Tools
installed would give:
xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'
Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
Fixes: nodejs/node#12531
PR-URL: nodejs/node#21520
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@maclover7

This comment has been minimized.

Sign in to view
@danbev@maclover7
gyp: fix ninja build failure (GYP patch)
a2425a3
Currently the files specified in libraries in node.gyp `cctest` target are
getting a '.lib' extension on windows when generated with ninja.
This commit adds a check to see if a file has a '.obj' extension and in
that case no '.lib' extension will be added.
Also, the LIBS specified in the 'libraries' section are not
being included in the --start-group --end-group section which
means that these libraries will not be searched causing issue
with linkers where the order matters.
PR-URL: nodejs/node#12484Fixes: nodejs/node#12448
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@maclover7
maclover7
commented
Jul 31, 2018

Copy link


Copy Markdown

ContributorAuthor
CI (https://ci.nodejs.org/job/nodegyp-test-pull-request/75/) is green besides for v0.10, which should be fixed by #1492
@maclover7maclover7
mentioned this pull request
Jul 31, 2018

Closed

@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
@maclover7 the biggest changes I see in here impact AIX and macOS, neither of which appear to be in nodegyp-test-pull-request. xcode_emulation.py is my main concern (compile_commands_json) is purely for Node's configure consumption). Can we get macOS into that Jenkins job?
@maclover7
maclover7
commented
Aug 8, 2018•

edited

Loading

Copy link


Copy Markdown

ContributorAuthor
@rvagg Created a clone of the usual job and added macOS 10.11 -- looks like macOS is failing with Node.js v0.10.48: https://ci.nodejs.org/job/maclover7-nodegyp-test-pull-request/4/
@rvagg
rvagg
commented
Aug 8, 2018

Copy link


Copy Markdown

Member
That v0.10 error got fixed in #1492 and improved in #1521, also I was on my mac messing around with this yesterday and couldn't find a problem so my comfort level has improved. Unfortunately it's edge-cases that are going to bite us since we have such a poor test suite here.
@rvagg
rvagg
commented
Aug 9, 2018

Copy link


Copy Markdown

Member
merged & published in v3.8.0
@rvaggrvagg
closed this
Aug 9, 2018
@maclover7
maclover7
deleted the

jm-mv-node-patches

branch
August 9, 2018 00:56
@richardlaurichardlau
mentioned this pull request
Jul 5, 2019

Closed

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

7 participants

@maclover7@rvagg@addaleax@bnoordhuis@danbev@sam-github@ryzokuken