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

feat(examples): add angular bazel architect #1516

Merged

Conversation

Toxicable
Copy link

Checking in this example https://github.com/alexeagle/prototype_bazel_architect
Which shows how you can build an angular app with @angular-devkit/build-angular but under bazel

@Toxicable
Copy link
Author

It dosen't actually run since something deletes random dirs in the node_modules, such as node_modules/patch-package/dist/patch/apply.js, so you get errors like:

Error: Cannot find module './patch/apply'
Require stack:
- /Users/fabianwiles/Documents/rules_nodejs/examples/angular_bazel_architect/node_modules/patch-package/dist/applyPatches.js
- /Users/fabianwiles/Documents/rules_nodejs/examples/angular_bazel_architect/node_modules/patch-package/dist/index.js
- /Users/fabianwiles/Documents/rules_nodejs/examples/angular_bazel_architect/node_modules/patch-package/index.js

So something happens during Fetching @npm; Running yarn install on //:package.json that causes this dir node_modules/patch-package/dist/patch to be deleted

If I ran yarn install with my local version, it works fine, the dir remains there.

if you skip the postinstall step then the build still fails with:

Error: Cannot find module './util/canReportError'
Require stack:
- /private/var/tmp/_bazel_fabianwiles/221afc9f487a381a9ffe5cc91d597ad0/sandbox/darwin-sandbox/2/execroot/angular_cli_demo/node_modules/@angular-devkit/architect/node_modules/rxjs/internal/Observable.js

@Toxicable
Copy link
Author

This looks related to the above npm_install issues im facing #1471

@Toxicable Toxicable force-pushed the examples/inital-angular-architect branch 3 times, most recently from 66b52de to 9850a40 Compare January 4, 2020 08:17
@Toxicable
Copy link
Author

Both build and test work fine locally but fail in CI due to the error below, but I thought that the patch patches/@ngtools+webpack+9.0.0-rc.0.patch should handle that, is it not applied during example_integration_test?

FAILED: //examples:examples_angular_bazel_architect (Summary)
~~SNIP~~
ERROR: /tmp/tmp-6275IrF4677WKijh/BUILD.bazel:3:1: Action build failed (Exit 1) architect.sh failed: error executing command bazel-out/host/bin/external/npm/@angular-devkit/architect-cli/bin/architect.sh frontend:build '--outputPath=bazel-out/k8-fastbuild/bin/build' ... (remaining 2 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
frontend:build: 
ERROR in ./src/main.ts
Module build failed (from ./external/npm/node_modules/@ngtools/webpack/src/index.js):
Error: Angular Compiler was detected but it was an instance of the wrong class.
This likely means you have several @ngtools/webpack packages installed. You can check this with `npm ls @ngtools/webpack`, and then remove the extra copies.
    at Object.ngcLoader (/home/circleci/.cache/bazel/_bazel_circleci/02fc451c9ce3986cffaddce25971cb5b/execroot/build_bazel_rules_nodejs/_tmp/07c12d54970c8cddb1ddf80282a55151/_bazel_circleci/d694256e64afdb0bd419bc134525f790/sandbox/processwrapper-sandbox/1/execroot/angular_cli_demo/external/npm/node_modules/@ngtools/webpack/src/loader.js:33:15)

@Toxicable Toxicable marked this pull request as ready for review January 4, 2020 08:48
@Toxicable Toxicable force-pushed the examples/inital-angular-architect branch from 9850a40 to 85e26e9 Compare January 6, 2020 06:12
@Toxicable Toxicable force-pushed the examples/inital-angular-architect branch 3 times, most recently from 372245a to f876b6a Compare January 11, 2020 19:58
@Toxicable
Copy link
Author

This all runs fine locally.
Failure in CI is due to the angular cli trying to write to a cache - angular/angular-cli#16649

@Toxicable Toxicable force-pushed the examples/inital-angular-architect branch 2 times, most recently from 70abcc0 to dcb9f04 Compare January 13, 2020 18:14
@gregmagolan
Copy link
Collaborator

gregmagolan commented Jan 13, 2020

Those patches should be applied in the integration test in CI as yarn_install for that example should run the same in CI as it does locally.

@Toxicable Toxicable force-pushed the examples/inital-angular-architect branch 2 times, most recently from 672217b to b6ca59c Compare January 14, 2020 20:18
@Toxicable Toxicable force-pushed the examples/inital-angular-architect branch 2 times, most recently from 0f5d09d to 0b6506b Compare January 15, 2020 07:33
@Toxicable
Copy link
Author

Current status

  • npx bazel test ... works locally on Ubuntu 18.04 fine.
  • npx bazel build ... works in CI for Ubuntu and Mac
  • :test target dosen't run in bazelci since there's no way to install the chrome system deps.
  • NG_BUILD_CACHE must be set to false and passed through to architect since build-angular would try to cache to bazel's readonly fs.
  • npx bazel build ... fails in CI due to some spawn and a missing or wrong path to node
Error: spawn D:\b\j3ai4z2n\execroot\build_bazel_rules_nodejs\_tmp\00ec2cae7910fb9a26b84ca52e1907d7\_bazel_b\nxx36vy6\external\build_bazel_rules_nodejs\internal\node\_node_bin\node ENOENT

@alan-agius4
Copy link
Contributor

@Toxicable the last error might be related to the fact that under Windows the spawn process is being invoked without the shell option.

See: angular/angular-cli#16119 I gave a more detailed explanation.

@gregmagolan
Copy link
Collaborator

gregmagolan commented Jan 28, 2020

I haven't seen the filesystem flake locally on OSX at all. This seems to happen only on buildkite. Maybe it is to do with something they are doing with macos virtualization?

But this PR is still blocked on the puppeteer usage for OSX as it won't work there yet until Bazel supports spaces in runfiles paths. Alternately, we don't include node_modules in runfiles but that is not ready to go yet.

@gregmagolan
Copy link
Collaborator

I have a path to land this:

  1. We need to remove puppeteer from the example as it will never work on OSX right now as Bazel can't handle spaces in runfiles. Karma should just use the local chrome at that point.

  2. Turn off symlinked_node_modules & managed_directories fixes the flake. I'll follow up with buildkite as to what they are doing on the macos machines. Still thinking it has to do with their virtualization layer.

  3. Run bazel test ... for the integration test. On CircleCI the local browser should work. Ubuntu on buildkite doesn't have the shared libs so you'll need to add a dummy_test target (see examples/angular/dummy_test.sh) or Bazel will complain that there are no test targets. On macos it should pass as the local chrome should work there. Windows is still broken and will be filtered out by fix-windows

@Toxicable Toxicable force-pushed the examples/inital-angular-architect branch 9 times, most recently from fdccfa3 to 4509aa9 Compare January 29, 2020 18:47
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

.circleci/config.yml Outdated Show resolved Hide resolved
examples/angular_bazel_architect/.bazelrc Outdated Show resolved Hide resolved
examples/angular_bazel_architect/.editorconfig Outdated Show resolved Hide resolved
examples/angular_bazel_architect/README.md Outdated Show resolved Hide resolved
@gregmagolan
Copy link
Collaborator

@googlebot I consent

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@Toxicable Toxicable force-pushed the examples/inital-angular-architect branch from 3de6b52 to 47d4dc5 Compare January 30, 2020 07:47
Copy link
Collaborator

@gregmagolan gregmagolan left a comment

Choose a reason for hiding this comment

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

Nice! This is great example.

examples/angular_bazel_architect/WORKSPACE Show resolved Hide resolved
@Toxicable Toxicable force-pushed the examples/inital-angular-architect branch from 47d4dc5 to be2c47a Compare January 30, 2020 17:48
Copy link
Collaborator

@gregmagolan gregmagolan left a comment

Choose a reason for hiding this comment

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

🚀

Use ChromeHeadess as base instead of passing —headless. This fixes the CI failure on CircleCI and fixes it locally for me on OSX. buildkite macos still fails with

```
        # ==================== Test output for //:test:
        # 29 01 2020 23:31:42.804:INFO [karma-server]: Karma v4.1.0 server started at http://0.0.0.0:9876/
        # 29 01 2020 23:31:42.806:INFO [launcher]: Launching browsers ChromeHeadlessNoSandbox with concurrency unlimited
        # 29 01 2020 23:31:42.808:INFO [launcher]: Starting browser ChromeHeadless
        # 29 01 2020 23:32:42.815:WARN [launcher]: ChromeHeadless have not captured in 60000 ms, killing.
        # 29 01 2020 23:32:44.818:WARN [launcher]: ChromeHeadless was not killed in 2000 ms, sending SIGKILL.
        # 29 01 2020 23:32:46.822:WARN [launcher]: ChromeHeadless was not killed by SIGKILL in 2000 ms, continuing.
```

but this seems to be something to do with the buildkite machine chrome version/configuration as it does work locally on mac for me with Chrome 79. Disabled the test on using `no-bazelci-mac` tag.
@Toxicable Toxicable force-pushed the examples/inital-angular-architect branch from be2c47a to cf3af3d Compare January 30, 2020 18:11
@Toxicable
Copy link
Author

I didn't properly update the sha256 on build_bazel_rules_nodejs - fixed now

@gregmagolan gregmagolan merged commit a4ca6f6 into bazel-contrib:master Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants