Bootstrap require patching fix

[gregmagolan]

Require patches need to be separated from loader as if you do the following

node --node_options=--require=/path/to/bootstrap.js /path/to/loader.js

bootstrap.js:

// patch require
if (process.env['BAZEL_NODE_RUNFILES_HELPER']) {
  require(process.env['BAZEL_NODE_RUNFILES_HELPER']).patchRequire();
}

patchRequire ends up loading the loader.js script (which does the patches but then does not execute as it has a if (require.main === module) { check). Then node does not run it at all as the "main" because it has already been loaded.

Test commit catches the bug (making use of expected_exit_code):

FAIL: //internal/node/test:fail_bootstrap_with_patch_test (see /private/var/tmp/_bazel_greg/35306ad70b8da8f7bc31590523de35c0/execroot/build_bazel_rules_nodejs/bazel-out/darwin-fastbuild/testlogs/internal/node/test/fail_bootstrap_with_patch_test/test.log)
INFO: From Testing //internal/node/test:fail_bootstrap_with_patch_test:
==================== Test output for //internal/node/test:fail_bootstrap_with_patch_test:
here
Expected exit code to be 55, but got 0
================================================================================
FAIL: //packages/jasmine/test:fail_bootstrap_test (see /private/var/tmp/_bazel_greg/35306ad70b8da8f7bc31590523de35c0/execroot/build_bazel_rules_nodejs/bazel-out/darwin-fastbuild/testlogs/packages/jasmine/test/fail_bootstrap_test/test.log)
INFO: From Testing //packages/jasmine/test:fail_bootstrap_test:
==================== Test output for //packages/jasmine/test:fail_bootstrap_test:
Expected exit code to be 3, but got 0
================================================================================
FAIL: //packages/jasmine/test:failing_sharding_bootstrap_test (shard 1 of 2) (see /private/var/tmp/_bazel_greg/35306ad70b8da8f7bc31590523de35c0/execroot/build_bazel_rules_nodejs/bazel-out/darwin-fastbuild/testlogs/packages/jasmine/test/failing_sharding_bootstrap_test/shard_1_of_2/test.log)
INFO: From Testing //packages/jasmine/test:failing_sharding_bootstrap_test (shard 1 of 2):
==================== Test output for //packages/jasmine/test:failing_sharding_bootstrap_test (shard 1 of 2):
Expected exit code to be 3, but got 0
================================================================================
FAIL: //packages/jasmine/test:failing_sharding_bootstrap_test (shard 2 of 2) (see /private/var/tmp/_bazel_greg/35306ad70b8da8f7bc31590523de35c0/execroot/build_bazel_rules_nodejs/bazel-out/darwin-fastbuild/testlogs/packages/jasmine/test/failing_sharding_bootstrap_test/shard_2_of_2/test.log)

FAILED: //packages/jasmine/test:failing_sharding_bootstrap_test (Summary)
      /private/var/tmp/_bazel_greg/35306ad70b8da8f7bc31590523de35c0/execroot/build_bazel_rules_nodejs/bazel-out/darwin-fastbuild/testlogs/packages/jasmine/test/failing_sharding_bootstrap_test/shard_1_of_2/test.log
      /private/var/tmp/_bazel_greg/35306ad70b8da8f7bc31590523de35c0/execroot/build_bazel_rules_nodejs/bazel-out/darwin-fastbuild/testlogs/packages/jasmine/test/failing_sharding_bootstrap_test/shard_2_of_2/test.log
INFO: From Testing //packages/jasmine/test:failing_sharding_bootstrap_test (shard 2 of 2):
==================== Test output for //packages/jasmine/test:failing_sharding_bootstrap_test (shard 2 of 2):
Expected exit code to be 3, but got 0
================================================================================

Fix commit fixes it.

We now --node_options=--require the require patches script first before any use bootstrap scripts so they will have access to the patches by default for nodejs_binary & nodejs_test. Tests added to verify this & test also added to verify that if a bootstrap scripts fails its exit code is propogated.

PR Checklist

Please check if your PR fulfills the following requirements:

• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature (please, look at the "Scope of the project" section in the README.md file)
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

Other information

[alexeagle]

What benefit are we getting from the loader at this point? It looks like it just adds an indirection before calling the users entry point script, which we could remove with sth like #1440

Is the loader only here now as a way to load the require patches?

[gregmagolan]

Yea. The only thing the loader does now is require the patches in the case when the linker is not run.

I'm hesitant to always enable the linker at least for this PR because I just need the bug fix to get angular up to rules_nodejs 1.0 (this bug is now blocking). But I'll try to get rid of the loader entirely and use --node_options=--require=/path/to/patcher to patch require instead. A good simplification without having to change the current behavior.

[gregmagolan]

Couldn't get rid of the loader as the correct path to the entry_point script is non-trivial. But I did remove the require patches from the loader entirely and they are now --node_options=--require'd first before any user node_options so all bootstrap scripts & the main script have access to the require patches.

Good bit of pre-factoring before we enable the linker for all in #1440