build: describe the fix for compiling android node.js on an x86_64 host #54995
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
for future reference, there's no need to keep making new PRs, you can reuse the old ones |
RedYetiDev
reviewed
Sep 18, 2024
RedYetiDev
added
build
|
I made modifications and used the built-in Python module shutil for reading |
|
Great. Can you squash the commits (removing the merge commit), and amend your message to accurately describe the change you made? Something like: To squash, use |
RedYetiDev
reviewed
Sep 25, 2024
| @@ -61,6 +62,9 @@ def patch_android(): | |||
| elif platform.system() == "Linux": | |||
| host_os = "linux" | |||
| toolchain_path = android_ndk_path + "/toolchains/llvm/prebuilt/linux-x86_64" | |||
| if platform.machine() in {"x86_64", "x86"}: | |||
There was a problem hiding this comment.
Is this exclusively for linux?
There was a problem hiding this comment.
This modification is made for Linux x86
PR-URL: #54878 Fixes: #54428 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Erick Wendel <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
PR-URL: #54983 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: #54984 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Ulises Gascón <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 12.9. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]>
dllexport introduces issues when compiling with MSVC. PR-URL: #47251 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]> PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
It introduces process hangs on some platforms because Node.js doesn't tear down V8 correctly. Disable it while we work on a solution. Refs: #47297 Refs: https://bugs.chromium.org/p/v8/issues/detail?id=13902 PR-URL: #47450 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]> PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
PR-URL: #54077 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]>
Co-Authored-By: Michaël Zasso <[email protected]> PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #53134 Refs: #52809 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]>
It's causing compiler errors with some classes on Xcode 11 and the attribute should have no runtime effect. PR-URL: #54077 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Original commit message:
[cfi] Add missing 'typename' in SegmentedTable
Makes the code more consistent and fixes compilation on older Clang
versions.
Change-Id: I82abebd500e6651ac5c5b180cd7b49b4f20e8299
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5805956
Reviewed-by: Samuel Groß <[email protected]>
Commit-Queue: Michaël Zasso <[email protected]>
Reviewed-by: Stephen Röttger <[email protected]>
Cr-Commit-Position: refs/heads/main@{#95809}
Refs: v8/v8@01a47f3
PR-URL: #54536
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Original commit message:
aix: add work around for f16 type
AIX builds started to fail after this commit https://chromium.googlesource.com/v8/v8.git/+/d057564707d3a5df074b7f49a12a2f1e96638f94.
Change-Id: I25a5c4ae3b4fe5c27a9fb9e35e2bcd2bbed40351
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5789180
Reviewed-by: Clemens Backes <[email protected]>
Reviewed-by: Nico Hartmann <[email protected]>
Commit-Queue: Clemens Backes <[email protected]>
Cr-Commit-Position: refs/heads/main@{#95952}
Refs: v8/v8@97199f6
PR-URL: #54536
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
On AIX, we now include src/wasm/float16.h from within src/utils/utils.h and src/wasm/float16.h includes additional header files. PR-URL: #54536 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #54569 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>
PR-URL: #54849 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Daniel Lemire <[email protected]>
PR-URL: #54988 Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: Raz Luvaton <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
Fixes: #54898 PR-URL: #54970 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
Update the list of toolchains used to build the official Node.js binaries for Node.js 23 onwards. PR-URL: #54967 Refs: nodejs/build#3806 Refs: #54081 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
There is no recent trace of failure for this test. Fixes: #47116 PR-URL: #54976 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
PR-URL: #54977 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
PR-URL: #55072 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
This patch implements a "module-sync" exports condition
for packages to supply a sycnrhonous ES module to the
Node.js module loader, no matter it's being required
or imported. This is similar to the "module" condition
that bundlers have been using to support `require(esm)`
in Node.js, and allows dual-package authors to opt into
ESM-first only newer versions of Node.js that supports
require(esm) while avoiding the dual-package hazard.
```json
{
"type": "module",
"exports": {
"node": {
// On new version of Node.js, both require() and import get
// the ESM version
"module-sync": "./index.js",
// On older version of Node.js, where "module" and
// require(esm) are not supported, use the transpiled CJS version
// to avoid dual-package hazard. Library authors can decide
// to drop support for older versions of Node.js when they think
// it's time.
"default": "./dist/index.cjs"
},
// On any other environment, use the ESM version.
"default": "./index.js"
}
}
```
We end up implementing a condition with a different name
instead of reusing "module", because existing code in the
ecosystem using the "module" condition sometimes also expect
the module resolution for these ESM files to work in CJS
style, which is supported by bundlers, but the native
Node.js loader has intentionally made ESM resolution
different from CJS resolution (e.g. forbidding `import
'./noext'` or `import './directory'`), so it would be
semver-major to implement a `"module"` condition
without implementing the forbidden ESM resolution rules.
For now, this just implments a new condition as semver-minor
so it can be backported to older LTS.
Refs: https://webpack.js.org/guides/package-exports/#target-environment-independent-packages
PR-URL: #54648
Fixes: #52173
Refs: https://github.com/joyeecheung/test-module-condition
Refs: #52697
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Jan Krems <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
PR-URL: #54512 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: #54987 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
This reverts commit 2a871df. It has been almost three years since the test was marked flaky. Remove the unstable designation to see if the problem persists on Windows. Fixes: #40728 PR-URL: #55079 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Stefan Stojanovic <[email protected]>
PR-URL: #55043 Refs: #44862 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: #55065 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: #53757 Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: #54904 Reviewed-By: Matteo Collina <[email protected]>
Tests should pass even if the path where the repo is cloned contains URL-significant chars. PR-URL: #55082 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
PR-URL: #55083 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Tierney Cyren <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]> Reviewed-By: Chemi Atlow <[email protected]>
PR-URL: #55084 Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
PR-URL: #55087 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Refs: v8/v8@12.9.202.18...12.9.202.19 PR-URL: #55057 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Marco Ippolito <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]>
PR-URL: #55088 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Refs: #53382 TLS spec seems to indicate there should should be a response sent when TLS handshake fails. See https://datatracker.ietf.org/doc/html/rfc8446#page-85 When compiled with OpenSSL32 we see the the following response '15 03 03 00 02 02 16' which decodes as a fatal (0x02) TLS error alert number 22 (0x16). which corresponds to TLS1_AD_RECORD_OVERFLOW which matches the error we see if NODE_DEBUG is turned on once you get through the define aliases. If there is a response from the server the test used to hang because the end event will not be emitted until after the response is consumed. This PR fixes the test so it consumes the response. Some earlier OpenSSL versions did not seem to send a response but the error handling seems to have been re-written/improved in OpenSSL32. Signed-off-by: Michael Dawson <[email protected]> PR-URL: #55089 Refs: #52482 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Jithil P Ponnan <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
The native error messages can sometimes contain e.g. path content that are typically passed in as chars with UTF8-encoded code points. The native error throwing code previously always decode the chars as Latin-1, which would be incorrect. PR-URL: #55024 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: #55019 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Richard Lau <[email protected]>
Instead of relying on a timer, verify that `socket.end()` is called when the `'finish'` event is emitted on the `ServerResponse` object. PR-URL: #55004 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
- Extracts Blob.prototype.arrayBuffer so it cannot be overridden in .text(), etc. - Make .bytes() enumerable. I guess the WPT runner is not running the idlharness tests? - Make .text() return a Promise, rather than being explicitly async. This is a non-documented part of the webidl spec. Refs: #49936 - Have .text(), .arrayBuffer(), and .bytes() reject for an invalid this instead of throwing. Fix the tests regarding this. PR-URL: #53372 Refs: #49936 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Vinícius Lourenço Claro Cardoso <[email protected]>
Fixes: #47612 PR-URL: #47613 Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
haotaoyanzhuce
force-pushed
the
main
branch
from
e26d677
to
264d3ee
Compare
|
@RedYetiDev Hey brother, I modified the commit message. |
|
Unfortunately, you also appear to have included many unrelated commits. Please remove them before this can land, thank you! Try using |
|
@nodejs/build I'm not sure this change is even needed, WDYT |
To compile for the Android ARM platform on a Linux x86 platform, need gcc and g++. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@RedYetiDev #54908 Modify the description