Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gyp: update xml string encoding conversion #1203

Closed
wants to merge 3 commits into from
Closed

gyp: update xml string encoding conversion #1203

wants to merge 3 commits into from

Conversation

lc-soft
Copy link
Contributor

@lc-soft lc-soft commented May 20, 2017

My project depends on the node-sass, but it can not successfully install, so I tried using the npm rebuild node-sass --force command to build the node-sass on Windows 10. But it could not be built successfully, error message is: "node_version.h": No such file or directory

I guess this is the problem of compiling configuration, and I found the node-sass\build\binding.vcxproj file from the output information, so I opened it with the editor and found that the configuration item has garbled text:

qq 20170520130717

My username can not be displayed correctly, there seems to be a encoding conversion bug when writing to the binding.vcxproj file. I took some time to debug it, Eventually I found the problem out here: easy_xml.py#L120

qq 20170520130900

Since xml_string contains non-ascii characters, xml_string.encode() will fail and throw an exception, and then this string will be treated as latin-1 encoded string to convert, but my default locale character encoding is not latin-1, therefore, the binding.vcproj file is written with the wrong content.

After modification, the contents of the output file is correct.

qq 20170520130513

node-sass can be built successfully.

gyp info ok
Installed to F:\work\XXXXXX\node_modules\node-sass\vendor\win32-x64-48\binding.node
node-sass@4.5.3 F:\work\XXXXXX\node_modules\node-sass

@gibfahn
Copy link
Member

gibfahn commented May 20, 2017

Maybe cc/ @joaocgreis @refack ?

Proposed change seems reasonable, but I don't know enough to review.

@refack
Copy link
Contributor

refack commented May 20, 2017

@lc-soft I'll have a look
Since it looks like a bug in GYP you should also open an issue at https://bugs.chromium.org/p/gyp/issues/list or even submit a patch

@refack
Copy link
Contributor

refack commented May 20, 2017

P.S. I keep forgetting to say...
Thank you very much for the contribution! 🥇

@lc-soft
Copy link
Contributor Author

lc-soft commented May 20, 2017

@refack I found these: #366 #945 #297.
This is old bug. but they did not submit a patch to upstream, this file has not been changed:
https://chromium.googlesource.com/external/gyp.git/+/master/pylib/gyp/easy_xml.py#105

@refack
Copy link
Contributor

refack commented May 20, 2017

I found these: #366 #945 #297, This is old bug. but they did not submit a patch to upstream, this file has not been changed:
https://chromium.googlesource.com/external/gyp.git/+/master/pylib/gyp/easy_xml.py#105

Yeah, for a long time upstream GYP was unresponsive so people didn't bother with submitting patches. They are trying to be better 🤷‍♂️ . Thank you for pointing me to those PRs!

I'm looking at your patch, we might land it here, and then try to push it upstream.

Anyway since AFAIK this is your first contribution, I'll give you some info on our review and landing process.
Ref: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md#step-7-discuss-and-update
We keep the PR open for at least 48-72 hours so that anyone that might have some input to give will have get a chance. We will go over your code change, maybe ask a few question or request some changes. Then we run our automated test suite, and if everything is Ok we land the change.
In the case of node-gyp one of the package owners will need to decide when and how to release a new package version. Since this seems like semver-patch this stage will probably be quick.

@refack refack self-assigned this May 20, 2017
xml_string = unicode(xml_string, 'latin-1').encode(encoding)

default_encoding = locale.getdefaultlocale()[1]
if default_encoding != encoding.upper():
Copy link
Contributor

Choose a reason for hiding this comment

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

why not .upper or .lower both sides?
Also are you sure this won't throw?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, this is my negligence, I mistakenly thought it is capitalized, by this result:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to "normalize" both sides, since AFAIK there are no two Language Tags that differ only by case.

@lc-soft
Copy link
Contributor Author

lc-soft commented May 20, 2017

I can not run gyptest.py on Windows 10.

F:\repos\gyp>python gyptest.py test/win
[  1/196] (msvs) test\win\generator-output-different-drive\gyptest-generator-output-different-drive.py failed 0.186s
    Error: could not find devenv
    Exception AttributeError: "'TestGypMSVS' object has no attribute '_dirlist'" in <bound method TestGypMSVS.__del__ of 2e20748> ignored
[  2/196] (msvs) test\win\gyptest-asm-files.py failed 0.184s
    Error: could not find devenv
    Exception AttributeError: "'TestGypMSVS' object has no attribute '_dirlist'" in <bound method TestGypMSVS.__del__ of 2a53908> ignored
[  3/196] (msvs) test\win\gyptest-cl-additional-include-dirs.py failed 0.195s
    Error: could not find devenv
    Exception AttributeError: "'TestGypMSVS' object has no attribute '_dirlist'" in <bound method TestGypMSVS.__del__ of 29f3908> ignored
[  4/196] (msvs) test\win\gyptest-cl-additional-options.py failed 0.185s
    Error: could not find devenv
    Exception AttributeError: "'TestGypMSVS' object has no attribute '_dirlist'" in <bound method TestGypMSVS.__del__ of 2983a20> ignored
[  6/196] (msvs) test\win\gyptest-cl-buffer-security-check.py failed 0.191s
    Error: could not find devenv
    Exception AttributeError: "'TestGypMSVS' object has no attribute '_dirlist'" in <bound method TestGypMSVS.__del__ of 2a93978> ignored
[  7/196] (msvs) test\win\gyptest-cl-calling-convention.py failed 0.199s
    Error: could not find devenv
    Exception AttributeError: "'TestGypMSVS' object has no attribute '_dirlist'" in <bound method TestGypMSVS.__del__ of 29c3978> ignored
[  8/196] (msvs) test\win\gyptest-cl-character-set.py failed 0.194s
    Error: could not find devenv
    Exception AttributeError: "'TestGypMSVS' object has no attribute '_dirlist'" in <bound method TestGypMSVS.__del__ of 2ad3978> ignored

@refack
Copy link
Contributor

refack commented May 20, 2017

Which version of Visual Studio you got Installed? Cause if it's VS2017 Build Tools, gyptest can't find them...
You'll need to set an environment variable GYP_BUILD_TOOL to the full path of your msbuild.exe.

Also you could run node-gyp's tests by npm test

BTW: could you add a test case here (maybe add a copy of test\node_modules\hello_world in a unicode path and a call in in test\test-addon.js)

@lc-soft
Copy link
Contributor Author

lc-soft commented May 20, 2017

@refack OK, It has been running successfully.

F:\repos\gyp>set GYP_BUILD_TOOL=D:\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe
F:\repos\gyp>python gyptest.py test/win

But there are some errors:

[  6/196] (msvs) test\win\gyptest-cl-buffer-security-check.py failed 3.843s
    'D:\Microsoft' is not recognized as an internal or external command, operable program or batch file. 
    Traceback (most recent call last):
      File "test\win\gyptest-cl-buffer-security-check.py", line 42, in <module>
        if 'security_cookie' not in GetDisassemblyOfMain('test_bsc_unset.exe'):
      File "test\win\gyptest-cl-buffer-security-check.py", line 27, in GetDisassemblyOfMain
        output = test.run_dumpbin('/disasm', full_path)
      File "F:\repos\gyp\test\lib\TestGyp.py", line 813, in run_dumpbin
        assert not proc.returncode
    AssertionError
    
...

[ 18/196] (msvs) test\win\gyptest-cl-function-level-linking.py failed 2.896s
    'D:\Microsoft' is not recognized as an internal or external command, operable program or batch file. 
    Traceback (most recent call last):
      File "test\win\gyptest-cl-function-level-linking.py", line 44, in <module>
        should_exist=True)
      File "test\win\gyptest-cl-function-level-linking.py", line 23, in CheckForSectionString
        output = test.run_dumpbin('/headers', binary)
      File "F:\repos\gyp\test\lib\TestGyp.py", line 813, in run_dumpbin
        assert not proc.returncode
    AssertionError
...

@lc-soft lc-soft changed the title Improve xml string encoding conversion gyp: update xml string encoding conversion May 20, 2017
@refack
Copy link
Contributor

refack commented May 20, 2017

Try set GYP_BUILD_TOOL="D:\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe"

@lc-soft
Copy link
Contributor Author

lc-soft commented May 21, 2017

@refack There will be more problems after using this method.

F:\repos\gyp>set GYP_BUILD_TOOL="D:\Microsoft Visual Studio\2017\Community\MSBuild\15.0\Bin\MSBuild.exe"
F:\repos\gyp>python gyptest.py test/win
[  1/196] (msvs) test\win\generator-output-different-drive\gyptest-generator-output-different-drive.py failed 0.806s
    STDOUT =========================================================================

    STDERR =========================================================================

    Exception trying to execute: ['F:\\repos\\gyp\\"D:\\Microsoft Visual Studio\\2017\\Community\\MSBuild\\15.0\\Bin\\MSBuild.exe"', 'prog.sln', '/t:Build', '/p:Configuration=Default']
    Traceback (most recent call last):
      File "test\win\generator-output-different-drive\gyptest-generator-output-different-drive.py", line 38, in <module>
        test.build('prog.gyp', test.ALL, chdir=os.path.join(output_drive, 'output'))
      File "F:\repos\gyp\test\lib\TestGyp.py", line 937, in build
        return self.run(program=self.build_tool, **kw)
      File "F:\repos\gyp\test\lib\TestGyp.py", line 333, in run
        super(TestGypBase, self).run(*args, **kw)
      File "F:\repos\gyp\test\lib\TestCommon.py", line 542, in run
        apply(TestCmd.run, [self], kw)
      File "F:\repos\gyp\test\lib\TestCmd.py", line 1224, in run
        stdin=stdin)
      File "F:\repos\gyp\test\lib\TestCommon.py", line 484, in start
        raise e
    WindowsError: [Error 5]
[  2/196] (msvs) test\win\gyptest-asm-files.py failed 1.532s
    STDOUT =========================================================================

    STDERR =========================================================================

    Exception trying to execute: ['F:\\repos\\gyp\\"D:\\Microsoft Visual Studio\\2017\\Community\\MSBuild\\15.0\\Bin\\MSBuild.exe"', 'asm-files.sln', '/t:Build', '/p:Configuration=Default']
    Traceback (most recent call last):
      File "test\win\gyptest-asm-files.py", line 24, in <module>
        test.build('asm-files.gyp', test.ALL, chdir=CHDIR)
      File "F:\repos\gyp\test\lib\TestGyp.py", line 937, in build
        return self.run(program=self.build_tool, **kw)
      File "F:\repos\gyp\test\lib\TestGyp.py", line 333, in run
        super(TestGypBase, self).run(*args, **kw)
      File "F:\repos\gyp\test\lib\TestCommon.py", line 542, in run
        apply(TestCmd.run, [self], kw)
      File "F:\repos\gyp\test\lib\TestCmd.py", line 1224, in run
        stdin=stdin)
      File "F:\repos\gyp\test\lib\TestCommon.py", line 484, in start
        raise e
    WindowsError: [Error 5]

@refack
Copy link
Contributor

refack commented May 21, 2017

Yeah, the test harness is very rusty.
I've pushed your patch to the Chromium code review system, and I've started a CI there - https://chromium-review.googlesource.com/c/509632/

@lc-soft
Copy link
Contributor Author

lc-soft commented May 21, 2017

@refack Good job! 👍

@refack
Copy link
Contributor

refack commented May 21, 2017

@refack
Copy link
Contributor

refack commented May 21, 2017

Did I click submit on a comment asking you to add a test?

Anyway, please do. I can suggest copying test\node_modules\hello_world to a path with non latin-1 characters in it, and adding a test case in "test\test-addon.js" building it.

Bonus points if you add a test also to https://github.com/refack/GYP/tree/node-gyp-1203 😉

Copy link
Contributor

@refack refack left a comment

Choose a reason for hiding this comment

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

LGTM, pending addition of a test.

@lc-soft
Copy link
Contributor Author

lc-soft commented May 21, 2017

