patches: drop internalModuleReadJSON hack of v14.16.0

[jesec]

Fixes: #118
Needs: vercel/pkg#1059

[bfivelson]

Hi @robertsLando

I am trying to work through #118

I followed your instructions on the - https://github.com/yao-pkg/pkg-binaries

from what i can tell the patch applied

I think I am missing something simple

my binary gives me this error

Error: Cannot find module 'PKG_DUMMY_ENTRYPOINT'
�[90m at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)�[39m
at Function.Module._resolveFilename (pkg/prelude/bootstrap.js:1355:46)
�[90m at Function.Module._load (internal/modules/cjs/loader.js:725:27)�[39m
�[90m at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)�[39m
�[90m at internal/main/run_main_module.js:17:47�[39m {
code: �[32m'MODULE_NOT_FOUND'�[39m,
requireStack: []
}

[david-mohr]

@jesec @robertsLando just FYI, removing this will break other things during pkg testing.

[david-mohr]

WIthout this patch, any module that relies on package.json to point to the "main" entrypoint will fail. Anything using index.js will work. The errors in 118 are due to using git apply instead of patch

[bfivelson]

@david-mohr Thank you for your help with this. I tried to download patch 2.5.9 for windows and it does not seem to do anything are you running this from a windows box? Is 2.5.9 the version of patch you are using?

I thought there was a issue with building a windows binary on linux box, maybe that was older version of pkg

[david-mohr]

I have successfully built node binaries using this patch on Windows, Mac and Linux. I simply run node lib-es5/upload.js it does all the work to apply patches and figure out the commands. You just need to disable the steps that check for github credentials and try to upload the assests.

[jesec]

WIthout this patch, any module that relies on package.json to point to the "main" entrypoint will fail. Anything using index.js will work. The errors in 118 are due to using git apply instead of patch

I don't see how that's related.

Anyways, this hack is not acceptable. Whatever the problem is, we have to resolve it in pkg instead.

See vercel/pkg#1059 .

[david-mohr]

@jesec I'm not sure it can be fixed in pkg, that's why it's here. All I did was update the function to match the new return value, which is an array. Rolling back my patch means that the return values no longer match.

https://github.com/nodejs/node/blob/v14.16.0/src/node_file.cc#L976-L984

[david-mohr]

@jesec please don't take my comments the wrong way, it's just that it took a lot of testing to get this function definition right and passing all the pkg tests. Unfortunately now I'll need to go back to using my fork instead.

[jesec]

@jesec I'm not sure it can be fixed in pkg, that's why it's here. All I did was update the function to match the new return value, which is an array. Rolling back my patch means that the return values no longer match.

https://github.com/nodejs/node/blob/v14.16.0/src/node_file.cc#L976-L984

It is properly fixed by vercel/pkg#1059.

vercel/pkg#1059 makes the hooked internalModuleReadJSON return an array, matching new Node versions:

https://github.com/vercel/pkg/blob/602ceb852de1ff22364930d7957f5960ec05ea37/prelude/bootstrap.js#L1194-L1225

The method has been working perfectly fine in production since Jan 18:

jesec/pkg-fetch@559111f
jesec/pkg-fetch@74075ef

[david-mohr]

Thanks for explaining @jesec, I appreciate it.