gyp: show descriptive Windows SDK detection error - #14597

Closed
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors
Closed

gyp: show descriptive Windows SDK detection error#14597
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors

Conversation

@jaimecbernardo

Copy link
Copy Markdown
Contributor

When building with Visual Studio 2017, gyp may fail with a non-descriptive message if Windows has stale registry keys for a version of Windows SDK that was previously uninstalled.

This commit adds a specific warning message when the directory for a detected SDK version doesn't exist and adds some fixes to avoid Python crashes that were blocking the detection of other SDK versions:

  • Only try to run listdir on a path if it exists and is a dir.
  • Avoid accessing names[0] if it has no elements.
  • Use %s instead of %o to print compatible_sdks (to avoid TypeError, since %o is the octal number format specifier in Python and %s can be used as a generic format specifier for objects).

Fixes: #14103

/cc @nodejs/build

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

win, build, tools, gyp

When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Fixes: nodejs#14103
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. tools Issues and PRs related to the tools directory. labels Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

/cc @bnoordhuis /cc @refack PTAL

@refackrefack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

@refack

refack commented Aug 2, 2017

Copy link
Copy Markdown
Contributor

@jaimecbernardo this looks good, would you mind submitting it upstream: https://gyp.gsrc.io/docs/Hacking.md

P.S. If you do add me as a reviewer

@refackrefack added the python PRs and issues that require attention from people who are familiar with Python. label Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Will submit it upstream, thanks!

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

I've submitted this PR upstream. Review link: https://chromium-review.googlesource.com/c/602133

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

This PR has landed upstream in https://chromium.googlesource.com/external/gyp/+/324dd166b7c0b39d513026fa52d6280ac6d56770

@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo thanks, I'll update #12632 (or open a new PR, since that one seems stalled).

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack I guess it makes more sense to update the current PR, if you can :) Thank you.

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

@refack

Copy link
Copy Markdown
Contributor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

#12632 started as a GYP bump to enable ninja on windows, but now it's just a GYP bump.
I'll close it and open a new PR with a new title, bumping GYP upto and including 324dd166b7c0b39d513026fa52d6280ac6d56770 maybe it'll be approved faster πŸ€·β€β™‚οΈ

@refack

Copy link
Copy Markdown
Contributor

So I'm closing this in favor of a new GYP bump PR - #14718

@refackrefack closed this Aug 9, 2017
@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo Thanks for the contribution and the "bureaucratic" dance πŸ₯‡

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Thanks for helping me along the way ;)

duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
Patch Set 1:
Hi,
This change comes originally from nodejs/node#14597
I've added as reviewers those accounts that seem to have committed/reviewed recent changes for Windows in gyp.
Please let me know if I should add another reviewer I might have missed.
Thanks, in advance.
Patch-set: 1
Reviewer: Gerrit User 1188132 <1188132@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1174099 <1174099@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1003232 <1003232@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1226816 <1226816@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Work-in-progress: false
duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some Fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Refs: nodejs/node#14597
Bug: nodejs/node#14103
Change-Id: Ifd50fe239f65b7b4a2d69c1c02038bada03066cb
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to Node.js builds or CI infrastructure.pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcbuild fails with non-descriptive error when the registry has stale keys

3 participants

@jaimecbernardo@refack@nodejs-github-bot
, '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

gyp: show descriptive Windows SDK detection error - #14597

Closed
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors
Closed

gyp: show descriptive Windows SDK detection error#14597
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors

Conversation

@jaimecbernardo

Copy link
Copy Markdown
Contributor

When building with Visual Studio 2017, gyp may fail with a non-descriptive message if Windows has stale registry keys for a version of Windows SDK that was previously uninstalled.

This commit adds a specific warning message when the directory for a detected SDK version doesn't exist and adds some fixes to avoid Python crashes that were blocking the detection of other SDK versions:

  • Only try to run listdir on a path if it exists and is a dir.
  • Avoid accessing names[0] if it has no elements.
  • Use %s instead of %o to print compatible_sdks (to avoid TypeError, since %o is the octal number format specifier in Python and %s can be used as a generic format specifier for objects).

