Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: fix //packages/angular_devkit/build_angular:build_angular_<foo>_test on OSX #17624

Closed
wants to merge 1 commit into from

Conversation

gregmagolan
Copy link
Contributor

These need to be run with "local" tag because puppeteer Chrome has lib files with spaces in them which Bazel does not support in runfiles. However, when tagged "local", this fixes that problem as the files with spaces are visible outside of the sandbox but it introduces the issue of the linker conflicting with the node_modules symlinks created by yarn workspaces. So in order for a test to work with "local" we need to also disable yarn workspaces.

…>_test on OSX

These need to be run with "local" tag because puppeteer Chrome has lib files with spaces in them which Bazel does not support in runfiles. However, when tagged "local", this fixes that problem as the files with spaces are visible outside of the sandbox but it introduces the issue of the linker conflicting with the node_modules symlinks created by yarn workspaces. So in order for a test to work with "local" we need to also disable yarn workspaces.
@@ -16,6 +17,17 @@ licenses(["notice"]) # MIT License

# @angular-devkit/core

copy_to_bin(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: this copy_to_bin is necessary because the linker is linking to bazel-out so any source files (such as package.json) that are required in the package need to be pushed to bazel-out with copy_to_bin. Not ideal but outside of the sandbox there is no better approach AFAIK. Side-effect is that this works on Windows which does not have a sandbox or runfiles.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, you'll get ```==================== Test output for //packages/angular_devkit/build_angular:build_angular_karma_test (shard 2 of 2):
internal/modules/cjs/loader.js:796
throw err;
^

Error: Cannot find module '@angular-devkit/core'
Require stack:

  • /private/var/tmp/_bazel_greg/dbb005a7eeb5fa8b89a9797588e36854/execroot/angular_cli/bazel-out/darwin-fastbuild/bin/packages/angular_devkit/build_angular/build_angular_karma_test.sh.runfiles/angular_cli/packages/angular_devkit/build_angular/src/karma/code-coverage_spec.js
  • /private/var/tmp/_bazel_greg/dbb005a7eeb5fa8b89a9797588e36854/execroot/angular_cli/bazel-out/darwin-fastbuild/bin/packages/angular_devkit/build_angular/build_angular_karma_test.sh.runfiles/npm/node_modules/jasmine/lib/jasmine.js
because there is no package.json in the bazel-out folder of `@angular-devkit/core` which is linked

@gregmagolan
Copy link
Contributor Author

For reference to the spaces/runfiles issue on Bazel, here is the open issue on bazelbuild/bazel: bazelbuild/bazel#4327 and the PR to fix it which passed OSS review but had some internal side-effects so it was dropped: bazelbuild/bazel#9351

@dgp1130
Copy link
Collaborator

dgp1130 commented Jun 3, 2021

We're going through some old PRs and came across this one. It's pretty out of date and it seems like a workaround for running certain Bazel tests on iOS. We have some inline comments about how to run these tests locally, so we do have known workarounds. Ideally they would work without requiring that knowledge, but it seems like it buts up against unsupported Bazel behavior, so it's a hard battle to win here.

Since we have these workarounds and there doesn't seem to be much need for this change right now, I think it's safe to close this PR. If we find a good strategy for handling spaces in file paths (or Bazel adds support), then we can definitely reevaluate this and see where we can improve our test infra.

@dgp1130 dgp1130 closed this Jun 3, 2021
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants