Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

Allow cross compiling #150

Merged
merged 1 commit into from
Apr 2, 2021
Merged

Allow cross compiling #150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion lib/build.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import path from 'path';
import { spawnSync } from 'child_process';
import uniqueTempDir from 'unique-temp-dir';

import { hostPlatform } from './system';
import { hostArch, hostPlatform } from './system';
import { log } from './log';
import patchesJson from '../patches/patches.json';

Expand Down Expand Up @@ -108,6 +108,12 @@ async function compileOnUnix(nodeVersion: string, targetArch: string) {
args.push('--dest-cpu', cpu);
}

if (hostArch !== targetArch) {
log.warn('Cross compiling!');
log.warn('You are responsible for appropriate env like CC, CC_host, etc.');
Copy link
Member

Choose a reason for hiding this comment

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

This should link out to more information. What is CC, CC_host? Where can I learn more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately there is no documentation there. Even Node.js does not provide one on the topic of cross-compilation.

CC is "C Compiler".
CC_host is "C Compiler for the host".

Users are kinda expected to know this, if they wish to take advantage of this more advanced option. Of course, I am always open to better docs, but I don't think I can do a "cross compile 101" here.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe instead of making it working by default and show a warning would be better to force the flag to be there, if it is not there throw like it was before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would fail (very soon). So there is no need for that. That would also drastically increase the complexity of matrix-based CI workflows, as we probably don't want the flag when compile natively.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok LGTM for me so

Copy link
Member

Choose a reason for hiding this comment

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

Let's add the documentation to the README then, and then link to the readme in the error message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and, FYI: upstream (Node.js) builds and tests natively. See https://github.com/nodejs/build . They have native machines donated by IBM, ARM, etc. That explains why there is very little documentation on cross compilation.

There is no guaranteed good documentation here. Cross compilation is quite complicated. For instance, GCC vs LLVM: GCC uses separate binaries but LLVM uses --target=<triple> and other flags. And, the instructions for how to install a cross toolchain obviously vary a lot between distros.

I think it would be challenging to add any documentation, and we should probably follow the example of upstream, and ask advanced users to explore the codes, workflows and Google for answers instead.

args.push('--cross-compiling');
}

// first of all v8_inspector introduces the use
// of `prime_rehash_policy` symbol that requires
// GLIBCXX_3.4.18 on some systems
Expand Down
7 changes: 0 additions & 7 deletions lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import path from 'path';
import semver from 'semver';
import {
abiToNodeRange,
hostArch,
hostPlatform, // eslint-disable-line no-duplicate-imports
isValidNodeRange,
knownArchs,
Expand Down Expand Up @@ -148,12 +147,6 @@ export async function need(opts: NeedOptions) {
);
}

if (hostArch !== arch) {
throw wasReported(
`Not able to build for '${opts.arch}' here, only for '${hostArch}'`
);
}

if (knownArchs.indexOf(arch) < 0) {
throw wasReported(
`Unknown arch '${opts.arch}'. Specify ${knownArchs.join(', ')}`
Expand Down