Commit Graph

19 Commits

Author SHA1 Message Date
Joey Perrott c214cad2b4 Revert "build: upgrade angular build, integration/bazel and @angular/bazel package to rule_nodejs 2.2.0 (#37727)" (#39097)
This reverts commit db56cf18ba.

PR Close #39097
2020-10-02 10:56:53 -07:00
Greg Magolan db56cf18ba build: upgrade angular build, integration/bazel and @angular/bazel package to rule_nodejs 2.2.0 (#37727)
Updates to rules_nodejs 2.2.0. This is the first major release in 7 months and includes a number of features as well
as breaking changes.

Release notes: https://github.com/bazelbuild/rules_nodejs/releases/tag/2.0.0

Features of note for angular/angular:

* stdout/stderr/exit code capture; this could be potentially be useful

* TypeScript (ts_project); a simpler tsc rule that ts_library that can be used in the repo where ts_library is too
  heavy weight

Breaking changes of note for angular/angular:

* loading custom rules from npm packages: `ts_library` is no longer loaded from `@npm_bazel_typescript//:index.bzl`
  (which no longer exists) but is now loaded from `@npm//@bazel/typescript:index.bzl`

* with the loading changes above, `load("@npm//:install_bazel_dependencies.bzl", "install_bazel_dependencies")` is
  no longer needed in the WORKSPACE which also means that yarn_install does not need to run unless building/testing
  a target that depends on @npm. In angular/angular this is a minor improvement as almost everything depends on @npm.

* @angular/bazel package is also updated in this PR to support the new load location; Angular + Bazel users that
  require it for ng_package (ng_module is no longer needed in OSS with Angular 10) will need to load from
  `@npm//@angular/bazel:index.bzl`. I investigated if it was possible to maintain backward compatability for the old
  load location `@npm_angular_bazel` but it is not since the package itself needs to be updated to load from
  `@npm//@bazel/typescript:index.bzl` instead of `@npm_bazel_typescript//:index.bzl` as it depends on ts_library
  internals for ng_module.

* runfiles.resolve will now throw instead of returning undefined to match behavior of node require

Other changes in angular/angular:

* integration/bazel has been updated to use both ng_module and ts_libary with use_angular_plugin=true.
  The latter is the recommended way for rules_nodejs users to compile Angular 10 with Ivy. Bazel + Angular ViewEngine is
  supported with @angular/bazel <= 9.0.5 and Angular <= 8. There is still Angular ViewEngine example on rules_nodejs
  https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_view_engine on these older versions but users
  that want to update to Angular 10 and are on Bazel must switch to Ivy and at that point ts_library with
  use_angular_plugin=true is more performant that ng_module. Angular example in rules_nodejs is configured this way
  as well: https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular. As an aside, we also have an
  example of building Angular 10 with architect() rule directly instead of using ts_library with angular plugin:
  https://github.com/bazelbuild/rules_nodejs/tree/stable/examples/angular_bazel_architect.

NB: ng_module is still required for angular/angular repository as it still builds ViewEngine & @angular/bazel
also provides the ng_package rule. ng_module can be removed in the future if ViewEngine is no longer needed in
angular repo.

* JSModuleInfo provider added to ng_module. this is for forward compat for future rules_nodejs versions.
  @josephperrott, this touches `packages/bazel/src/external.bzl` which will make the sync to g3 non-trivial.

PR Close #37727
2020-10-01 15:34:36 -07:00
Andrew Scott 67e3ecc7e3 fix(dev-infra): Ensure conditions with groups do not fail verification (#37798)
There are a few changes in this PR to ensure conditions that are based
on groups (i.e. `- groups.pending.length == 0`) do not fail the verify
task:

* Remove the warning when a condition is encountered that depends on the
`groups` state. The warning will otherwise be printed once for every
file that triggers the execution of the condition (400,000+ times)
* Add an `unverifiable` flag to `GroupCondition` interface and set it to
true when an error is encountered due to attempting to get the state of
`groups` in a condition
* Ignore any unverifiable conditions when gathering unmatched
conditions. These should not be considered `unmatched` for verification
purposes.
* Print the unverifiable conditions by group in the results

Sample output:
```

┌──────────────────────────────────────────────────────────────────────────────┐
│                         PullApprove results by group                         │
└──────────────────────────────────────────────────────────────────────────────┘
Groups skipped (4 groups)
Matched conditions by Group (37 groups)
Unmatched conditions by Group (0 groups)
Unverifiable conditions by Group (3 groups)
  [public-api]
    len(groups.pending.exclude("required-minimum-review")...
    len(groups.rejected.exclude("required-minimum-review")...
  [size-tracking]
    len(groups.pending.exclude("required-minimum-review")...
    len(groups.rejected.exclude("required-minimum-review")...
  [circular-dependencies]
    len(groups.pending.exclude("required-minimum-review")...
    len(groups.rejected.exclude("required-minimum-review")...

```

PR Close #37798
2020-07-24 17:59:39 -07:00
Andrew Scott d0060dcfdc refactor(dev-infra): Add support for groups in the conditions evaluator (#38164)
Conditions can refer to the groups array that is a list of the preceding groups.
This commit adds support to the verification for those conditions.

This commit also adds some tests to the parsing and condition matching
to ensure everything works as expected.

PR Close #38164
2020-07-23 11:05:42 -07:00
Joey Perrott cc37af2314 fix(dev-infra): allow for deep merging of pullapprove config aliases (#36915)
Set the yaml parser to support deep merges of yaml aliases,
to support having a default value for all rules to build upon.

PR Close #36915
2020-06-15 14:29:32 -07:00
Joey Perrott ed3c549063 ci: remove IgorMinar from reviewers list for pullapprove fallback group (#36456)
Historically we have had a pullapprove group `fallback` which acted as
a catch all for files which did not match any other groups.  This
group assigned reviews to IgorMinar, however it was not apparent that
this group was assigned.  This change removes this assignment.  This
group as active should always coincide with failures of the pullapprove
verification script. We continue to have this group as a secondary test
ensuring all files in the repo are captured by the pullapprove config.

PR Close #36456
2020-06-08 10:07:45 -07:00
Joey Perrott dffcca73e4 fix(dev-infra): clean up usages within pullapprove tooling (#37338)
Clean up pullapprove tooling to use newly created common utils.
Additionally, use newly created logging levels rather than
verbose flagging.

PR Close #37338
2020-06-04 12:43:44 -07:00
Joey Perrott d1ea1f4c7f build: update license headers to reference Google LLC (#37205)
Update the license headers throughout the repository to reference Google LLC
rather than Google Inc, for the required license headers.

PR Close #37205
2020-05-26 14:26:58 -04:00
Joey Perrott 367f6fe12d feat(dev-infra): migrate pullapprove tool to use new logging system (#37232)
Migrate the pullapprove tool in ng-dev to use new logging system rather
than directly calling console.* to create a better experience
for users.

PR Close #37232
2020-05-21 17:40:35 -04:00
Paul Gschwendtner 8fe532e593 build(dev-infra): cleanup package dependencies for shared package (#36980)
As per our discussion in the dev-infra sync meeting, we don't want
to have all dependencies show up as peer dependencies. Instead, we
only want to have larger dependencies such as `typescript` or buildifier
as peer dependencies. Tslib is also included for the sake of it being
generally a peer dependency of all Angular framework packages.

The rationale is that Yarn is smart enough to collapse packages
if all satisfy a given range. This means that we don't necessarily
need to have all dependencies as peer dependencies. The initial
idea was to keep all dependencies as peer dependencies so that
we have control over duplication of packages as downloading multiple
packages w/ different versions impacts local dev, CI and caches.

At the same time though, we don't want to bother with setting
up peer dependencies all the time. Not every consumer of the
shared dev-infra package would like to manually specify `yaml`
or `multimatch` etc. in the project `package.json`. Hence we
decided to go with a hybrid approach where only more impactful
dependencies are peer dependencies, and other smaller ones can
be standard depdencies that are usually collapsed by Yarn anyway.

Also this commit removes tslib from build targets that don't
rely on it.

PR Close #36980
2020-05-12 10:51:05 -07:00
Joey Perrott 7b5a0ba8c3 feat(dev-infra): create format tool in @angular/dev-infra-private (#36726)
Previously we used gulp to run our formatter, currently clang-format,
across our repository.  This new tool within ng-dev allows us to
migrate away from our gulp based solution as our gulp solution had
issue with memory pressure and would cause OOM errors with too large
of change sets.

PR Close #36726
2020-04-24 12:32:18 -07:00
Paul Gschwendtner f0c570bd41 feat(dev-infra): pullapprove verify should handle `files` in conditions (#36661)
Currently, when verifying our pullapprove configuration, we don't
respect modifications to the set of files in a condition.

e.g. It's not possible to do the following:

```
contains_any_globs(files.exclude(...), [
```

This prevents us from having codeowner groups which match a directory,
but want to filter out specific sub directories. For example, `fw-core`
matches all files in the core package. We want to exclude the schematics
from that glob. Usually we do this by another exclude condition.

This has a *significant* downside though. It means that fw-core will not
be requested if a PR changes schematic code, _and_ actual fw-core code.

To support these conditions, the pullapprove verification tool is
refactored, so that it no longer uses Regular expressions for parsing,
but rather evaluates the code through a dynamic function. This is
possible since the conditions are written in simple Python that can
be run in NodeJS too (with small modifications/transformations).

PR Close #36661
2020-04-23 12:17:10 -07:00
Joey Perrott 36535e9abd fix(dev-infra): correct pullapprove global approval regex (#36384)
PR Close #36384
2020-04-03 11:11:30 -07:00
Joey Perrott 43006bcc45 feat(dev-infra): standard CLI commands using yargs (#36326)
Creates a standard model for CLI commands provided by ng-dev.
Allows for us to have any of the tools/scripts extend to be
included in the ng-dev command, or be standalone using the same
yargs parser.

PR Close #36326
2020-04-01 15:24:28 -07:00
Joey Perrott 719224bffd feat(dev-infra): add support for new global approvers in pullapprove (#36324)
Pullapprove as added a few new features to allow for us to better
execute our expectation for global approvals. We need to allow for
an expectation that our global approver groups are not in the list
of approved groups. Additionally, since approval groups apply to
all files in the repo, the global approval groups also do not have
conditions defined for them, which means pullapprove verification
need to allow for no conditions need to be defined.

PR Close #36324
2020-04-01 13:25:48 -07:00
Andrew Kushnir c94a33c525 refactor(dev-infra): fix lint warnings for Pullapprove-related scripts (#36287)
The `dev-infra` scripts were added to the list of sources that should be verified with clang (b07b6edc2a), but the Pullapprove-related scripts that were merged before (83e4a76afa) doesn't pass these checks. This commit updates a couple scripts to have a proper formatting.

PR Close #36287
2020-03-28 23:21:12 -07:00
Joey Perrott 83e4a76afa feat(dev-infra): handle excluding files via globs in pullapprove (#36162)
Updates the pullapprove verification script to handle
cases of excluding globs from groups.

PR Close #36162
2020-03-27 11:12:48 -07:00
Paul Gschwendtner b7138c1ec5 build: remove rollup packaging from dev-infra (#35647)
The dev-infra package currently uses rollup for packaging. This has been
done initially as a way to workaround manifest paths being used in the
AMD JavaScript output.

The actual solution to this problem is setting module names that match
the `package.json` name. This ensures that the package can be consumed
correctly in Bazel, and through NPM. This allows us to get rid of the
rollup bundling, and we don't need to hard-code which dependencies
should be external or included.

Additionally, tools that are part of `dev-infra` can now specify
their external dependencies simply in the `package.json`. To reduce
version duplication, and out-of-sync versions, a new genrule has been
created that syncs the versions with the top-level project
`package.json`.

PR Close #35647
2020-03-17 12:27:07 -07:00
Joey Perrott e4b1e6c622 build: Add pullapprove verification tool to dev-infra-private package (#35911)
Migrates pullapprove verification tool to be available in the dev-infra-private
package

PR Close #35911
2020-03-10 14:17:56 -04:00