Skip to content

fix(builtin): use updated rules_js launcher logic to source RUNFILES#3557

Merged
gregmagolan merged 1 commit into
stablefrom
rules_js_launcher
Sep 27, 2022
Merged

fix(builtin): use updated rules_js launcher logic to source RUNFILES#3557
gregmagolan merged 1 commit into
stablefrom
rules_js_launcher

Conversation

@kormide

@kormide kormide commented Sep 27, 2022

Copy link
Copy Markdown
Collaborator

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?

RUNFILES_MANIFEST_ONLY is always set to 1 (see https://github.com/angular/angular/pull/47527/files#diff-30590eade0d767af96219fac23a137e7996bff554e9569f1db54ed40be435854R17-R23). This seems like a bug. When runfiles are enabled, during a bazel test the environment var RUNFILES is not set correctly because of it.

Separately from addressing the above, the slightly modified launcher logic in rules_js already works around the issue by getting the runfiles directory from TEST_SRCDIR.

What is the new behavior?

Use rules_js updated launcher logic.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@gregmagolan
gregmagolan force-pushed the rules_js_launcher branch 3 times, most recently from 4bea901 to ee1540b Compare September 27, 2022 16:51
Comment thread internal/node/launcher.sh Outdated
@gregmagolan
gregmagolan merged commit c725169 into stable Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants