src: use wide string for findPackageJson onWindows

[islandryu]

Fix error when searching for package.json with non-ASCII characters in paths

Fixes: #55773

[aduh95]

Can you add a test?

[RedYetiDev]

This can be a fixture, right?

[islandryu]

Thank you. I should do that.

[RedYetiDev]

Common must be the first import. FWIW running make lint will catch this.

[islandryu]

sorry for the lint error.
I've fixed it.

[codecov]

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.93%. Comparing base (b02cd41) to head (c295818).
Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
src/node_modules.cc	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55861      +/-   ##
==========================================
- Coverage   88.42%   87.93%   -0.49%     
==========================================
  Files         654      654              
  Lines      187852   188038     +186     
  Branches    36134    35871     -263     
==========================================
- Hits       166102   165350     -752     
- Misses      14989    15868     +879     
- Partials     6761     6820      +59     

Files with missing lines	Coverage Δ
src/node_file.cc	77.39% <100.00%> (+0.04%)	⬆️
src/util.cc	87.39% <ø> (ø)
src/util.h	91.52% <100.00%> (+0.29%)	⬆️
src/node_modules.cc	78.64% <83.33%> (-0.14%)	⬇️

... and 95 files with indirect coverage changes

[anonrig]

These macros are unnecessary. Also, str.ToString() makes an unnecessary copy, even tho, the function is const std::string&. str.ToStringView() would remove the unnecessary copy here.

[islandryu]

I modified the code to avoid using macros and to use std::string_view instead.

[islandryu]

During testing, it became evident that the occurrence of the error described in issue #55773 is influenced by the character encoding used by the user.

For instance, users working with CP1252 are unlikely to encounter any errors, regardless of the characters they use. In contrast, users with encodings like CP932 are more prone to experiencing errors.

Take the character "月" as an example, which is represented in UTF-8 as:
E6 9C 88

In CP932, the byte 88 is interpreted as the leading byte of a 2-byte character, potentially causing issues.

The key takeaway is that to ensure accurate regression testing, it might be necessary to test the runtime environment with character encodings other than CP1252.

I also looked for other tests that might include regression testing against Windows-specific character encodings but couldn’t find any. If you have any good ideas, I’d appreciate your input.

[huseyinacacak-janea]

From what I understand, you’re looking for something along these lines:

const stdoutExec = execSync('@chcp 932 >nul & \"' + process.execPath + '\" \"' + nonAsciiPath + '\"', 
                              { encoding: 'utf8'});
assert.strictEqual(stdoutExec, 'check non-ascii\n');

However, I wanted to point out that this code does not fail when run on the main branch on my local machine. Because of this, it might not be ideal to use this approach directly without further investigation.

[islandryu]

Thank you for your response.
I tried it as well, and it seems that the behavior was not affected.
It appears that we need to change the system's character encoding rather than just the console's encoding.

For instance, the following minimal C++ code reproduces the same error:

int main() {
  std::string path = "\x88";
  std::filesystem::path file(path);
}

This code alone triggers the same issue.

[islandryu]

I am currently investigating whether it is possible to change the system's character encoding within a cctest.

[islandryu]

The key takeaway is that to ensure accurate regression testing, it might be necessary to test the runtime environment with character encodings other than CP1252.

Regarding the content mentioned above:
The root cause of the error is that std::filesystem::path internally converts file paths using the CP_ACP code page.
Based on my investigation, CP_ACP cannot be modified either through command-line arguments or programmatically.
Therefore, in order to execute an accurate regression test, the CI system environment itself would need to be adjusted.