(xcode) feat: --cross-compiling overrides arch-specific settings

[rvagg]

Continuing from nodejs/build#2474 (comment)

GYP_CROSSCOMPILE isn't used in the Xcode path at all, so this shouldn't conflict with anything afaik. This feels like a bit of a hack but elegance is beyond me for this. The problem we have is that -arch X is provided to all compile commands via CFLAGS_Release and friends, regardless of host or target, and you can't override it by any other means, even if you can sneak in extra args via CC.target, CC.host and friends.

I think this wouldn't be a problem if we didn't need the built-time tooling to be runnable on the host arch. So even though there's logic in there for "iphone" etc., those assume a single compile target, not the intermediate host compile for build-time tooling. I think. But maybe you folks can have a bit of a look to see if there's a more elegant way or something I'm overlooking here?

[rvagg]

(view whitespace changes off btw https://github.com/nodejs/gyp-next/pull/78/files?diff=unified&w=1)

[bcoe]

@rvagg sorry for being a pendant, but for the parser to pick up this change for a release, you'll want to go with:

feat(xcode): --cross-compiling overrides arch-specific settings

When you land 👍

[rvagg]

pointer appreciated, have amended and force pushed, thanks @bcoe

[gengjiawen]

LGTM

[gengjiawen]

cc @MarshallOfSound @targos

[MarshallOfSound]

Can we validate this doesn't break existing X-compilatiom of native add-ons as that currently works. If no one gets to it I can test it later this week. Also unclear to me how X-compilation of native add-ons works but doesn't for node itself, they both use the same gyp-next toolchain right?

[gengjiawen]

Can we validate this doesn't break existing X-compilatiom of native add-ons as that currently works. If no one gets to it I can test it later this week.

This will be great.

Also unclear to me how X-compilation of native add-ons works but doesn't for node itself, they both use the same gyp-next toolchain right?

Yeap. But gyp-next in node only responsible for build Node.js itself.

[rvagg]

Can we validate this doesn't break existing X-compilatiom of native add-ons as that currently works

For cross compiling on macos? Or in general? Currently the only cross that should be supported on macos (using the xcode path) is i386. But if you're successfully cross compiling anything else on macos then that'd be interesting to hear about! Perhaps I've missed something.

[gengjiawen]

@MarshallOfSound ping.

[gengjiawen]

@MarshallOfSound friendly ping again.

[MarshallOfSound]

Ah sorry folks, this got drowned out in my other GH notifications.

For cross compiling on macos? Or in general? Currently the only cross that should be supported on macos (using the xcode path) is i386. But if you're successfully cross compiling anything else on macos then that'd be interesting to hear about! Perhaps I've missed something.

We X-compile arm64 from x64 at the moment using the xcode path.

Gave this a quick test going both arm64 from x64 and arm64 via Rosetta on an M1 and it doesn't look like it messes with that so LGTM 👍

[gengjiawen]

Ah sorry folks, this got drowned out in my other GH notifications.

For cross compiling on macos? Or in general? Currently the only cross that should be supported on macos (using the xcode path) is i386. But if you're successfully cross compiling anything else on macos then that'd be interesting to hear about! Perhaps I've missed something.

We X-compile arm64 from x64 at the moment using the xcode path.

Gave this a quick test going both arm64 from x64 and arm64 via Rosetta on an M1 and it doesn't look like it messes with that so LGTM 👍

Thanks for the test :)

[staltz]

TL;DR this commit broke compilation of nodejs-mobile for iOS from an x64 macBook. See PR nodejs-mobile/nodejs-mobile#9

(I'm not sure if it's good practice to comment on old PRs, so feel free to tell me to move this discussion elsewhere.)

I maintain nodejs-mobile lately, and I'm updating our fork from Node 12.x to Node 16.x. Since we're compiling node.js for iOS, we want the arch to be arm64, but the host clang has flags like __amd64__ and this mixed with the iPhoneOS SDK headers ends up causing an Unsupported architecture compilation error.

Maybe a solution would be to patch this gyp-next script to detect whether we're running in arm64 darwin (such checks are common throughout the codebase, I've noticed) and then use the new logic. Else, use the old logic.

What do you think?