Skip to content

Commit

Permalink
[0.72] Properly detect the correct machine architecture in CLI (#11987)
Browse files Browse the repository at this point in the history
This PR backports #11978 to RNW 0.72.

## Description

Node's `process.arch` and `os.arch()` methods both return the architecture that the node binaries were built for, not the architecture of the machine they're running.

This can cause issues with the various hardware virtualization layers, specifically when running 32-bit Node on 64-bit machines.

Closes: #11977

### Type of Change
- Bug fix (non-breaking change which fixes an issue)

### Why
We keep hitting issues when users run the 32-bit Node on 64-bit Windows configuration.

Closes: #11977

### What

This PR:
* Updates and exports the `deviceArchitecture()` API in the `@react-native-windows/telemetry` library, which now correctly detects when x86 Node is being used on an x64 machine
* Adds and exports a new `nodeArchitecture()` method, which is also now recorded in telemetry
* Updates the `react-native-windows-init` and `@react-native-windows/cli` commands to warn the user if there's a mismatch
* Fixes an issue where querying for a registry key would fail due to querying the wrong registry because of the SysWOW64 redirection

## Screenshots
N/A

## Testing

Updated the telemetry tests with a new test for `nodeArchitecture`.

## Changelog
Should this change be included in the release notes: Yes

Properly detect the correct machine architecture for 32-bit Node on 64-bit Windows scenarios
  • Loading branch information
jonthysell committed Aug 7, 2023
1 parent cef0edd commit 219790d
Show file tree
Hide file tree
Showing 10 changed files with 95 additions and 16 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "[0.72] Properly detect the correct machine architecture",
"packageName": "@react-native-windows/cli",
"email": "jthysell@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "[0.72] Properly detect the correct machine architecture",
"packageName": "@react-native-windows/telemetry",
"email": "jthysell@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "[0.72] Properly detect the correct machine architecture",
"packageName": "react-native-windows-init",
"email": "jthysell@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
* @format
*/

import os from 'os';
import {CommandOption} from '@react-native-community/cli-types';
import {deviceArchitecture} from '@react-native-windows/telemetry';

export type BuildArch = 'x86' | 'x64' | 'ARM64';
export type BuildConfig = 'Debug' | 'DebugBundle' | 'Release' | 'ReleaseBundle';
Expand Down Expand Up @@ -72,12 +72,7 @@ export const runWindowsOptions: CommandOption[] = [
{
name: '--arch [string]',
description: 'The build architecture (ARM64, x86, x64)',
default:
os.arch() === 'ia32'
? 'x86'
: os.arch() === 'arm64'
? 'ARM64'
: os.arch(),
default: parseBuildArch(deviceArchitecture()),
parse: parseBuildArch,
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ import {
OptionSanitizer,
configToProjectInfo,
getProjectFileFromConfig,
deviceArchitecture,
nodeArchitecture,
} from '@react-native-windows/telemetry';

import {newInfo, newWarn} from './commandWithProgress';

/**
* Calculates a the default values of a given react-native CLI command's options.
* @param config Config passed from react-native CLI.
Expand Down Expand Up @@ -89,13 +93,27 @@ export async function startTelemetrySession(
defaultOptions: CommanderOptionsType,
optionSanitizer: OptionSanitizer,
) {
const verbose = options.logging === true;

if (!options.telemetry) {
if (options.logging) {
console.log('Telemetry is disabled');
if (verbose) {
newInfo('Telemetry is disabled');
}
return;
}

if (verbose) {
newInfo(
`Running ${nodeArchitecture()} node on a ${deviceArchitecture()} machine`,
);
}

if (deviceArchitecture() !== nodeArchitecture()) {
newWarn(
'This version of node was built for a different architecture than this machine and may cause unintended behavior',
);
}

await Telemetry.setup();

const sanitizedOptions = commanderOptionsToOptions(options, optionSanitizer);
Expand Down
2 changes: 2 additions & 0 deletions packages/@react-native-windows/telemetry/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ export {
CommandEndInfo,
} from './telemetry';

export {deviceArchitecture, nodeArchitecture} from './utils/basePropUtils';

export {CodedError, CodedErrorType, CodedErrors} from './utils/errorUtils';

export {
Expand Down
2 changes: 2 additions & 0 deletions packages/@react-native-windows/telemetry/src/telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ export class Telemetry {
await basePropUtils.deviceId();
Telemetry.client!.commonProperties.deviceArchitecture =
basePropUtils.deviceArchitecture();
Telemetry.client!.commonProperties.nodeArchitecture =
basePropUtils.nodeArchitecture();
Telemetry.client!.commonProperties.devicePlatform =
basePropUtils.devicePlatform();
Telemetry.client!.commonProperties.deviceLocale =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ test('deviceArchitecture() is valid', () => {
expect(value).not.toBeNull();
});

test('nodeArchitecture() is valid', () => {
const value = basePropUtils.nodeArchitecture();
expect(value).toBeDefined();
expect(value).not.toBe('');
expect(value).not.toBeNull();
});

test('devicePlatform() is valid', () => {
const value = basePropUtils.devicePlatform();
expect(value).toBeDefined();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import {execSync} from 'child_process';
import {totalmem, cpus, arch, platform} from 'os';
import {totalmem, cpus, platform} from 'os';

import ci from 'ci-info';
import {randomBytes} from 'crypto';
Expand All @@ -20,9 +20,12 @@ const DeviceIdRegKey = 'MachineId';
*/
export async function deviceId(): Promise<string> {
try {
const output = execSync(
`${process.env.windir}\\System32\\reg.exe query ${DeviceIdRegPath} /v ${DeviceIdRegKey}`,
).toString();
let regCommand = `${process.env.windir}\\System32\\reg.exe query ${DeviceIdRegPath} /v ${DeviceIdRegKey}`;
if (deviceArchitecture() === 'x64') {
// Ensure we query the correct registry
regCommand += ' /reg:64';
}
const output = execSync(regCommand).toString();

const result = output.match(/\{([0-9A-Fa-f-]{36})\}/);
if (result && result.length > 1) {
Expand All @@ -33,11 +36,26 @@ export async function deviceId(): Promise<string> {
}

/**
* Gets the device architecture, like x64/arm64.
* Gets the device architecture, like x86/x64/arm64.
* @returns The device architecture.
*/
export function deviceArchitecture(): string {
return arch();
const nodeArch = nodeArchitecture();

// Check if we're running x86 node on x64 hardware
if (nodeArch === 'x86' && process.env.PROCESSOR_ARCHITEW6432 === 'AMD64') {
return 'x64';
}

return nodeArch;
}

/**
* Gets the node architecture, like x86/x64/arm64.
* @returns The node architecture.
*/
export function nodeArchitecture(): string {
return process.arch === 'ia32' ? 'x86' : process.arch;
}

/**
Expand Down
18 changes: 17 additions & 1 deletion packages/react-native-windows-init/src/Cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import {
getProjectFileFromConfig,
OptionSanitizer,
YargsOptionsType,
deviceArchitecture,
nodeArchitecture,
} from '@react-native-windows/telemetry';

/**
Expand Down Expand Up @@ -373,13 +375,27 @@ async function startTelemetrySession(
args: string[],
options: YargsOptionsType,
) {
const verbose = options.verbose === true;

if (!options.telemetry) {
if (options.verbose) {
if (verbose) {
console.log('Telemetry is disabled');
}
return;
}

if (verbose) {
console.log(
`Running ${nodeArchitecture()} node on a ${deviceArchitecture()} machine`,
);
}

if (deviceArchitecture() !== nodeArchitecture()) {
console.warn(
'This version of node was built for a different architecture than this machine and may cause unintended behavior',
);
}

// Setup telemetry, but don't get NPM package version info right away as
// we're going to change things and this may interfere with the resolver
await Telemetry.setup({populateNpmPackageVersions: false});
Expand Down

0 comments on commit 219790d

Please sign in to comment.