Fixes: #14103

/cc @nodejs/build

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

win, build, tools, gyp

When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Fixes: nodejs#14103
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. tools Issues and PRs related to the tools directory. labels Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

/cc @bnoordhuis /cc @refack PTAL

@refackrefack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

@refack

refack commented Aug 2, 2017

Copy link
Copy Markdown
Contributor

@jaimecbernardo this looks good, would you mind submitting it upstream: https://gyp.gsrc.io/docs/Hacking.md

P.S. If you do add me as a reviewer

@refackrefack added the python PRs and issues that require attention from people who are familiar with Python. label Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Will submit it upstream, thanks!

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

I've submitted this PR upstream. Review link: https://chromium-review.googlesource.com/c/602133

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

This PR has landed upstream in https://chromium.googlesource.com/external/gyp/+/324dd166b7c0b39d513026fa52d6280ac6d56770

@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo thanks, I'll update #12632 (or open a new PR, since that one seems stalled).

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack I guess it makes more sense to update the current PR, if you can :) Thank you.

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

@refack

Copy link
Copy Markdown
Contributor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

#12632 started as a GYP bump to enable ninja on windows, but now it's just a GYP bump.
I'll close it and open a new PR with a new title, bumping GYP upto and including 324dd166b7c0b39d513026fa52d6280ac6d56770 maybe it'll be approved faster πŸ€·β€β™‚οΈ

@refack

Copy link
Copy Markdown
Contributor

So I'm closing this in favor of a new GYP bump PR - #14718

@refackrefack closed this Aug 9, 2017
@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo Thanks for the contribution and the "bureaucratic" dance πŸ₯‡

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Thanks for helping me along the way ;)

duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
Patch Set 1:
Hi,
This change comes originally from nodejs/node#14597
I've added as reviewers those accounts that seem to have committed/reviewed recent changes for Windows in gyp.
Please let me know if I should add another reviewer I might have missed.
Thanks, in advance.
Patch-set: 1
Reviewer: Gerrit User 1188132 <1188132@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1174099 <1174099@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1003232 <1003232@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1226816 <1226816@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Work-in-progress: false
duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some Fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Refs: nodejs/node#14597
Bug: nodejs/node#14103
Change-Id: Ifd50fe239f65b7b4a2d69c1c02038bada03066cb
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to Node.js builds or CI infrastructure.pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcbuild fails with non-descriptive error when the registry has stale keys

3 participants

@jaimecbernardo@refack@nodejs-github-bot
, '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

gyp: show descriptive Windows SDK detection error - #14597

Closed
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors
Closed

gyp: show descriptive Windows SDK detection error#14597
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors

Conversation

@jaimecbernardo

Copy link
Copy Markdown
Contributor

When building with Visual Studio 2017, gyp may fail with a non-descriptive message if Windows has stale registry keys for a version of Windows SDK that was previously uninstalled.

This commit adds a specific warning message when the directory for a detected SDK version doesn't exist and adds some fixes to avoid Python crashes that were blocking the detection of other SDK versions:

  • Only try to run listdir on a path if it exists and is a dir.
  • Avoid accessing names[0] if it has no elements.
  • Use %s instead of %o to print compatible_sdks (to avoid TypeError, since %o is the octal number format specifier in Python and %s can be used as a generic format specifier for objects).

Fixes: #14103

/cc @nodejs/build

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

win, build, tools, gyp

When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Fixes: nodejs#14103
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. tools Issues and PRs related to the tools directory. labels Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

/cc @bnoordhuis /cc @refack PTAL

@refackrefack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

@refack

refack commented Aug 2, 2017

Copy link
Copy Markdown
Contributor

@jaimecbernardo this looks good, would you mind submitting it upstream: https://gyp.gsrc.io/docs/Hacking.md

P.S. If you do add me as a reviewer

@refackrefack added the python PRs and issues that require attention from people who are familiar with Python. label Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Will submit it upstream, thanks!

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

I've submitted this PR upstream. Review link: https://chromium-review.googlesource.com/c/602133

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

This PR has landed upstream in https://chromium.googlesource.com/external/gyp/+/324dd166b7c0b39d513026fa52d6280ac6d56770

@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo thanks, I'll update #12632 (or open a new PR, since that one seems stalled).

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack I guess it makes more sense to update the current PR, if you can :) Thank you.

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

@refack

Copy link
Copy Markdown
Contributor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

#12632 started as a GYP bump to enable ninja on windows, but now it's just a GYP bump.
I'll close it and open a new PR with a new title, bumping GYP upto and including 324dd166b7c0b39d513026fa52d6280ac6d56770 maybe it'll be approved faster πŸ€·β€β™‚οΈ

@refack

Copy link
Copy Markdown
Contributor

So I'm closing this in favor of a new GYP bump PR - #14718

@refackrefack closed this Aug 9, 2017
@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo Thanks for the contribution and the "bureaucratic" dance πŸ₯‡

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Thanks for helping me along the way ;)

duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
Patch Set 1:
Hi,
This change comes originally from nodejs/node#14597
I've added as reviewers those accounts that seem to have committed/reviewed recent changes for Windows in gyp.
Please let me know if I should add another reviewer I might have missed.
Thanks, in advance.
Patch-set: 1
Reviewer: Gerrit User 1188132 <1188132@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1174099 <1174099@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1003232 <1003232@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1226816 <1226816@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Work-in-progress: false
duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some Fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Refs: nodejs/node#14597
Bug: nodejs/node#14103
Change-Id: Ifd50fe239f65b7b4a2d69c1c02038bada03066cb
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to Node.js builds or CI infrastructure.pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcbuild fails with non-descriptive error when the registry has stale keys

3 participants

@jaimecbernardo@refack@nodejs-github-bot
, '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

gyp: show descriptive Windows SDK detection error - #14597

Closed
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors
Closed

gyp: show descriptive Windows SDK detection error#14597
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors

Conversation

@jaimecbernardo

Copy link
Copy Markdown
Contributor

When building with Visual Studio 2017, gyp may fail with a non-descriptive message if Windows has stale registry keys for a version of Windows SDK that was previously uninstalled.

This commit adds a specific warning message when the directory for a detected SDK version doesn't exist and adds some fixes to avoid Python crashes that were blocking the detection of other SDK versions:

  • Only try to run listdir on a path if it exists and is a dir.
  • Avoid accessing names[0] if it has no elements.
  • Use %s instead of %o to print compatible_sdks (to avoid TypeError, since %o is the octal number format specifier in Python and %s can be used as a generic format specifier for objects).

Fixes: #14103

/cc @nodejs/build

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

win, build, tools, gyp

When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Fixes: nodejs#14103
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. tools Issues and PRs related to the tools directory. labels Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

/cc @bnoordhuis /cc @refack PTAL

@refackrefack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

@refack

refack commented Aug 2, 2017

Copy link
Copy Markdown
Contributor

@jaimecbernardo this looks good, would you mind submitting it upstream: https://gyp.gsrc.io/docs/Hacking.md

P.S. If you do add me as a reviewer

@refackrefack added the python PRs and issues that require attention from people who are familiar with Python. label Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Will submit it upstream, thanks!

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

I've submitted this PR upstream. Review link: https://chromium-review.googlesource.com/c/602133

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

This PR has landed upstream in https://chromium.googlesource.com/external/gyp/+/324dd166b7c0b39d513026fa52d6280ac6d56770

@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo thanks, I'll update #12632 (or open a new PR, since that one seems stalled).

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack I guess it makes more sense to update the current PR, if you can :) Thank you.

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

@refack

Copy link
Copy Markdown
Contributor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