OK, I will take a few days to add test.

}
runCmd(t, [nodeGyp, 'configure', '-C', addonPath, '--loglevel=verbose',
'-nodedir=' + testNodeDir])
runCmd(t, [nodeGyp, 'build', '-C', addonPath, '--loglevel=verbose'])
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a clean up (try { fs.unlinkSync...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@refack I have tried to use the rebuild command before, but there will be errors:

...
gyp verb clean removing "build" directory
gyp ERR! clean error
gyp ERR! stack Error: EPERM: operation not permitted, unlink 'F:\repos\GitHub\node-gyp\test\node_modules\hello_world\build\Release\hello.node'
...

how to solve this problem? a temporary solution is to use configure and build commands.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess the reason hello.node can not be deleted is that it is occupied at the time of execution to require('hello_world'). so, solution is to create a new project (E.g:
hello_world_2), and rebuild it.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO execFileSync is also completely fine.
OR you could create a hello_非英文字符 fixture.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean something like execFileSync(node, ['-e', "var x = require('hello_world'); if (x.hello() !== 'hello') process.exit(2)"])
Which is more sanitary anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tried to use execFileSync before, but there have been some error messages, so I switched to the current solution.
OK, I will try to use execFileSync again.

@refack
Copy link
Contributor

refack commented May 23, 2017

Thank you!
One comment and I'll land this.

t.plan(5)

configure(t).on('exit', function () {
var data, config, nodeDir, testNodeDir
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@refack I'am directly merged this test into the test build simple addon.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it works I'm +1
Another solution is to use execFileSync

}
}

rebuild(t, testNodeDir).on('exit', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

})

test('build simple addon in path with non-ascii characters', function (t) {
t.plan(3)
Copy link
Contributor Author

@lc-soft lc-soft May 24, 2017

Choose a reason for hiding this comment

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

@refack OK, this test has been able to run successfully.

F:\repos\GitHub\node-gyp>node ./test/test-addon.js
TAP version 13
# build simple addon
ok 1 should be equal
ok 2 should end in ok
ok 3 should be equal
# build simple addon in path with non-ascii characters
ok 4 should be equal
ok 5 should end in ok
ok 6 should be equal

1..6
# tests 6
# pass  6

# ok

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@refack
Copy link
Contributor

refack commented May 24, 2017

var path = require('path')
var fs = require('graceful-fs')
var child_process = require('child_process')
Copy link
Contributor

Choose a reason for hiding this comment

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

question to @nodejs/node-gyp
Can we use destructuring?

const { execFileSync, execFile } = require('child_process')

Or do we test on pre ES6 versions?

Copy link
Member

Choose a reason for hiding this comment

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

I believe the CI still tests 0.10 and 0.12.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not any more 😄

@lc-soft
Copy link
Contributor Author

lc-soft commented May 24, 2017

CI test did not pass.

gyp: c:\workspace\nodegyp-test-commit\nodes\win2012r2\test\node_modules\hello_world\?????\include\node\common.gypi not found (cwd: c:\workspace\nodegyp-test-commit\nodes\win2012r2\test\node_modules\hello_world) while reading includes of binding.gyp while trying to load binding.gyp
gyp ERR! configure error 
gyp ERR! stack Error: `gyp` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onCpExit (c:\workspace\nodegyp-test-commit\nodes\win2012r2\lib\configure.js:336:16)
gyp ERR! stack     at emitTwo (events.js:87:13)
gyp ERR! stack     at ChildProcess.emit (events.js:172:7)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:211:12)
gyp ERR! System Windows_NT 6.3.9600
gyp ERR! command "c:\\workspace\\nodegyp-test-commit\\nodes\\win2012r2\\local_node\\node.exe" "c:\\workspace\\nodegyp-test-commit\\nodes\\win2012r2\\bin\\node-gyp.js" "rebuild" "-C" "c:\\workspace\\nodegyp-test-commit\\nodes\\win2012r2\\test\\node_modules\\hello_world" "--loglevel=verbose" "-nodedir=c:\\workspace\\nodegyp-test-commit\\nodes\\win2012r2\\test\\node_modules\\hello_world\\�英文字符"
gyp ERR! cwd c:\workspace\nodegyp-test-commit\nodes\win2012r2\test\node_modules\hello_world
gyp ERR! node -v v4.8.3
gyp ERR! node-gyp -v v3.6.1
gyp ERR! not ok 

@refack
Copy link
Contributor

refack commented May 24, 2017

if (!checkCharmapValid()) {
return t.skip('python console app can\'t encode non-ascii character.')
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@refack I added the character map check, if python app can not convert non-ASCII characters, then skip this test. console output like this:

TAP version 13
# build simple addon
ok 1 should be equal
ok 2 should end in ok
ok 3 should be equal
# build simple addon in path with non-ascii characters
node : Traceback (most recent call last):
At line:5 char:1
+ node ./test/test-addon
+ ~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (Traceback (most recent call last)::String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError
 
  File "test-charmap.py", line 20, in <module>
    print main()
  File "test-charmap.py", line 15, in main
    print textmap[encoding]
  File "C:\Python27\lib\encodings\cp1252.py", line 12, in encode
    return codecs.charmap_encode(input,errors,encoding_table)
UnicodeEncodeError: 'charmap' codec can't encode character u'\u012b' in position 3: character maps to <undefined>
ok 4 python console app can't encode non-ascii character. # SKIP
1..4
# tests 4
# pass  4
# ok

@lc-soft
Copy link
Contributor Author

lc-soft commented Jun 8, 2017

@refack
What else do I need to do for this PR ?

package.json Outdated
@@ -46,6 +46,6 @@
"require-inject": "~1.3.0"
},
"scripts": {
"test": "tape test/test-*"
"test": "tape test/test-*.js"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather you move test-charmap.py to test/fixtures

@refack
Copy link
Contributor

refack commented Jun 8, 2017

var path = require('path')
var fs = require('graceful-fs')
var child_process = require('child_process')
Copy link
Contributor Author

@lc-soft lc-soft Jun 8, 2017

Choose a reason for hiding this comment

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

@refack
Do I need rename child_process to childProcess ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@refack
Copy link
Contributor

refack commented Jun 8, 2017

If you could rebase on master it will make the CI not test on node < 4

@lc-soft
Copy link
Contributor Author

lc-soft commented Jun 8, 2017

https://ci.nodejs.org/job/nodegyp-test-commit/187/nodes=win2012r2/console
execFileSync is undefined ?

c:\workspace\nodegyp-test-commit\nodes\win2012r2>call npm test 

> node-gyp@3.6.1 test c:\workspace\nodegyp-test-commit\nodes\win2012r2
> tape test/test-*.js

TAP version 13
# build simple addon
ok 1 should be equal
ok 2 should end in ok

c:\workspace\nodegyp-test-commit\nodes\win2012r2\test\test-addon.js:14
  return execFileSync('node', ['-e', testCode], { cwd: __dirname }).toString()
         ^
TypeError: undefined is not a function
    at runHello (c:\workspace\nodegyp-test-commit\nodes\win2012r2\test\test-addon.js:14:10)
    at c:\workspace\nodegyp-test-commit\nodes\win2012r2\test\test-addon.js:43:19
    at ChildProcess.exithandler (child_process.js:656:7)
    at ChildProcess.emit (events.js:98:17)
    at maybeClose (child_process.js:766:16)
    at Process.ChildProcess._handle.onexit (child_process.js:833:5)
npm ERR! Test failed.  See above for more details.
Build step 'Conditional steps (multiple)' marked build as failure
Notifying upstream pro

@refack
Copy link
Contributor

refack commented Jun 8, 2017

Ignore it's node@0.10 https://ci.nodejs.org/job/nodegyp-test-commit/187/nodes=win2012r2/
As I said if you rebase onto master we bumped the version to 4 and don't test on node < 4

@refack
Copy link
Contributor

refack commented Jun 8, 2017

@lc-soft
Copy link
Contributor Author

lc-soft commented Jun 8, 2017

I need to modify this test to make it support node@0.10 ?

@refack
Copy link
Contributor

refack commented Jun 8, 2017

I need to modify this test to make it support node@0.10 ?

No. We stopped supporting node < 4 a week ago.

refack pushed a commit to refack/node-gyp that referenced this pull request Jun 8, 2017
* test: build simple addon in path with non-ascii characters
* test: add test-charmap.py

PR-URL: nodejs#1203
Reviewed-By: Refael Ackermann <refack@gmail.com>
@refack
Copy link
Contributor

refack commented Jun 8, 2017

Landed in d6139b5

@bnoordhuis
Copy link
Member

Kind of late to the party but I think there is a bug in the python script. It assumes that locale.getdefaultlocale()[1] is a string but that isn't always true:

$ env LC_ALL=C python -c 'import locale; print locale.getdefaultlocale()'
(None, None)

It makes the test fail (or rather: pass when it shouldn't) like this:

$ env LC_ALL=C node test/test-addon.js 
TAP version 13
# build simple addon
ok 1 should be equal
ok 2 should end in ok
ok 3 should be equal
# build simple addon in path with non-ascii characters
Traceback (most recent call last):
  File "fixtures/test-charmap.py", line 19, in <module>
    print main()
  File "fixtures/test-charmap.py", line 8, in main
    sys.setdefaultencoding(encoding)
TypeError: setdefaultencoding() argument 1 must be string, not None
ok 4 python console app can't encode non-ascii character. # SKIP

1..4
# tests 4
# pass  4

# ok

@lc-soft
Copy link
Contributor Author

lc-soft commented Jul 24, 2017

@bnoordhuis
I have not tested it when locale.getdefaultlocale()[1] value is None, and I do not know whether python can process non-ascii encoding string, So, in this case, I set default result is python can not process non-ascii encoding string, not need to run the test.

@refack
Copy link
Contributor

refack commented Jul 24, 2017

xml_string = unicode(xml_string, 'latin-1').encode(encoding)

default_encoding = locale.getdefaultlocale()[1]
if default_encoding.upper() != encoding.upper():
Copy link
Contributor

Choose a reason for hiding this comment

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

should test default_encoding and default_encoding.upper() != encoding.upper():

@refack
Copy link
Contributor

refack commented Jul 24, 2017

@richardlau richardlau mentioned this pull request Jul 20, 2018
2 tasks
duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
Bug: nodejs/node-gyp#1203
Change-Id: I30d71a2bb3d4b09e7bd9409c3c45c32bd182d736
duqingnian pushed a commit to duqingnian/gyp that referenced this pull request Apr 26, 2024
Bug: nodejs/node-gyp#1203
Change-Id: I30d71a2bb3d4b09e7bd9409c3c45c32bd182d736
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants