From df37fca0113f9e7e7536185cb907e2e6ca9e33f2 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Tue, 3 Sep 2019 13:06:09 -0700 Subject: [PATCH] feat: add default DEBUG and VERBOSE_LOGS configuration_env_vars to nodejs_binary (#1080) - all rules updated to use DEBUG and VERBOSE_LOGS environment variables - added golden_debug attribute golden_file_test to support the case where a rule has different output if DEBUG is set; for example ``` golden_file_test( name = "test", actual = "out.min.js", golden = "output.golden.js_", golden_debug = "output.debug.golden.js_", ) ``` --- .bazelci/presubmit.yml | 10 +++ .circleci/config.yml | 2 +- DEVELOPING.md | 4 + common.bazelrc | 27 ++++++- docs/Terser.md | 4 +- docs/index.md | 34 ++++++++ .../bazel_integration_test/test_runner.js | 45 ++++++----- internal/golden_file_test/bin.js | 7 +- .../golden_file_test/golden_file_test.bzl | 14 +++- internal/node/node.bzl | 15 +++- internal/node/node_loader.js | 79 ++++++++----------- internal/npm_install/browserify-wrapped.js | 12 +-- internal/npm_install/generate_build_file.js | 21 +++-- .../npm_install/pre_process_package_json.js | 8 +- internal/rollup/rollup.config.js | 27 +++---- internal/rollup/terser-wrapped.js | 14 ++-- internal/rollup/tsc-directory.js | 12 +-- packages/karma/src/karma.conf.js | 27 +++---- packages/protractor/src/protractor.conf.js | 16 ++-- packages/terser/src/terser_minified.bzl | 4 +- packages/terser/test/debug/BUILD.bazel | 4 +- packages/terser/test/inline_sourcemap/spec.js | 4 +- packages/terser/test/user_config/BUILD.bazel | 1 + .../test/user_config/output.debug.golden.js_ | 3 + 24 files changed, 250 insertions(+), 144 deletions(-) create mode 100644 packages/terser/test/user_config/output.debug.golden.js_ diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 33e5a67b01..41bf2022a4 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -106,6 +106,16 @@ tasks: - "--test_tag_filters=-e2e,-examples,-fix-bazelci-ubuntu" test_targets: - "//..." + ubuntu1804_debug: + name: ubuntu1804_debug + platform: ubuntu1804 + test_flags: + - "--define=VERBOSE_LOGS=1" + - "--define=DEBUG=1" + - "--test_tag_filters=-e2e,-examples,-fix-bazelci-ubuntu" + test_targets: + - "//..." + - "//packages/terser/test/debug:test_define_DEBUG" ubuntu1804_e2e: name: ubuntu1804_e2e platform: ubuntu1804 diff --git a/.circleci/config.yml b/.circleci/config.yml index 9b20f24ded..9c6a14e968 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -152,7 +152,7 @@ jobs: - *init_bazel - *hide_node_and_yarn_local_binaries - - run: bazel test ... --test_tag_filters=-e2e,-examples + - run: bazel test --test_tag_filters=-e2e,-examples ... - store_artifacts: path: dist/bin/release.tar.gz diff --git a/DEVELOPING.md b/DEVELOPING.md index 4702ed98f0..bd85bd779e 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -24,6 +24,10 @@ To do a full clean run: yarn clean_all ``` +## Debugging + +See `Debugging` section under `/docs/index.md`. + ## Releasing Start from a clean checkout at master/HEAD. diff --git a/common.bazelrc b/common.bazelrc index f561c064ba..07a3678690 100644 --- a/common.bazelrc +++ b/common.bazelrc @@ -26,7 +26,32 @@ test --test_output=errors # Support for debugging NodeJS tests # Add the Bazel option `--config=debug` to enable this -test:debug --test_arg=--node_options=--inspect-brk --test_output=streamed --test_strategy=exclusive --test_timeout=9999 --nocache_test_results +# --test_output=streamed +# Stream stdout/stderr output from each test in real-time. +# See https://docs.bazel.build/versions/master/user-manual.html#flag--test_output for more details. +# --test_strategy=exclusive +# Run one test at a time. +# --test_timeout=9999 +# Prevent long running tests from timing out +# See https://docs.bazel.build/versions/master/user-manual.html#flag--test_timeout for more details. +# --nocache_test_results +# Always run tests +# --node_options=--inspect-brk +# Pass the --inspect-brk option to all tests which enables the node inspector agent. +# See https://nodejs.org/de/docs/guides/debugging-getting-started/#command-line-options for more details. +# --define=VERBOSE_LOGS=1 +# Rules will output verbose logs if the VERBOSE_LOGS environment variable is set. `VERBOSE_LOGS` will be passed to +# `nodejs_binary` and `nodejs_test` via the default value of the `default_env_vars` attribute of those rules. +# --define=DEBUG=1 +# Rules may change their build outputs if the DEBUG environment variable is set. For example, +# mininfiers such as terser may make their output more human readable when this is set. `DEBUG` will be passed to +# `nodejs_binary` and `nodejs_test` via the default value of the `default_env_vars` attribute of those rules. +test:debug --test_output=streamed --test_strategy=exclusive --test_timeout=9999 --nocache_test_results --define=VERBOSE_LOGS=1 +# Use bazel run with `--config=debug` to turn on the NodeJS inspector agent. +# The node process will break before user code starts and wait for the debugger to connect. +run:debug --define=VERBOSE_LOGS=1 -- --node_options=--inspect-brk +# The following option will change the build output of certain rules such as terser and may not be desirable in all cases +build:debug --define=DEBUG=1 # Turn off legacy external runfiles # This prevents accidentally depending on this feature, which Bazel will remove. diff --git a/docs/Terser.md b/docs/Terser.md index 8b271c3829..8ce0758657 100755 --- a/docs/Terser.md +++ b/docs/Terser.md @@ -87,7 +87,7 @@ Bazel will make a copy of your config file, treating it as a template. If you use the magic strings `"bazel_debug"` or `"bazel_no_debug"`, these will be replaced with `true` and `false` respecting the value of the `debug` attribute -or the `--define=DEBUG=true` bazel flag. +or the `--define=DEBUG=1` bazel flag. For example, @@ -107,7 +107,7 @@ If `config_file` isn't supplied, Bazel will use a default config file. (*Boolean*): Configure terser to produce more readable output. Instead of setting this attribute, consider setting the DEBUG variable instead -bazel build --define=DEBUG=true //my/terser:target +bazel build --define=DEBUG=1 //my/terser:target so that it only affects the current build. diff --git a/docs/index.md b/docs/index.md index b7274ca1dd..752b1184a5 100644 --- a/docs/index.md +++ b/docs/index.md @@ -90,6 +90,40 @@ See the `examples/program` directory in this repository. The `examples/program/index.spec.js` file illustrates testing. Another usage is in https://github.com/angular/tsickle/blob/master/test/BUILD +### Debugging + +Add the options in the `Support for debugging NodeJS tests` section from https://github.com/bazelbuild/rules_nodejs/blob/master/common.bazelrc to your project's `.bazelrc` file to add support for debugging NodeJS programs. + +Using the `--config=debug` command line option with bazel will set a number of flags that are specified there are useful for debugging. See the comments under `Support for debugging NodeJS tests` for details on the flags that are set. + +Use `--config=debug` with `bazel test` as follow, + +``` +bazel test --config=debug //test:... +``` + +or with `bazel run`, + +``` +bazel run --config=debug //test:test1 +``` + +to also turn on the NodeJS inspector agent which will break before any user code starts. You should then see, + +``` +Executing tests from //test:test1 +----------------------------------------------------------------------------- +Debugger listening on ws://127.0.0.1:9229/3f20777a-242c-4d18-b88b-5ed4b3fed61c +For help, see: https://nodejs.org/en/docs/inspector +``` + +when the test is run. + +To inspect with Chrome DevTools 55+, open `chrome://inspect` in a Chromium-based browser and attach to the waiting process. +A Chrome DevTools window should open and you should see `Debugger attached.` in the console. + +See https://nodejs.org/en/docs/guides/debugging-getting-started/ for more details. + ### Stamping Bazel is generally only a build tool, and is unaware of your version control system. diff --git a/internal/bazel_integration_test/test_runner.js b/internal/bazel_integration_test/test_runner.js index f51cc41ee2..783929d437 100644 --- a/internal/bazel_integration_test/test_runner.js +++ b/internal/bazel_integration_test/test_runner.js @@ -15,8 +15,6 @@ * limitations under the License. */ -const DEBUG = false; - // Set TEST_MANIFEST to true and use `bazel run` to excersize the MANIFEST // file code path on Linux and OSX const TEST_MANIFEST = false; @@ -26,17 +24,28 @@ const fs = require('fs'); const path = require('path'); const tmp = require('tmp'); +const DEBUG = !!process.env['DEBUG']; +const VERBOSE_LOGS = !!process.env['VERBOSE_LOGS']; + +function log(...m) { + console.error('[test_runner.js]', ...m); +} + +function log_verbose(...m) { + if (VERBOSE_LOGS) console.error('[test_runner.js]', ...m); +} + const config = require(process.argv[2]); -if (DEBUG) console.log(`config: ${JSON.stringify(config, null, 2)}`); +log_verbose(`config: ${JSON.stringify(config, null, 2)}`); const testArgs = process.argv.slice(3); -if (DEBUG) console.log(`testArgs: ${JSON.stringify(testArgs, null, 2)}`); +log_verbose(`testArgs: ${JSON.stringify(testArgs, null, 2)}`); /** - * Helper function to log out the contents of a file. + * Helper function to debug log out the contents of a file. */ function logFileContents(desc, contents) { - console.log(`\n\n${ + log_verbose(`${ desc}\n========================================================================================\n${ contents}\n^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n`); } @@ -82,7 +91,7 @@ function copyFolderSync(from, to) { if (fs.statSync(src).isFile()) { mkdirp(path.dirname(dest)); fs.copyFileSync(src, dest); - if (DEBUG) console.log(`copying ${src} -> ${dest}`); + log_verbose(`copying ${src} -> ${dest}`); } else { copyFolderSync(src, dest); } @@ -130,7 +139,7 @@ function copyWorkspace(workspace) { const element = key.slice(start.length); const dest = path.posix.join(to, element); mkdirp(path.dirname(dest)); - if (DEBUG) console.log(`copying (MANIFEST) ${RUNFILES_MANIFEST[key]} -> ${dest}`); + log_verbose(`copying (MANIFEST) ${RUNFILES_MANIFEST[key]} -> ${dest}`); fs.copyFileSync(RUNFILES_MANIFEST[key], dest); } } @@ -158,7 +167,7 @@ function copyNpmPackage(packagePath) { return to; } -if (DEBUG) console.log(`\n\ncopying workspace under test ${config.workspaceUnderTest} to tmp`); +log_verbose(`copying workspace under test ${config.workspaceUnderTest} to tmp`); const workspaceRoot = copyWorkspace(config.workspaceUnderTest); // Handle .bazelrc import replacements @@ -172,7 +181,7 @@ if (bazelrcImportsKeys.length && isFile(bazelrcFile)) { bazelrcContents = bazelrcContents.replace(importKey, importContents); } fs.writeFileSync(bazelrcFile, bazelrcContents); - if (DEBUG) logFileContents('.bazelrc file with replacements:', bazelrcContents); + logFileContents('.bazelrc file with replacements:', bazelrcContents); } // Handle appending to .bazelrc @@ -182,7 +191,7 @@ if (config.bazelrcAppend) { bazelrcContents += '\n\n# Appended by bazel_integration_test\n'; bazelrcContents += config.bazelrcAppend; fs.writeFileSync(bazelrcFile, bazelrcContents); - if (DEBUG) logFileContents('.bazelrc file after appending:', bazelrcContents); + logFileContents('.bazelrc file after appending:', bazelrcContents); } // Handle WORKSPACE replacements @@ -206,7 +215,7 @@ if (config.bazelrcAppend) { } } fs.writeFileSync(workspaceFile, workspaceContents); - if (DEBUG) logFileContents('WORKSPACE file with replacements:', workspaceContents); + logFileContents('WORKSPACE file with replacements:', workspaceContents); } // Handle package.json replacements @@ -217,7 +226,7 @@ if (isFile(packageJsonFile)) { const npmPackageKeys = Object.keys(config.npmPackages); if (npmPackageKeys.length) { for (const packageJsonKey of npmPackageKeys) { - if (DEBUG) console.log(`\n\ncopying npm package ${packageJsonKey} to tmp`); + log_verbose(`copying npm package ${packageJsonKey} to tmp`); const packagePath = copyNpmPackage(config.npmPackages[packageJsonKey]).replace(/\\/g, '/'); const regex = new RegExp(`\"${packageJsonKey}\"\\s*\:\\s*\"[^"]+`) const replacement = `"${packageJsonKey}": "file:${packagePath}`; @@ -255,21 +264,21 @@ if (isFile(packageJsonFile)) { } } - if (DEBUG) logFileContents('package.json file with replacements:', packageJsonContents); + logFileContents('package.json file with replacements:', packageJsonContents); } const isWindows = process.platform === 'win32'; const bazelBinary = require.resolve(`${config.bazelBinaryWorkspace}/bazel${isWindows ? '.exe' : ''}`); -console.log(`\n\nRunning 'bazel version'`); +log(`running 'bazel version'`); let spawnedProcess = spawnSync(bazelBinary, ['version'], {cwd: workspaceRoot, stdio: 'inherit'}); if (spawnedProcess.status) { process.exit(spawnedProcess.status); } -if (DEBUG) { - console.log(`\n\nRunning 'bazel info'`); +if (VERBOSE_LOGS) { + log_verbose(`running 'bazel info'`); spawnedProcess = spawnSync(bazelBinary, ['info'], {cwd: workspaceRoot, stdio: 'inherit'}); if (spawnedProcess.status) { process.exit(spawnedProcess.status); @@ -286,7 +295,7 @@ for (const bazelCommand of config.bazelCommands) { } else { bazelArgs.push(...testArgs); } - console.log(`\n\nRunning 'bazel ${bazelArgs.join(' ')}'`); + log(`running 'bazel ${bazelArgs.join(' ')}'`); spawnedProcess = spawnSync(bazelBinary, bazelArgs, {cwd: workspaceRoot, stdio: 'inherit'}); if (spawnedProcess.status) { process.exit(spawnedProcess.status); diff --git a/internal/golden_file_test/bin.js b/internal/golden_file_test/bin.js index 3e7602b4e8..f40560cd60 100644 --- a/internal/golden_file_test/bin.js +++ b/internal/golden_file_test/bin.js @@ -3,7 +3,9 @@ const path = require('path'); const unidiff = require('unidiff'); function main(args) { - const [mode, golden, actual] = args; + const [mode, golden_no_debug, golden_debug, actual] = args; + const debug = !!process.env['DEBUG']; + const golden = debug ? golden_debug : golden_no_debug; const actualContents = fs.readFileSync(require.resolve(actual), 'utf-8').replace(/\r\n/g, '\n'); const goldenContents = fs.readFileSync(require.resolve(golden), 'utf-8').replace(/\r\n/g, '\n'); @@ -22,7 +24,8 @@ ${prettyDiff} Update the golden file: - bazel run ${process.env['BAZEL_TARGET'].replace(/_bin$/, '')}.accept + bazel run ${debug ? '--define=DEBUG=1 ' : ''}${ + process.env['BAZEL_TARGET'].replace(/_bin$/, '')}.accept `); } else { throw new Error('unknown mode', mode); diff --git a/internal/golden_file_test/golden_file_test.bzl b/internal/golden_file_test/golden_file_test.bzl index d2eca2ef11..f7d091da45 100644 --- a/internal/golden_file_test/golden_file_test.bzl +++ b/internal/golden_file_test/golden_file_test.bzl @@ -3,13 +3,23 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary", "nodejs_test") def golden_file_test(name, golden, actual, **kwargs): + """Tests an actual output against a golden output. + +Use `golden_debug` if the actual output changes when DEBUG is set. +""" data = [golden, actual, "@npm//unidiff"] + golden_debug = kwargs.pop("golden_debug", []) + if golden_debug: + data.extend([golden_debug]) + else: + golden_debug = golden + loc = "$(location %s)" nodejs_test( name = name, entry_point = "@build_bazel_rules_nodejs//internal/golden_file_test:bin.js", - templated_args = ["--verify", loc % golden, loc % actual], + templated_args = ["--verify", loc % golden, loc % golden_debug, loc % actual], data = data, **kwargs ) @@ -18,7 +28,7 @@ def golden_file_test(name, golden, actual, **kwargs): name = name + ".accept", testonly = True, entry_point = "@build_bazel_rules_nodejs//internal/golden_file_test:bin.js", - templated_args = ["--out", loc % golden, loc % actual], + templated_args = ["--out", loc % golden, loc % golden_debug, loc % actual], data = data, **kwargs ) diff --git a/internal/node/node.bzl b/internal/node/node.bzl index 03d5cd4631..fb36c3abb0 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -156,7 +156,7 @@ def _nodejs_binary_impl(ctx): ctx.outputs.loader.short_path, ]) env_vars = "export BAZEL_TARGET=%s\n" % ctx.label - for k in ctx.attr.configuration_env_vars: + for k in ctx.attr.configuration_env_vars + ctx.attr.default_env_vars: if k in ctx.var.keys(): env_vars += "export %s=\"%s\"\n" % (k, ctx.var[k]) @@ -269,6 +269,19 @@ _NODEJS_EXECUTABLE_ATTRS = { allow_files = True, aspects = [sources_aspect, module_mappings_runtime_aspect, collect_node_modules_aspect], ), + "default_env_vars": attr.string_list( + doc = """Default environment variables that are added to `configuration_env_vars`. + +This is separate from the default of `configuration_env_vars` so that a user can set `configuration_env_vars` +without losing the defaults that should be set in most cases. + +The set of default environment variables is: + +`DEBUG`: rules use this environment variable to turn on debug information in their output artifacts +`VERBOSE_LOGS`: rules use this environment variable to turn on debug output in their logs +""", + default = ["DEBUG", "VERBOSE_LOGS"], + ), "entry_point": attr.label( doc = """The script which should be executed first, usually containing a main function. diff --git a/internal/node/node_loader.js b/internal/node/node_loader.js index d2034ff760..7a02c7cdd3 100644 --- a/internal/node/node_loader.js +++ b/internal/node/node_loader.js @@ -29,7 +29,12 @@ const isWindows = /^win/i.test(process.platform); // Ensure that node is added to the path for any subprocess calls process.env.PATH = [path.dirname(process.execPath), process.env.PATH].join(isWindows ? ';' : ':'); -const DEBUG = false; +const VERBOSE_LOGS = !!process.env['VERBOSE_LOGS']; + +function log_verbose(...m) { + // This is a template file so we use __filename to output the actual filename + if (VERBOSE_LOGS) console.error(`[${path.basename(__filename)}]`, ...m); +} /** * The module roots as pairs of a RegExp to match the require path, and a @@ -54,9 +59,7 @@ const GEN_DIR = 'TEMPLATED_gen_dir'; const INSTALL_SOURCE_MAP_SUPPORT = TEMPLATED_install_source_map_support; const TARGET = 'TEMPLATED_target'; -if (DEBUG) - console.error(` -node_loader: running ${TARGET} with +log_verbose(`running ${TARGET} with cwd: ${process.cwd()} runfiles: ${process.env.RUNFILES} @@ -98,7 +101,7 @@ function resolveToModuleRoot(path) { * See https://github.com/bazelbuild/bazel/issues/3726 */ function loadRunfilesManifest(manifestPath) { - if (DEBUG) console.error(`node_loader: using manifest ${manifestPath}`); + log_verbose(`using manifest ${manifestPath}`); // Create the manifest and reverse manifest maps. const runfilesManifest = Object.create(null); @@ -146,9 +149,9 @@ function loadRunfilesManifest(manifestPath) { genRoot = `${execRoot}${GEN_DIR}/`; } - if (DEBUG) console.error(`node_loader: using binRoot ${binRoot}`); - if (DEBUG) console.error(`node_loader: using genRoot ${genRoot}`); - if (DEBUG) console.error(`node_loader: using localWorkspacePath ${localWorkspacePath}`); + log_verbose(`using binRoot ${binRoot}`); + log_verbose(`using genRoot ${genRoot}`); + log_verbose(`using localWorkspacePath ${localWorkspacePath}`); return { runfilesManifest, reverseRunfilesManifest, binRoot, genRoot, localWorkspacePath }; } @@ -311,31 +314,31 @@ function resolveRunfiles(parent, ...pathSegments) { // Normalize and replace path separators to conform to the ones in the manifest. runfilesEntry = path.normalize(runfilesEntry).replace(/\\/g, '/'); - if (DEBUG) console.error('node_loader: try to resolve in runfiles manifest', runfilesEntry); + log_verbose('try to resolve in runfiles manifest', runfilesEntry); let maybe = resolveManifestFile(runfilesEntry); if (maybe) { - if (DEBUG) console.error('node_loader: resolved manifest file', maybe); + log_verbose('resolved manifest file', maybe); return maybe; } maybe = resolveManifestDirectory(runfilesEntry); if (maybe) { - if (DEBUG) console.error('node_loader: resolved via manifest directory', maybe); + log_verbose('resolved via manifest directory', maybe); return maybe; } } else { - if (DEBUG) console.error('node_loader: try to resolve in runfiles', runfilesPath); + log_verbose('try to resolve in runfiles', runfilesPath); let maybe = loadAsFileSync(runfilesPath); if (maybe) { - if (DEBUG) console.error('node_loader: resolved file', maybe); + log_verbose('resolved file', maybe); return maybe; } maybe = loadAsDirectorySync(runfilesPath); if (maybe) { - if (DEBUG) console.error('node_loader: resolved via directory', maybe); + log_verbose('resolved via directory', maybe); return maybe; } } @@ -346,7 +349,7 @@ function resolveRunfiles(parent, ...pathSegments) { var originalResolveFilename = module.constructor._resolveFilename; module.constructor._resolveFilename = function(request, parent, isMain, options) { const parentFilename = (parent && parent.filename) ? parent.filename : undefined; - if (DEBUG) console.error(`\n\nnode_loader: resolve ${request} from ${parentFilename}`); + log_verbose(`resolve ${request} from ${parentFilename}`); const failedResolutions = []; @@ -378,11 +381,9 @@ module.constructor._resolveFilename = function(request, parent, isMain, options) const resolved = originalResolveFilename(request, parent, isMain, options); if (resolved === request || request.startsWith('.') || request.startsWith('/') || request.match(/^[A-Z]\:[\\\/]/i)) { - if (DEBUG) - console.error( - `node_loader: resolved ${request} to built-in, relative or absolute import ` + - `${resolved} from ${parentFilename}` - ); + log_verbose( + `resolved ${request} to built-in, relative or absolute import ` + + `${resolved} from ${parentFilename}`); return resolved; } else { // Resolved is not a built-in module, relative or absolute import @@ -395,11 +396,9 @@ module.constructor._resolveFilename = function(request, parent, isMain, options) const relative = path.relative(parentRoot, resolved); if (!relative.startsWith('..')) { // Resolved within parent node_modules - if (DEBUG) - console.error( - `node_loader: resolved ${request} within parent node_modules to ` + - `${resolved} from ${parentFilename}` - ); + log_verbose( + `resolved ${request} within parent node_modules to ` + + `${resolved} from ${parentFilename}`); return resolved; } else { throw new Error( @@ -416,10 +415,7 @@ module.constructor._resolveFilename = function(request, parent, isMain, options) // dependency of an npm package, attempt to resolve against the runfiles location try { const resolved = originalResolveFilename(resolveRunfiles(parentFilename, request), parent, isMain, options); - if (DEBUG) - console.error( - `node_loader: resolved ${request} within runfiles to ${resolved} from ${parentFilename}` - ); + log_verbose(`resolved ${request} within runfiles to ${resolved} from ${parentFilename}`); return resolved; } catch (e) { failedResolutions.push(`runfiles - ${e.toString()}`); @@ -441,11 +437,9 @@ module.constructor._resolveFilename = function(request, parent, isMain, options) if (parentSegments[0] !== USER_WORKSPACE_NAME) { try { const resolved = originalResolveFilename(resolveRunfiles(undefined, parentSegments[0], 'node_modules', request), parent, isMain, options); - if (DEBUG) - console.error( - `node_loader: resolved ${request} within node_modules ` + - `(${parentSegments[0]}/node_modules) to ${resolved} from ${relativeParentFilename}` - ); + log_verbose( + `resolved ${request} within node_modules ` + + `(${parentSegments[0]}/node_modules) to ${resolved} from ${relativeParentFilename}`); return resolved; } catch (e) { failedResolutions.push(`${parentSegments[0]}/node_modules - ${e.toString()}`); @@ -457,11 +451,9 @@ module.constructor._resolveFilename = function(request, parent, isMain, options) // within the node_modules filegroup in use try { const resolved = originalResolveFilename(resolveRunfiles(undefined, NODE_MODULES_ROOT, request), parent, isMain, options); - if (DEBUG) - console.error( - `node_loader: resolved ${request} within node_modules (${NODE_MODULES_ROOT}) to ` + - `${resolved} from ${parentFilename}` - ); + log_verbose( + `resolved ${request} within node_modules (${NODE_MODULES_ROOT}) to ` + + `${resolved} from ${parentFilename}`); return resolved; } catch (e) { failedResolutions.push(`node_modules attribute (${NODE_MODULES_ROOT}) - ${e.toString()}`); @@ -471,7 +463,7 @@ module.constructor._resolveFilename = function(request, parent, isMain, options) // See https://github.com/bazelbuild/rules_nodejs/issues/1015 let moduleNotFoundError = `Cannot find module '${request}'. ` + 'Please verify that the package.json has a valid "main" entry'; - if (DEBUG) { + if (VERBOSE_LOGS) { moduleNotFoundError += `\nrequired in target ${TARGET} by '${parentFilename}'\n looked in:\n` + failedResolutions.map(r => ` ${r}`).join('\n') + '\n'; } @@ -493,12 +485,9 @@ if (INSTALL_SOURCE_MAP_SUPPORT) { '../build_bazel_rules_nodejs/third_party/github.com/source-map-support'); require(sourcemap_support_package).install(); } catch (_) { - if (DEBUG) { - console.error(`WARNING: source-map-support module not installed. + log_verbose(`WARNING: source-map-support module not installed. Stack traces from languages like TypeScript will point to generated .js files. - Set install_source_map_support = False in ${TARGET} to turn off this warning. - `); - } + Set install_source_map_support = False in ${TARGET} to turn off this warning.`); } } // Load all bootstrap modules before loading the entrypoint. diff --git a/internal/npm_install/browserify-wrapped.js b/internal/npm_install/browserify-wrapped.js index b2c2f2a335..0479759977 100644 --- a/internal/npm_install/browserify-wrapped.js +++ b/internal/npm_install/browserify-wrapped.js @@ -20,7 +20,9 @@ const child_process = require('child_process'); -const DEBUG = false; +function log_verbose(...m) { + if (!!process.env['VERBOSE_LOGS']) console.error('[browserify-wrapped.js]', ...m); +} const BABEL_PLUGINS = [ '@babel/plugin-transform-modules-commonjs', @@ -29,9 +31,7 @@ const BABEL_PLUGINS = [ runBrowserify(...process.argv.slice(2)); function runBrowserify(workspaceName, packageName, entryPoint, output, excluded = '') { - if (DEBUG) - console.error(` -browserify-wrapped: running with + log_verbose(`running with cwd: ${process.cwd()} workspaceName: ${workspaceName}, packageName: ${packageName} @@ -39,7 +39,7 @@ browserify-wrapped: running with output: ${output} excluded: ${excluded}`); - const browserify = require.resolve(`browserify/index${DEBUG ? '.debug' : ''}.js`); + const browserify = require.resolve(`browserify/index.js`); const namedAmd = require.resolve('named-amd'); const babelify = require.resolve('babelify'); const plugins = BABEL_PLUGINS.map(p => require.resolve(p)); @@ -59,7 +59,7 @@ browserify-wrapped: running with args = args.concat(['-u', e]) } - if (DEBUG) console.error(`\nRunning: node ${args.join(' ')}\n`); + log_verbose(`running node ${args.join(' ')}\n`); const isWindows = /^win/i.test(process.platform); child_process.execFileSync( diff --git a/internal/npm_install/generate_build_file.js b/internal/npm_install/generate_build_file.js index c3a81e7a58..6ec5c17443 100644 --- a/internal/npm_install/generate_build_file.js +++ b/internal/npm_install/generate_build_file.js @@ -42,7 +42,9 @@ const fs = require('fs'); const path = require('path'); -const DEBUG = false; +function log_verbose(...m) { + if (!!process.env['VERBOSE_LOGS']) console.error('[generate_build_file.js]', ...m); +} const BUILD_FILE_HEADER = `# Generated file from yarn_install/npm_install rule. # See $(bazel info output_base)/external/build_bazel_rules_nodejs/internal/npm_install/generate_build_file.js @@ -608,7 +610,8 @@ function cleanupEntryPointPath(p) { /** * Cleans up the given path - * Then tries to resolve the path into a file and warns if in DEBUG and the file dosen't exist + * Then tries to resolve the path into a file and warns if VERBOSE_LOGS set and the file dosen't + * exist * @param {any} pkg * @param {string} path * @returns {string | undefined} @@ -622,10 +625,8 @@ function findEntryFile(pkg, path) { // This can happen // in some npm packages that list an incorrect main such as v8-coverage@1.0.8 // which lists `"main": "index.js"` but that file does not exist. - if (DEBUG) { - console.error( - `Could not find entry point for the path ${cleanPath} given by npm package ${pkg._name}`) - } + log_verbose( + `could not find entry point for the path ${cleanPath} given by npm package ${pkg._name}`); } return entryFile; } @@ -687,10 +688,8 @@ function resolvePkgMainFile(pkg) { return maybeSelfNamedIndex; } - if (DEBUG) { - // none of the methods we tried resulted in a file - console.error(`Could not find entry point for npm package ${pkg._name}`) - } + // none of the methods we tried resulted in a file + log_verbose(`could not find entry point for npm package ${pkg._name}`); // at this point there's nothing left for us to try, so return nothing return undefined; @@ -724,7 +723,7 @@ function flattenPkgDependencies(pkg, dep, pkgsMap) { } // dependency not found if (required) { - console.error(`Could not find ${depType} '${targetDep}' of '${dep._dir}'`); + console.error(`could not find ${depType} '${targetDep}' of '${dep._dir}'`); process.exit(1); } return null; diff --git a/internal/npm_install/pre_process_package_json.js b/internal/npm_install/pre_process_package_json.js index 8e85318a53..e0c704d4a0 100644 --- a/internal/npm_install/pre_process_package_json.js +++ b/internal/npm_install/pre_process_package_json.js @@ -26,7 +26,9 @@ const fs = require('fs'); const child_process = require('child_process'); -const DEBUG = false; +function log_verbose(...m) { + if (!!process.env['VERBOSE_LOGS']) console.error('[pre_process_package_json.js]', ...m); +} const args = process.argv.slice(2); const packageJson = args[0]; @@ -44,7 +46,7 @@ function main() { const pkg = JSON.parse(fs.readFileSync(packageJson, {encoding: 'utf8'})); - if (DEBUG) console.error(`Pre-processing package.json`); + log_verbose(`pre-processing package.json`); if (isYarn) { // Work-around for https://github.com/yarnpkg/yarn/issues/2165 @@ -85,7 +87,7 @@ function clearYarnFilePathCaches(pkg) { } if (clearPackages.length) { - if (DEBUG) console.error(`Cleaning packages from yarn cache: ${clearPackages.join(' ')}`); + log_verbose(`cleaning packages from yarn cache: ${clearPackages.join(' ')}`); for (const c of clearPackages) { child_process.execFileSync( 'yarn', ['--mutex', 'network', 'cache', 'clean', c], diff --git a/internal/rollup/rollup.config.js b/internal/rollup/rollup.config.js index bfd8a17e15..1dcaa9d705 100644 --- a/internal/rollup/rollup.config.js +++ b/internal/rollup/rollup.config.js @@ -10,7 +10,10 @@ const isBuiltinModule = require('is-builtin-module'); const path = require('path'); const fs = require('fs'); -const DEBUG = false; +function log_verbose(...m) { + // This is a template file so we use __filename to output the actual filename + if (!!process.env['VERBOSE_LOGS']) console.error(`[${path.basename(__filename)}]`, ...m); +} const workspaceName = 'TMPL_workspace_name'; const rootDir = 'TMPL_rootDir'; @@ -20,9 +23,7 @@ const moduleMappings = TMPL_module_mappings; const nodeModulesRoot = 'TMPL_node_modules_root'; const isDefaultNodeModules = TMPL_is_default_node_modules; -if (DEBUG) - console.error(` -Rollup: running with +log_verbose(`running with cwd: ${process.cwd()} workspaceName: ${workspaceName} rootDir: ${rootDir} @@ -44,11 +45,11 @@ function fileExists(filePath) { // This resolver mimics the TypeScript Path Mapping feature, which lets us resolve // modules based on a mapping of short names to paths. function resolveBazel(importee, importer, baseDir = process.cwd(), resolve = require.resolve, root = rootDir) { - if (DEBUG) console.error(`\nRollup: resolving '${importee}' from ${importer}`); + log_verbose(`resolving '${importee}' from ${importer}`); function resolveInRootDir(importee) { var candidate = path.join(baseDir, root, importee); - if (DEBUG) console.error(`Rollup: try to resolve '${importee}' at '${candidate}'`); + log_verbose(`try to resolve '${importee}' at '${candidate}'`); try { var result = resolve(candidate); return result; @@ -64,7 +65,7 @@ function resolveBazel(importee, importer, baseDir = process.cwd(), resolve = req // If import is fully qualified then resolve it directly if (fileExists(importee)) { - if (DEBUG) console.error(`Rollup: resolved fully qualified '${importee}'`); + log_verbose(`resolved fully qualified '${importee}'`); return importee; } @@ -101,7 +102,7 @@ function resolveBazel(importee, importer, baseDir = process.cwd(), resolve = req // .d.ts suffix and let node require.resolve do its thing. var v = moduleMappings[k].replace(/\.d\.ts$/, ''); const mappedImportee = path.join(v, normalizedImportee.slice(k.length + 1)); - if (DEBUG) console.error(`Rollup: module mapped '${importee}' to '${mappedImportee}'`); + log_verbose(`module mapped '${importee}' to '${mappedImportee}'`); resolved = resolveInRootDir(mappedImportee); if (resolved) break; } @@ -114,12 +115,10 @@ function resolveBazel(importee, importer, baseDir = process.cwd(), resolve = req resolved = resolveInRootDir(userWorkspacePath.startsWith('..') ? importee : userWorkspacePath); } - if (DEBUG) { - if (resolved) { - console.error(`Rollup: resolved to ${resolved}`); - } else { - console.error(`Rollup: allowing rollup to resolve '${importee}' with node module resolution`); - } + if (resolved) { + log_verbose(`resolved to ${resolved}`); + } else { + log_verbose(`allowing rollup to resolve '${importee}' with node module resolution`); } return resolved; diff --git a/internal/rollup/terser-wrapped.js b/internal/rollup/terser-wrapped.js index fc4fffe0a1..faed63a45c 100644 --- a/internal/rollup/terser-wrapped.js +++ b/internal/rollup/terser-wrapped.js @@ -26,13 +26,15 @@ const path = require('path'); const tmp = require('tmp'); const child_process = require('child_process'); -const DEBUG = false; +function log_verbose(...m) { + if (!!process.env['VERBOSE_LOGS']) console.error('[terser-wrapped.js]', ...m); +} // capture the inputs and output options const argv = require('minimist')(process.argv.slice(2)); const inputs = argv._; const output = argv.output || argv.o; -const debug = argv.debug; +const debug = !!process.env['debug'] || argv.debug; const configFile = argv['config-file']; // delete the properties extracted above as the remaining // arguments are forwarded to terser in execFileSync below @@ -42,9 +44,7 @@ delete argv.o; delete argv.debug; delete argv['config-file']; -if (DEBUG) - console.error(` -terser: running with +log_verbose(`running with cwd: ${process.cwd()} argv: ${process.argv.slice(2).join(' ')} inputs: ${JSON.stringify(inputs)} @@ -59,7 +59,7 @@ if (inputs.length != 1) { const input = inputs[0]; function runterser(inputFile, outputFile, sourceMapFile) { - if (DEBUG) console.error(`Minifying ${inputFile} -> ${outputFile} (sourceMap ${sourceMapFile})`); + log_verbose(`minifying ${inputFile} -> ${outputFile} (sourceMap ${sourceMapFile})`); const terserConfig = { 'sourceMap': {'filename': sourceMapFile}, @@ -96,7 +96,7 @@ function runterser(inputFile, outputFile, sourceMapFile) { } } - if (DEBUG) console.error(`Running node ${args.join(' ')}`); + log_verbose(`running node ${args.join(' ')}`); const isWindows = /^win/i.test(process.platform); child_process.execFileSync( diff --git a/internal/rollup/tsc-directory.js b/internal/rollup/tsc-directory.js index 3e0b03e22a..a06b8c55fa 100644 --- a/internal/rollup/tsc-directory.js +++ b/internal/rollup/tsc-directory.js @@ -23,7 +23,9 @@ const fs = require('fs'); const path = require('path'); const child_process = require('child_process'); -const DEBUG = false; +function log_verbose(...m) { + if (!!process.env['VERBOSE_LOGS']) console.error('[terser-wrapped.js]', ...m); +} // capture the inputs and output options const argv = require('minimist')(process.argv.slice(2)); @@ -31,9 +33,7 @@ const input = argv.input; const output = argv.output; const project = argv.project; -if (DEBUG) - console.error(` -tsc-directory: running with +log_verbose(`running with cwd: ${process.cwd()} input: ${input} output: ${output} @@ -41,7 +41,7 @@ tsc-directory: running with `); function runTsc(inputDir, outputDir, projectFile) { - if (DEBUG) console.error(`Running tsc with ${project}`); + log_verbose(`running tsc with ${project}`); const inputBasename = path.relative(path.dirname(projectFile), inputDir); const outputBasename = path.relative(path.dirname(projectFile), outputDir); @@ -67,7 +67,7 @@ function runTsc(inputDir, outputDir, projectFile) { '--project', projectFile ]; - if (DEBUG) console.error(`Running node ${args.join(' ')}`); + log_verbose(`running node ${args.join(' ')}`); const isWindows = /^win/i.test(process.platform); child_process.execFileSync( diff --git a/packages/karma/src/karma.conf.js b/packages/karma/src/karma.conf.js index f0ebabc998..3a36042189 100644 --- a/packages/karma/src/karma.conf.js +++ b/packages/karma/src/karma.conf.js @@ -6,7 +6,12 @@ try { const tmp = require('tmp'); const child_process = require('child_process'); - const DEBUG = false; + const VERBOSE_LOGS = !!process.env['VERBOSE_LOGS']; + + function log_verbose(...m) { + // This is a template file so we use __filename to output the actual filename + if (VERBOSE_LOGS) console.error(`[${path.basename(__filename)}]`, ...m); + } // BEGIN ENV VARS TMPL_env_vars @@ -14,8 +19,7 @@ try { const configPath = 'TMPL_config_file'; - if (DEBUG) - console.info(`Karma test starting with: + log_verbose(`running with cwd: ${process.cwd()} configPath: ${configPath}`); @@ -49,9 +53,8 @@ try { } child_process.execFileSync( extractExe, [archiveFile, '.'], {stdio: [process.stdin, process.stdout, process.stderr]}); - if (DEBUG) - console.info(`Extracting web archive ${archiveFile} with ${extractExe} to ${ - extractedExecutablePath}`); + log_verbose( + `Extracting web archive ${archiveFile} with ${extractExe} to ${extractedExecutablePath}`); return extractedExecutablePath; } catch (e) { console.error(`Failed to extract ${archiveFile}`); @@ -170,11 +173,7 @@ try { // level of logging // possible values: config.LOG_DISABLE || config.LOG_ERROR || // config.LOG_WARN || config.LOG_INFO || config.LOG_DEBUG - if (DEBUG) { - conf.logLevel = config.LOG_DEBUG; - } else if (!conf.logLevel) { - conf.logLevel = config.LOG_INFO; - } + conf.logLevel = VERBOSE_LOGS ? config.LOG_DEBUG : config.LOG_INFO; // enable / disable watching file and executing tests whenever // any file changes @@ -307,7 +306,7 @@ try { overrideConfigValue(conf, 'customLaunchers', null); const webTestMetadata = require(process.env['WEB_TEST_METADATA']); - if (DEBUG) console.info(`WEB_TEST_METADATA: ${JSON.stringify(webTestMetadata, null, 2)}`); + log_verbose(`WEB_TEST_METADATA: ${JSON.stringify(webTestMetadata, null, 2)}`); if (webTestMetadata['environment'] === 'sauce') { // If a sauce labs browser is chosen for the test such as // "@io_bazel_rules_webtesting//browsers/sauce:chrome-win10" @@ -454,7 +453,7 @@ try { }; baseConf(config); config.set = originalSetConfig; - if (DEBUG) console.info(`Base karma configuration: ${JSON.stringify(conf, null, 2)}`); + log_verbose(`base karma configuration: ${JSON.stringify(conf, null, 2)}`); } configureBazelConfig(config, conf); @@ -463,7 +462,7 @@ try { configureTsWebTestConfig(conf); configureFormatError(conf); - if (DEBUG) console.info(`Karma configuration: ${JSON.stringify(conf, null, 2)}`); + log_verbose(`karma configuration: ${JSON.stringify(conf, null, 2)}`); config.set(conf); } diff --git a/packages/protractor/src/protractor.conf.js b/packages/protractor/src/protractor.conf.js index 6769c472d2..6f81d4052c 100644 --- a/packages/protractor/src/protractor.conf.js +++ b/packages/protractor/src/protractor.conf.js @@ -14,15 +14,19 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -const DEBUG = false; +const path = require('path'); + +function log_verbose(...m) { + // This is a template file so we use __filename to output the actual filename + if (!!process.env['VERBOSE_LOGS']) console.error(`[${path.basename(__filename)}]`, ...m); +} const configPath = 'TMPL_config'; const onPreparePath = 'TMPL_on_prepare'; const workspace = 'TMPL_workspace'; const server = 'TMPL_server'; -if (DEBUG) - console.info(`Protractor test starting with: +log_verbose(`running with cwd: ${process.cwd()} configPath: ${configPath} onPreparePath: ${onPreparePath} @@ -77,7 +81,7 @@ if (configPath) { throw new Error('Invalid base protractor configuration. Expected config to be exported.'); } conf = baseConf.config; - if (DEBUG) console.info(`Base protractor configuration: ${JSON.stringify(conf, null, 2)}`); + log_verbose(`base protractor configuration: ${JSON.stringify(conf, null, 2)}`); } // Import the user's on prepare function if specified @@ -109,7 +113,7 @@ setConf(conf, 'specs', specs, 'are determined by the srcs and deps attribute'); // We setup the protractor configuration based on the values in this object if (process.env['WEB_TEST_METADATA']) { const webTestMetadata = require(process.env['WEB_TEST_METADATA']); - if (DEBUG) console.info(`WEB_TEST_METADATA: ${JSON.stringify(webTestMetadata, null, 2)}`); + log_verbose(`WEB_TEST_METADATA: ${JSON.stringify(webTestMetadata, null, 2)}`); if (webTestMetadata['environment'] === 'sauce') { // If a sauce labs browser is chosen for the test such as // "@io_bazel_rules_webtesting//browsers/sauce:chrome-win10" @@ -183,6 +187,6 @@ if (process.env['WEB_TEST_METADATA']) { } // Export the complete protractor configuration -if (DEBUG) console.info(`Protractor configuration: ${JSON.stringify(conf, null, 2)}`); +log_verbose(`protractor configuration: ${JSON.stringify(conf, null, 2)}`); exports.config = conf; diff --git a/packages/terser/src/terser_minified.bzl b/packages/terser/src/terser_minified.bzl index b7792868ac..2c6bf1f64a 100644 --- a/packages/terser/src/terser_minified.bzl +++ b/packages/terser/src/terser_minified.bzl @@ -35,7 +35,7 @@ Bazel will make a copy of your config file, treating it as a template. If you use the magic strings `"bazel_debug"` or `"bazel_no_debug"`, these will be replaced with `true` and `false` respecting the value of the `debug` attribute -or the `--define=DEBUG=true` bazel flag. +or the `--define=DEBUG=1` bazel flag. For example, @@ -59,7 +59,7 @@ If `config_file` isn't supplied, Bazel will use a default config file. doc = """Configure terser to produce more readable output. Instead of setting this attribute, consider setting the DEBUG variable instead -bazel build --define=DEBUG=true //my/terser:target +bazel build --define=DEBUG=1 //my/terser:target so that it only affects the current build. """, ), diff --git a/packages/terser/test/debug/BUILD.bazel b/packages/terser/test/debug/BUILD.bazel index 773a3e2abe..d5c1cd5f34 100644 --- a/packages/terser/test/debug/BUILD.bazel +++ b/packages/terser/test/debug/BUILD.bazel @@ -21,12 +21,12 @@ terser_minified( name = "debug_from_env", src = "input.js", # Don't specify debug = True - # Instead we'll run the test with --define=DEBUG=true + # Instead we'll run the test with --define=DEBUG=1 ) golden_file_test( name = "test_define_DEBUG", actual = "debug_from_env.js", golden = "output.golden.js_", - tags = ["manual"], # Only passes when --define=DEBUG=true is set + tags = ["manual"], # Only passes when --define=DEBUG=1 is set ) diff --git a/packages/terser/test/inline_sourcemap/spec.js b/packages/terser/test/inline_sourcemap/spec.js index b6cf0a0fe8..4d7eb461b2 100644 --- a/packages/terser/test/inline_sourcemap/spec.js +++ b/packages/terser/test/inline_sourcemap/spec.js @@ -6,7 +6,9 @@ describe('terser sourcemap handling', () => { const file = require.resolve(__dirname + '/out.min.js.map'); const rawSourceMap = JSON.parse(fs.readFileSync(file, 'utf-8')); await sm.SourceMapConsumer.with(rawSourceMap, null, consumer => { - const pos = consumer.originalPositionFor({line: 1, column: 89}); + // terser will produce different output if the DEBUG environment variable is set + const pos = consumer.originalPositionFor( + !process.env['DEBUG'] ? {line: 1, column: 89} : {line: 6, column: 22}); expect(pos.name).toBe('something'); expect(pos.line).toBe(3); expect(pos.column).toBe(14); diff --git a/packages/terser/test/user_config/BUILD.bazel b/packages/terser/test/user_config/BUILD.bazel index 4a02b7a5cd..944b5be545 100644 --- a/packages/terser/test/user_config/BUILD.bazel +++ b/packages/terser/test/user_config/BUILD.bazel @@ -11,4 +11,5 @@ golden_file_test( name = "test", actual = "out.min.js", golden = "output.golden.js_", + golden_debug = "output.debug.golden.js_", ) diff --git a/packages/terser/test/user_config/output.debug.golden.js_ b/packages/terser/test/user_config/output.debug.golden.js_ new file mode 100644 index 0000000000..283f499b00 --- /dev/null +++ b/packages/terser/test/user_config/output.debug.golden.js_ @@ -0,0 +1,3 @@ +const a = () => { + return "hello"; +}; \ No newline at end of file