#12632 started as a GYP bump to enable ninja on windows, but now it's just a GYP bump.
I'll close it and open a new PR with a new title, bumping GYP upto and including 324dd166b7c0b39d513026fa52d6280ac6d56770 maybe it'll be approved faster πŸ€·β€β™‚οΈ

@refack

Copy link
Copy Markdown
Contributor

So I'm closing this in favor of a new GYP bump PR - #14718

@refackrefack closed this Aug 9, 2017
@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo Thanks for the contribution and the "bureaucratic" dance πŸ₯‡

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Thanks for helping me along the way ;)

duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
Patch Set 1:
Hi,
This change comes originally from nodejs/node#14597
I've added as reviewers those accounts that seem to have committed/reviewed recent changes for Windows in gyp.
Please let me know if I should add another reviewer I might have missed.
Thanks, in advance.
Patch-set: 1
Reviewer: Gerrit User 1188132 <1188132@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1174099 <1174099@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1003232 <1003232@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1226816 <1226816@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Work-in-progress: false
duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some Fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Refs: nodejs/node#14597
Bug: nodejs/node#14103
Change-Id: Ifd50fe239f65b7b4a2d69c1c02038bada03066cb
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to Node.js builds or CI infrastructure.pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcbuild fails with non-descriptive error when the registry has stale keys

3 participants

@jaimecbernardo@refack@nodejs-github-bot
, '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

gyp: show descriptive Windows SDK detection error - #14597

Closed
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors
Closed

gyp: show descriptive Windows SDK detection error#14597
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors

Conversation

@jaimecbernardo

Copy link
Copy Markdown
Contributor

When building with Visual Studio 2017, gyp may fail with a non-descriptive message if Windows has stale registry keys for a version of Windows SDK that was previously uninstalled.

This commit adds a specific warning message when the directory for a detected SDK version doesn't exist and adds some fixes to avoid Python crashes that were blocking the detection of other SDK versions:

  • Only try to run listdir on a path if it exists and is a dir.
  • Avoid accessing names[0] if it has no elements.
  • Use %s instead of %o to print compatible_sdks (to avoid TypeError, since %o is the octal number format specifier in Python and %s can be used as a generic format specifier for objects).

Fixes: #14103

/cc @nodejs/build

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

win, build, tools, gyp

When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Fixes: nodejs#14103
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. tools Issues and PRs related to the tools directory. labels Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

/cc @bnoordhuis /cc @refack PTAL

@refackrefack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

@refack

refack commented Aug 2, 2017

Copy link
Copy Markdown
Contributor

@jaimecbernardo this looks good, would you mind submitting it upstream: https://gyp.gsrc.io/docs/Hacking.md

P.S. If you do add me as a reviewer

@refackrefack added the python PRs and issues that require attention from people who are familiar with Python. label Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Will submit it upstream, thanks!

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

I've submitted this PR upstream. Review link: https://chromium-review.googlesource.com/c/602133

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

This PR has landed upstream in https://chromium.googlesource.com/external/gyp/+/324dd166b7c0b39d513026fa52d6280ac6d56770

@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo thanks, I'll update #12632 (or open a new PR, since that one seems stalled).

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack I guess it makes more sense to update the current PR, if you can :) Thank you.

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

@refack

Copy link
Copy Markdown
Contributor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

#12632 started as a GYP bump to enable ninja on windows, but now it's just a GYP bump.
I'll close it and open a new PR with a new title, bumping GYP upto and including 324dd166b7c0b39d513026fa52d6280ac6d56770 maybe it'll be approved faster πŸ€·β€β™‚οΈ

@refack

Copy link
Copy Markdown
Contributor

So I'm closing this in favor of a new GYP bump PR - #14718

@refackrefack closed this Aug 9, 2017
@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo Thanks for the contribution and the "bureaucratic" dance πŸ₯‡

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Thanks for helping me along the way ;)

duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
Patch Set 1:
Hi,
This change comes originally from nodejs/node#14597
I've added as reviewers those accounts that seem to have committed/reviewed recent changes for Windows in gyp.
Please let me know if I should add another reviewer I might have missed.
Thanks, in advance.
Patch-set: 1
Reviewer: Gerrit User 1188132 <1188132@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1174099 <1174099@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1003232 <1003232@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1226816 <1226816@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Work-in-progress: false
duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some Fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Refs: nodejs/node#14597
Bug: nodejs/node#14103
Change-Id: Ifd50fe239f65b7b4a2d69c1c02038bada03066cb
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to Node.js builds or CI infrastructure.pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcbuild fails with non-descriptive error when the registry has stale keys

3 participants

@jaimecbernardo@refack@nodejs-github-bot
, '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

gyp: show descriptive Windows SDK detection error - #14597

Closed
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors
Closed

gyp: show descriptive Windows SDK detection error#14597
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors

Conversation

@jaimecbernardo

Copy link
Copy Markdown
Contributor

When building with Visual Studio 2017, gyp may fail with a non-descriptive message if Windows has stale registry keys for a version of Windows SDK that was previously uninstalled.

This commit adds a specific warning message when the directory for a detected SDK version doesn't exist and adds some fixes to avoid Python crashes that were blocking the detection of other SDK versions:

  • Only try to run listdir on a path if it exists and is a dir.
  • Avoid accessing names[0] if it has no elements.
  • Use %s instead of %o to print compatible_sdks (to avoid TypeError, since %o is the octal number format specifier in Python and %s can be used as a generic format specifier for objects).

Fixes: #14103

/cc @nodejs/build

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

win, build, tools, gyp

When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Fixes: nodejs#14103
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. tools Issues and PRs related to the tools directory. labels Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

/cc @bnoordhuis /cc @refack PTAL

@refackrefack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

@refack

refack commented Aug 2, 2017

Copy link
Copy Markdown
Contributor

@jaimecbernardo this looks good, would you mind submitting it upstream: https://gyp.gsrc.io/docs/Hacking.md

P.S. If you do add me as a reviewer

@refackrefack added the python PRs and issues that require attention from people who are familiar with Python. label Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Will submit it upstream, thanks!

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

I've submitted this PR upstream. Review link: https://chromium-review.googlesource.com/c/602133

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

This PR has landed upstream in https://chromium.googlesource.com/external/gyp/+/324dd166b7c0b39d513026fa52d6280ac6d56770

@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo thanks, I'll update #12632 (or open a new PR, since that one seems stalled).

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack I guess it makes more sense to update the current PR, if you can :) Thank you.

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

@refack

Copy link
Copy Markdown
Contributor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

#12632 started as a GYP bump to enable ninja on windows, but now it's just a GYP bump.
I'll close it and open a new PR with a new title, bumping GYP upto and including 324dd166b7c0b39d513026fa52d6280ac6d56770 maybe it'll be approved faster πŸ€·β€β™‚οΈ

@refack

Copy link
Copy Markdown
Contributor

So I'm closing this in favor of a new GYP bump PR - #14718

@refackrefack closed this Aug 9, 2017
@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo Thanks for the contribution and the "bureaucratic" dance πŸ₯‡

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Thanks for helping me along the way ;)

duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
Patch Set 1:
Hi,
This change comes originally from nodejs/node#14597
I've added as reviewers those accounts that seem to have committed/reviewed recent changes for Windows in gyp.
Please let me know if I should add another reviewer I might have missed.
Thanks, in advance.
Patch-set: 1
Reviewer: Gerrit User 1188132 <1188132@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1174099 <1174099@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1003232 <1003232@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1226816 <1226816@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Work-in-progress: false
duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some Fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Refs: nodejs/node#14597
Bug: nodejs/node#14103
Change-Id: Ifd50fe239f65b7b4a2d69c1c02038bada03066cb
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to Node.js builds or CI infrastructure.pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcbuild fails with non-descriptive error when the registry has stale keys

