build: allow bazel build ... #22168

alexeagle · 2018-02-12T16:36:15Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
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
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure 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 · 2018-02-12T18:49:28Z

Note, the reason this PR removes firebase-tools is:

firebase-tools has an optional dependency on https://www.npmjs.com/package/@google-cloud/functions-emulator
yarn's --ignore-optional doesn't work for transitive deps, so there's no way to yarn install without getting that functions-emulator package
functions-emulator has a transitive dep on grpc
the version of grpc we get has BUILD files and no WORKSPACE file so it always breaks bazel build ...

It could be solved by any of:

remove firebase-tools - this is what I did
fix yarn so you can omit optional deps of a transitive dep
make functions-emulator depend transitively on a more recent grpc version
patch grpc after install by doing an rm command in our postinstall or something

gkalpak · 2018-02-12T22:22:54Z

Maybe one could use resolutions to pin grpc to a new version (as long as it is compatible with functions-emulator - or maybe we don't care 😁)

alexeagle · 2018-02-13T00:03:04Z

Thanks @gkalpak I'll try that if Igor doesn't like this approach

IgorMinar

the rest looks great. thanks

IgorMinar · 2018-02-13T06:49:45Z

package.json

@@ -63,7 +63,6 @@
    "cors": "2.8.4",
    "domino": "1.0.29",
    "entities": "1.1.1",
-    "firebase-tools": "3.12.0",


this change has nothing to do with the bazel build ... commit. Please move to a separate commit, or remove in favor of #22186

like I explained above, this is required for bazel build ... - it's the main thing that was in the way

copied the explanation to the commit message

ngbot · 2018-02-13T19:42:18Z

Hi @alexeagle! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

IgorMinar

please amend the commit message with explanation about the protobuffjs

Note, the reason this commit removes `firebase-tools` is: 1) firebase-tools has an optional dependency on https://www.npmjs.com/package/@google-cloud/functions-emulator 2) yarn's `--ignore-optional` doesn't work for transitive deps, so there's no way to yarn install without getting that functions-emulator package 3) functions-emulator has a transitive dep on `grpc` 4) the version of `grpc` we get has `BUILD` files and no `WORKSPACE` file so it always breaks `bazel build ...` It could be solved by any of: 1) remove firebase-tools - this is what I did 2) fix yarn so you can omit optional deps of a transitive dep 3) make functions-emulator depend transitively on a more recent `grpc` version 4) patch `grpc` after install by doing an `rm` command in our postinstall or something In its place we must install protobufjs. This is needed by the ngc-wrapped test, which needs jasmine as well as bazel's worker mode dependencies, and therefore cannot simply rely on node_modules = "@build_bazel_rules_typescript_tsc_wrapped_deps//:node_modules" PR Close #22168

This reverts commit 265ac8a.

ngbot · 2018-03-20T22:47:40Z

Hi @alexeagle! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

matsko · 2018-03-22T23:28:57Z

@alexeagle please rebase onto master.

Also switch our CircleCI commands to just bazel build //... bazel test //... as this is easier to understand. Note, the reason this commit removes `firebase-tools` is: 1) firebase-tools has an optional dependency on https://www.npmjs.com/package/@google-cloud/functions-emulator 2) yarn's `--ignore-optional` doesn't work for transitive deps, so there's no way to yarn install without getting that functions-emulator package 3) functions-emulator has a transitive dep on `grpc` 4) the version of `grpc` we get has `BUILD` files and no `WORKSPACE` file so it always breaks `bazel build ...` It could be solved by any of: 1) remove firebase-tools - this is what I did 2) fix yarn so you can omit optional deps of a transitive dep 3) make functions-emulator depend transitively on a more recent `grpc` version 4) patch `grpc` after install by doing an `rm` command in our postinstall or something In its place we must install protobufjs. This is needed by the ngc-wrapped test, which needs jasmine as well as bazel's worker mode dependencies, and therefore cannot simply rely on node_modules = "@build_bazel_rules_typescript_tsc_wrapped_deps//:node_modules"

alexeagle · 2018-03-23T00:55:45Z

Rebased

Note, the reason this commit removes `firebase-tools` is: 1) firebase-tools has an optional dependency on https://www.npmjs.com/package/@google-cloud/functions-emulator 2) yarn's `--ignore-optional` doesn't work for transitive deps, so there's no way to yarn install without getting that functions-emulator package 3) functions-emulator has a transitive dep on `grpc` 4) the version of `grpc` we get has `BUILD` files and no `WORKSPACE` file so it always breaks `bazel build ...` It could be solved by any of: 1) remove firebase-tools - this is what I did 2) fix yarn so you can omit optional deps of a transitive dep 3) make functions-emulator depend transitively on a more recent `grpc` version 4) patch `grpc` after install by doing an `rm` command in our postinstall or something In its place we must install protobufjs. This is needed by the ngc-wrapped test, which needs jasmine as well as bazel's worker mode dependencies, and therefore cannot simply rely on node_modules = "@build_bazel_rules_typescript_tsc_wrapped_deps//:node_modules" PR Close angular#22168

This reverts commit 265ac8a.

Also switch our CircleCI commands to just bazel build //... bazel test //... as this is easier to understand. Note, the reason this commit removes `firebase-tools` is: 1) firebase-tools has an optional dependency on https://www.npmjs.com/package/@google-cloud/functions-emulator 2) yarn's `--ignore-optional` doesn't work for transitive deps, so there's no way to yarn install without getting that functions-emulator package 3) functions-emulator has a transitive dep on `grpc` 4) the version of `grpc` we get has `BUILD` files and no `WORKSPACE` file so it always breaks `bazel build ...` It could be solved by any of: 1) remove firebase-tools - this is what I did 2) fix yarn so you can omit optional deps of a transitive dep 3) make functions-emulator depend transitively on a more recent `grpc` version 4) patch `grpc` after install by doing an `rm` command in our postinstall or something In its place we must install protobufjs. This is needed by the ngc-wrapped test, which needs jasmine as well as bazel's worker mode dependencies, and therefore cannot simply rely on node_modules = "@build_bazel_rules_typescript_tsc_wrapped_deps//:node_modules" PR Close angular#22168

angular-automatic-lock-bot · 2019-09-13T11:21:32Z

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.}

googlebot added the cla: yes label Feb 12, 2018

alexeagle force-pushed the build_dotdotdot branch from 51e0730 to fba5ed7 Compare February 12, 2018 17:33

alexeagle requested a review from IgorMinar February 12, 2018 17:33

alexeagle added target: patch This PR is targeted for the next patch release area: bazel Issues related to the published `@angular/bazel` build rules labels Feb 12, 2018

IgorMinar approved these changes Feb 13, 2018

View reviewed changes

IgorMinar added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Feb 13, 2018

IgorMinar assigned alexeagle Feb 13, 2018

IgorMinar approved these changes Feb 14, 2018

View reviewed changes

IgorMinar mentioned this pull request Feb 14, 2018

build: remove firebase-tools from root package.json #22186

Closed

alexeagle force-pushed the build_dotdotdot branch 3 times, most recently from 379b673 to 0de84cd Compare February 14, 2018 03:07

alexeagle added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Feb 14, 2018

alexeagle force-pushed the build_dotdotdot branch 2 times, most recently from 977ff36 to 0de84cd Compare February 14, 2018 15:36

matsko closed this in 265ac8a Feb 14, 2018

vicb reopened this Feb 15, 2018

vicb closed this Feb 15, 2018

matsko added a commit that referenced this pull request Feb 15, 2018

Revert "build: allow bazel build ... (#22168)"

dac2e6e

This reverts commit 265ac8a.

matsko added a commit that referenced this pull request Feb 15, 2018

Revert "build: allow bazel build ... (#22168)"

0aa9b46

This reverts commit 265ac8a.

vicb reopened this Feb 15, 2018

vicb added the state: blocked label Feb 15, 2018

smdunn pushed a commit to smdunn/angular that referenced this pull request Feb 28, 2018

Revert "build: allow bazel build ... (angular#22168)"

81fca16

This reverts commit 265ac8a.

alexeagle closed this Mar 20, 2018

alexeagle force-pushed the build_dotdotdot branch from 0de84cd to 4648597 Compare March 20, 2018 19:32

alexeagle reopened this Mar 20, 2018

alexeagle added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: blocked labels Mar 20, 2018

alexeagle force-pushed the build_dotdotdot branch 2 times, most recently from 2441c3e to 970128b Compare March 20, 2018 20:58

alexeagle force-pushed the build_dotdotdot branch 2 times, most recently from 34cc777 to a0f9f05 Compare March 21, 2018 00:04

alexeagle added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 22, 2018

alexeagle requested review from IgorMinar and removed request for IgorMinar March 22, 2018 17:31

mhevery approved these changes Mar 22, 2018

View reviewed changes

alexeagle force-pushed the build_dotdotdot branch from a0f9f05 to 1ee0d5e Compare March 23, 2018 00:55

IgorMinar approved these changes Mar 23, 2018

View reviewed changes

mhevery added target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Mar 23, 2018

mhevery closed this in 4f0cae0 Mar 23, 2018

leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018

Revert "build: allow bazel build ... (angular#22168)"

c556f4e

This reverts commit 265ac8a.

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: allow bazel build ... #22168

build: allow bazel build ... #22168

alexeagle commented Feb 12, 2018

alexeagle commented Feb 12, 2018

gkalpak commented Feb 12, 2018

alexeagle commented Feb 13, 2018

IgorMinar left a comment

IgorMinar Feb 13, 2018

alexeagle Feb 13, 2018

alexeagle Feb 14, 2018

ngbot bot commented Feb 13, 2018

IgorMinar left a comment

ngbot bot commented Mar 20, 2018

matsko commented Mar 22, 2018

alexeagle commented Mar 23, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

build: allow bazel build ... #22168

build: allow bazel build ... #22168

Conversation

alexeagle commented Feb 12, 2018

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

alexeagle commented Feb 12, 2018

gkalpak commented Feb 12, 2018

alexeagle commented Feb 13, 2018

IgorMinar left a comment

Choose a reason for hiding this comment

IgorMinar Feb 13, 2018

Choose a reason for hiding this comment

alexeagle Feb 13, 2018

Choose a reason for hiding this comment

alexeagle Feb 14, 2018

Choose a reason for hiding this comment

ngbot bot commented Feb 13, 2018

IgorMinar left a comment

Choose a reason for hiding this comment

ngbot bot commented Mar 20, 2018

matsko commented Mar 22, 2018

alexeagle commented Mar 23, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019