Skip to content

Commit

Permalink
build: remove enable_run_as_node build flag (electron#38413)
Browse files Browse the repository at this point in the history
* feat: remove enable_run_as_node flag

* drop features.isRunAsNodeEnabled()

* use IsEnvSet() helper in electron_main_linux.cc

* cleanup [[maybe_unused]]

---------

Co-authored-by: Milan Burda <miburda@microsoft.com>
  • Loading branch information
2 people authored and MrHuangJser committed Dec 11, 2023
1 parent 39d1c4d commit 6656576
Show file tree
Hide file tree
Showing 18 changed files with 29 additions and 96 deletions.
29 changes: 3 additions & 26 deletions .circleci/config/base.yml
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,6 @@ env-mac-large-release: &env-mac-large-release
env-ninja-status: &env-ninja-status
NINJA_STATUS: "[%r processes, %f/%t @ %o/s : %es] "

env-disable-run-as-node: &env-disable-run-as-node
GN_BUILDFLAG_ARGS: 'enable_run_as_node = false'

env-32bit-release: &env-32bit-release
# Set symbol level to 1 for 32 bit releases because of https://crbug.com/648948
GN_BUILDFLAG_ARGS: 'symbol_level = 1'
Expand Down Expand Up @@ -254,7 +251,7 @@ step-depot-tools-get: &step-depot-tools-get
--- a/gclient.py
+++ b/gclient.py
@@ -712,7 +712,8 @@ class Dependency(gclient_utils.WorkItem, DependencySettings):
if dep_type == 'cipd':
cipd_root = self.GetCipdRoot()
- for package in dep_value.get('packages', []):
Expand Down Expand Up @@ -1058,7 +1055,7 @@ commands:
artifact-key:
type: string
build-type:
type: string
type: string
build-nonproprietary-ffmpeg:
type: boolean
default: true
Expand Down Expand Up @@ -1276,7 +1273,7 @@ commands:
artifact-key:
type: string
build-type:
type: string
type: string
after-build-and-save:
type: steps
default: []
Expand Down Expand Up @@ -1730,23 +1727,6 @@ jobs:
artifact-key: 'linux-x64-asan'
build-type: 'Linux'

linux-x64-testing-no-run-as-node:
executor:
name: linux-docker
size: xlarge
environment:
<<: *env-linux-2xlarge
<<: *env-testing-build
<<: *env-ninja-status
<<: *env-disable-run-as-node
GCLIENT_EXTRA_ARGS: '--custom-var=checkout_arm=True --custom-var=checkout_arm64=True'
steps:
- electron-build:
persist: false
checkout: true
artifact-key: 'linux-x64-no-run-as-node'
build-type: 'Linux'

linux-x64-testing-gn-check:
executor:
name: linux-docker
Expand Down Expand Up @@ -2230,9 +2210,6 @@ workflows:
- linux-x64-testing-asan:
requires:
- linux-make-src-cache
- linux-x64-testing-no-run-as-node:
requires:
- linux-make-src-cache
- linux-x64-testing-gn-check:
requires:
- linux-make-src-cache
Expand Down
7 changes: 0 additions & 7 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -669,13 +669,6 @@ source_set("electron_lib") {
]
}

if (enable_run_as_node) {
sources += [
"shell/app/node_main.cc",
"shell/app/node_main.h",
]
}

if (enable_osr) {
sources += [
"shell/browser/osr/osr_host_display_client.cc",
Expand Down
1 change: 0 additions & 1 deletion buildflags/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ buildflag_header("buildflags") {
header = "buildflags.h"

flags = [
"ENABLE_RUN_AS_NODE=$enable_run_as_node",
"ENABLE_OSR=$enable_osr",
"ENABLE_VIEWS_API=$enable_views_api",
"ENABLE_PDF_VIEWER=$enable_pdf_viewer",
Expand Down
3 changes: 0 additions & 3 deletions buildflags/buildflags.gni
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
# found in the LICENSE file.

declare_args() {
# Allow running Electron as a node binary.
enable_run_as_node = true

enable_osr = true

enable_views_api = true
Expand Down
2 changes: 2 additions & 0 deletions filenames.gni
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ filenames = {
"shell/app/electron_crash_reporter_client.h",
"shell/app/electron_main_delegate.cc",
"shell/app/electron_main_delegate.h",
"shell/app/node_main.cc",
"shell/app/node_main.h",
"shell/app/uv_task_runner.cc",
"shell/app/uv_task_runner.h",
"shell/browser/api/electron_api_app.cc",
Expand Down
3 changes: 0 additions & 3 deletions shell/app/electron_library_main.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,9 @@
#if BUILDFLAG(IS_MAC)
extern "C" {
__attribute__((visibility("default"))) int ElectronMain(int argc, char* argv[]);

#if BUILDFLAG(ENABLE_RUN_AS_NODE)
__attribute__((visibility("default"))) int ElectronInitializeICUandStartNode(
int argc,
char* argv[]);
#endif
}
#endif

Expand Down
2 changes: 0 additions & 2 deletions shell/app/electron_library_main.mm
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ int ElectronMain(int argc, char* argv[]) {
return content::ContentMain(std::move(params));
}

#if BUILDFLAG(ENABLE_RUN_AS_NODE)
int ElectronInitializeICUandStartNode(int argc, char* argv[]) {
if (!electron::fuses::IsRunAsNodeEnabled()) {
CHECK(false) << "run_as_node fuse is disabled";
Expand All @@ -43,4 +42,3 @@ int ElectronInitializeICUandStartNode(int argc, char* argv[]) {
base::i18n::InitializeICU();
return electron::NodeMain(argc, argv);
}
#endif
15 changes: 10 additions & 5 deletions shell/app/electron_main_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,23 @@
#include "shell/common/electron_command_line.h"
#include "shell/common/electron_constants.h"

namespace {

bool IsEnvSet(const char* name) {
char* indicator = getenv(name);
return indicator && indicator[0] != '\0';
}

} // namespace

int main(int argc, char* argv[]) {
FixStdioStreams();

#if BUILDFLAG(ENABLE_RUN_AS_NODE)
char* indicator = getenv(electron::kRunAsNode);
if (electron::fuses::IsRunAsNodeEnabled() && indicator &&
indicator[0] != '\0') {
if (electron::fuses::IsRunAsNodeEnabled() && IsEnvSet(electron::kRunAsNode)) {
base::i18n::InitializeICU();
base::AtExitManager atexit_manager;
return electron::NodeMain(argc, argv);
}
#endif

electron::ElectronMainDelegate delegate;
content::ContentMainParams params(&delegate);
Expand Down
4 changes: 1 addition & 3 deletions shell/app/electron_main_mac.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ void abort_report_np(const char* fmt, ...);

namespace {

[[maybe_unused]] bool IsEnvSet(const char* name) {
bool IsEnvSet(const char* name) {
char* indicator = getenv(name);
return indicator && indicator[0] != '\0';
}
Expand All @@ -53,12 +53,10 @@ int main(int argc, char* argv[]) {
partition_alloc::EarlyMallocZoneRegistration();
FixStdioStreams();

#if BUILDFLAG(ENABLE_RUN_AS_NODE)
if (electron::fuses::IsRunAsNodeEnabled() &&
IsEnvSet("ELECTRON_RUN_AS_NODE")) {
return ElectronInitializeICUandStartNode(argc, argv);
}
#endif

#if defined(HELPER_EXECUTABLE) && !IS_MAS_BUILD()
uint32_t exec_path_size = 0;
Expand Down
10 changes: 2 additions & 8 deletions shell/app/electron_main_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ namespace {
const char kUserDataDir[] = "user-data-dir";
const char kProcessType[] = "type";

[[maybe_unused]] bool IsEnvSet(const char* name) {
bool IsEnvSet(const char* name) {
size_t required_size;
getenv_s(&required_size, nullptr, 0, name);
return required_size != 0;
Expand Down Expand Up @@ -150,12 +150,8 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) {
}
#endif

#if BUILDFLAG(ENABLE_RUN_AS_NODE)
bool run_as_node =
electron::fuses::IsRunAsNodeEnabled() && IsEnvSet(electron::kRunAsNode);
#else
bool run_as_node = false;
#endif

// Make sure the output is printed to console.
if (run_as_node || !IsEnvSet("ELECTRON_NO_ATTACH_CONSOLE"))
Expand All @@ -164,15 +160,13 @@ int APIENTRY wWinMain(HINSTANCE instance, HINSTANCE, wchar_t* cmd, int) {
std::vector<char*> argv(arguments.argc);
std::transform(arguments.argv, arguments.argv + arguments.argc, argv.begin(),
[](auto& a) { return _strdup(base::WideToUTF8(a).c_str()); });
#if BUILDFLAG(ENABLE_RUN_AS_NODE)
if (electron::fuses::IsRunAsNodeEnabled() && run_as_node) {
if (run_as_node) {
base::AtExitManager atexit_manager;
base::i18n::InitializeICU();
auto ret = electron::NodeMain(argv.size(), argv.data());
std::for_each(argv.begin(), argv.end(), free);
return ret;
}
#endif

base::CommandLine::Init(argv.size(), argv.data());
const base::CommandLine* command_line =
Expand Down
6 changes: 0 additions & 6 deletions shell/common/api/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
// found in the LICENSE file.

#include "electron/buildflags/buildflags.h"
#include "electron/fuses.h"
#include "printing/buildflags/buildflags.h"
#include "shell/common/gin_helper/dictionary.h"
#include "shell/common/node_includes.h"
Expand All @@ -22,10 +21,6 @@ bool IsPDFViewerEnabled() {
return BUILDFLAG(ENABLE_PDF_VIEWER);
}

bool IsRunAsNodeEnabled() {
return electron::fuses::IsRunAsNodeEnabled() && BUILDFLAG(ENABLE_RUN_AS_NODE);
}

bool IsFakeLocationProviderEnabled() {
return BUILDFLAG(OVERRIDE_LOCATION_PROVIDER);
}
Expand Down Expand Up @@ -58,7 +53,6 @@ void Initialize(v8::Local<v8::Object> exports,
dict.SetMethod("isBuiltinSpellCheckerEnabled", &IsBuiltinSpellCheckerEnabled);
dict.SetMethod("isOffscreenRenderingEnabled", &IsOffscreenRenderingEnabled);
dict.SetMethod("isPDFViewerEnabled", &IsPDFViewerEnabled);
dict.SetMethod("isRunAsNodeEnabled", &IsRunAsNodeEnabled);
dict.SetMethod("isFakeLocationProviderEnabled",
&IsFakeLocationProviderEnabled);
dict.SetMethod("isViewApiEnabled", &IsViewApiEnabled);
Expand Down
10 changes: 2 additions & 8 deletions shell/common/crash_keys.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,8 @@ void GetCrashKeys(std::map<std::string, std::string>* keys) {

namespace {
bool IsRunningAsNode() {
#if BUILDFLAG(ENABLE_RUN_AS_NODE)
if (!electron::fuses::IsRunAsNodeEnabled())
return false;

return base::Environment::Create()->HasVar(electron::kRunAsNode);
#else
return false;
#endif
return electron::fuses::IsRunAsNodeEnabled() &&
base::Environment::Create()->HasVar(electron::kRunAsNode);
}
} // namespace

Expand Down
2 changes: 0 additions & 2 deletions shell/common/electron_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ const char kDeviceVendorIdKey[] = "vendorId";
const char kDeviceProductIdKey[] = "productId";
const char kDeviceSerialNumberKey[] = "serialNumber";

#if BUILDFLAG(ENABLE_RUN_AS_NODE)
const char kRunAsNode[] = "ELECTRON_RUN_AS_NODE";
#endif

#if BUILDFLAG(ENABLE_PDF_VIEWER)
const char kPDFExtensionPluginName[] = "Chromium PDF Viewer";
Expand Down
2 changes: 0 additions & 2 deletions shell/common/electron_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ extern const char kDeviceVendorIdKey[];
extern const char kDeviceProductIdKey[];
extern const char kDeviceSerialNumberKey[];

#if BUILDFLAG(ENABLE_RUN_AS_NODE)
extern const char kRunAsNode[];
#endif

#if BUILDFLAG(ENABLE_PDF_VIEWER)
extern const char kPDFExtensionPluginName[];
Expand Down
12 changes: 2 additions & 10 deletions spec/asar-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import { getRemoteContext, ifdescribe, ifit, itremote, useRemoteContext } from '
import * as importedFs from 'fs';
import { once } from 'events';

const features = process._linkedBinding('electron_common_features');

describe('asar package', () => {
const fixtures = path.join(__dirname, 'fixtures');
const asarDir = path.join(fixtures, 'test.asar');
Expand Down Expand Up @@ -1222,7 +1220,7 @@ describe('asar package', function () {
});
});

ifdescribe(features.isRunAsNodeEnabled())('child_process.fork', function () {
describe('child_process.fork', function () {
itremote('opens a normal js file', async function () {
const child = require('child_process').fork(path.join(asarDir, 'a.asar', 'ping.js'));
child.send('message');
Expand Down Expand Up @@ -1410,12 +1408,6 @@ describe('asar package', function () {

/*
describe('process.env.ELECTRON_NO_ASAR', function () {
before(function () {
if (!features.isRunAsNodeEnabled()) {
this.skip();
}
});
itremote('disables asar support in forked processes', function (done) {
const forked = ChildProcess.fork(path.join(__dirname, 'fixtures', 'module', 'no-asar.js'), [], {
env: {
Expand Down Expand Up @@ -1509,7 +1501,7 @@ describe('asar package', function () {
});

/*
ifit(features.isRunAsNodeEnabled())('is available in forked scripts', async function () {
it('is available in forked scripts', async function () {
const child = ChildProcess.fork(path.join(fixtures, 'module', 'original-fs.js'));
const message = once(child, 'message');
child.send('message');
Expand Down
5 changes: 2 additions & 3 deletions spec/modules-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { once } from 'events';

const Module = require('module');

const features = process._linkedBinding('electron_common_features');
const nativeModulesEnabled = !process.env.ELECTRON_SKIP_NATIVE_MODULE_TESTS;

describe('modules support', () => {
Expand All @@ -28,7 +27,7 @@ describe('modules support', () => {
).to.be.fulfilled();
});

ifit(features.isRunAsNodeEnabled())('can be required in node binary', async function () {
it('can be required in node binary', async function () {
const child = childProcess.fork(path.join(fixtures, 'module', 'echo.js'));
const [msg] = await once(child, 'message');
expect(msg).to.equal('ok');
Expand Down Expand Up @@ -60,7 +59,7 @@ describe('modules support', () => {
await expect(w.webContents.executeJavaScript('{ require(\'@electron-ci/uv-dlopen\'); null }')).to.be.fulfilled();
});

ifit(features.isRunAsNodeEnabled())('can be required in node binary', async function () {
it('can be required in node binary', async function () {
const child = childProcess.fork(path.join(fixtures, 'module', 'uv-dlopen.js'));
const [exitCode] = await once(child, 'exit');
expect(exitCode).to.equal(0);
Expand Down

0 comments on commit 6656576

Please sign in to comment.