3 participants

@jaimecbernardo@refack@nodejs-github-bot
, '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

gyp: show descriptive Windows SDK detection error - #14597

Closed
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors
Closed

gyp: show descriptive Windows SDK detection error#14597
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors

Conversation

@jaimecbernardo

Copy link
Copy Markdown
Contributor

When building with Visual Studio 2017, gyp may fail with a non-descriptive message if Windows has stale registry keys for a version of Windows SDK that was previously uninstalled.

This commit adds a specific warning message when the directory for a detected SDK version doesn't exist and adds some fixes to avoid Python crashes that were blocking the detection of other SDK versions:

  • Only try to run listdir on a path if it exists and is a dir.
  • Avoid accessing names[0] if it has no elements.
  • Use %s instead of %o to print compatible_sdks (to avoid TypeError, since %o is the octal number format specifier in Python and %s can be used as a generic format specifier for objects).

Fixes: #14103

/cc @nodejs/build

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

win, build, tools, gyp

When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Fixes: nodejs#14103
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. tools Issues and PRs related to the tools directory. labels Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

/cc @bnoordhuis /cc @refack PTAL

@refackrefack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

@refack

refack commented Aug 2, 2017

Copy link
Copy Markdown
Contributor

@jaimecbernardo this looks good, would you mind submitting it upstream: https://gyp.gsrc.io/docs/Hacking.md

P.S. If you do add me as a reviewer

@refackrefack added the python PRs and issues that require attention from people who are familiar with Python. label Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Will submit it upstream, thanks!

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

I've submitted this PR upstream. Review link: https://chromium-review.googlesource.com/c/602133

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

This PR has landed upstream in https://chromium.googlesource.com/external/gyp/+/324dd166b7c0b39d513026fa52d6280ac6d56770

@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo thanks, I'll update #12632 (or open a new PR, since that one seems stalled).

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack I guess it makes more sense to update the current PR, if you can :) Thank you.

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

@refack

Copy link
Copy Markdown
Contributor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

#12632 started as a GYP bump to enable ninja on windows, but now it's just a GYP bump.
I'll close it and open a new PR with a new title, bumping GYP upto and including 324dd166b7c0b39d513026fa52d6280ac6d56770 maybe it'll be approved faster πŸ€·β€β™‚οΈ

@refack

Copy link
Copy Markdown
Contributor

So I'm closing this in favor of a new GYP bump PR - #14718

@refackrefack closed this Aug 9, 2017
@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo Thanks for the contribution and the "bureaucratic" dance πŸ₯‡

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Thanks for helping me along the way ;)

duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
Patch Set 1:
Hi,
This change comes originally from nodejs/node#14597
I've added as reviewers those accounts that seem to have committed/reviewed recent changes for Windows in gyp.
Please let me know if I should add another reviewer I might have missed.
Thanks, in advance.
Patch-set: 1
Reviewer: Gerrit User 1188132 <1188132@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1174099 <1174099@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1003232 <1003232@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1226816 <1226816@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Work-in-progress: false
duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some Fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Refs: nodejs/node#14597
Bug: nodejs/node#14103
Change-Id: Ifd50fe239f65b7b4a2d69c1c02038bada03066cb
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to Node.js builds or CI infrastructure.pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcbuild fails with non-descriptive error when the registry has stale keys

3 participants

@jaimecbernardo@refack@nodejs-github-bot
, '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

gyp: show descriptive Windows SDK detection error - #14597

Closed
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors
Closed

gyp: show descriptive Windows SDK detection error#14597
jaimecbernardo wants to merge 1 commit into
nodejs:masterfrom
JaneaSystems:jaimecb-H81-gyp-winsdk-errors

Conversation

@jaimecbernardo

Copy link
Copy Markdown
Contributor

When building with Visual Studio 2017, gyp may fail with a non-descriptive message if Windows has stale registry keys for a version of Windows SDK that was previously uninstalled.

This commit adds a specific warning message when the directory for a detected SDK version doesn't exist and adds some fixes to avoid Python crashes that were blocking the detection of other SDK versions:

  • Only try to run listdir on a path if it exists and is a dir.
  • Avoid accessing names[0] if it has no elements.
  • Use %s instead of %o to print compatible_sdks (to avoid TypeError, since %o is the octal number format specifier in Python and %s can be used as a generic format specifier for objects).

Fixes: #14103

/cc @nodejs/build

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

win, build, tools, gyp

When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Fixes: nodejs#14103
@nodejs-github-botnodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. tools Issues and PRs related to the tools directory. labels Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

/cc @bnoordhuis /cc @refack PTAL

@refackrefack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

@refack

refack commented Aug 2, 2017

Copy link
Copy Markdown
Contributor

@jaimecbernardo this looks good, would you mind submitting it upstream: https://gyp.gsrc.io/docs/Hacking.md

P.S. If you do add me as a reviewer

@refackrefack added the python PRs and issues that require attention from people who are familiar with Python. label Aug 2, 2017
@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Will submit it upstream, thanks!

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

I've submitted this PR upstream. Review link: https://chromium-review.googlesource.com/c/602133

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

This PR has landed upstream in https://chromium.googlesource.com/external/gyp/+/324dd166b7c0b39d513026fa52d6280ac6d56770

@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo thanks, I'll update #12632 (or open a new PR, since that one seems stalled).

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack I guess it makes more sense to update the current PR, if you can :) Thank you.

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

@refack

Copy link
Copy Markdown
Contributor

@refack Actually, that PR is for ninja, right? This commit doesn't seem to fall under that umbrella.

#12632 started as a GYP bump to enable ninja on windows, but now it's just a GYP bump.
I'll close it and open a new PR with a new title, bumping GYP upto and including 324dd166b7c0b39d513026fa52d6280ac6d56770 maybe it'll be approved faster πŸ€·β€β™‚οΈ

@refack

Copy link
Copy Markdown
Contributor

So I'm closing this in favor of a new GYP bump PR - #14718

@refackrefack closed this Aug 9, 2017
@refack

Copy link
Copy Markdown
Contributor

@jaimecbernardo Thanks for the contribution and the "bureaucratic" dance πŸ₯‡

@jaimecbernardo

Copy link
Copy Markdown
ContributorAuthor

@refack Thanks for helping me along the way ;)

duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
Patch Set 1:
Hi,
This change comes originally from nodejs/node#14597
I've added as reviewers those accounts that seem to have committed/reviewed recent changes for Windows in gyp.
Please let me know if I should add another reviewer I might have missed.
Thanks, in advance.
Patch-set: 1
Reviewer: Gerrit User 1188132 <1188132@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1174099 <1174099@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1003232 <1003232@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Reviewer: Gerrit User 1226816 <1226816@3ce6091f-6c88-37e8-8c75-72f92ae8dfba>
Work-in-progress: false
duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
When building with Visual Studio 2017, gyp may fail with a
non-descriptive message if Windows has stale registry keys for a
version of Windows SDK that was previously uninstalled.
This commit adds a specific warning message when the directory for
a detected SDK version doesn't exist and adds some Fixes to avoid
Python crashes that were blocking the detection of other SDK
versions:
- Only try to run listdir on a path if it exists and is a dir.
- Avoid accessing names[0] if it has no elements.
- Use %s instead of %o to print compatible_sdks (to avoid TypeError,
since %o is the octal number format specifier in Python and %s can be
used as a generic format specifier for objects).
Refs: nodejs/node#14597
Bug: nodejs/node#14103
Change-Id: Ifd50fe239f65b7b4a2d69c1c02038bada03066cb
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

buildIssues and PRs related to Node.js builds or CI infrastructure.pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vcbuild fails with non-descriptive error when the registry has stale keys

3 participants

@jaimecbernardo@refack@nodejs-github-bot