Currently if a PR modifies any file that configures a Github action
(e.g. a workflow file), the caretaker might face an error when merging
such PR:
```
! [remote rejected] merge_pr_target_11.2.x -> 11.2.x (refusing to allow a Personal Access Token to create or update workflow
```
This happens because Github requires the token being used for the
push operation to have the `workflow` scope set. This is a special
scope added by Github to ensure that no changes can be made on
upstream branches that might expose the `GITHUB_TOKEN` environment
variable, which is available for push builds and could cause the
token being leaked.
With this commit we enforce that the caretaker adds the workflow
scope to their github token. Since PRs can only be merged if reviewed
thoroughly, it's acceptable to allow workflow file changes being
merged through the merge tool by the caretaker (especially since we
also allow CircleCI config files being merged with the default
`repo`/`public_repo` scope).
PR Close#41989
As `getRepoBaseDir()` relies on git, it should be a method on `GitClient` for retrieval
rather than its own utility outside of the common GitClient used for all git ineractions.
PR Close#41527
With this change we update several dependencies to avoid Renovate creating a lot of PRs during onboarding. We also remove yarn workspaces as after further analysis these are not needed.
Certain dependencies such as `@octokit/rest`, `remark` and `@babel/*` have not been updated as they require a decent amount of work to update, and it's best to leave them for a seperate PR.
PR Close#41434
Creates a singleton class for GitClient rather than relying on creating an instance to
require being passed around throughout its usages.
PR Close#41515
During merging with `ng-dev pr merge` tooling will ensure that pull requests are
properly labeled for breaking changes. Pull requests with commits noting breaking
changes must also be labeled as such, additionally pull requests with breaking
change labels must contain commits noting breaking changes.
Fixes#38776
PR Close#41546
After updating to use the v4 graphql api in the merge tooling, the githubTargetBranch
was set to be the sha of the latest commit rather than the branch name of the target
branch. This caused our tooling to mismatch which branches were actually being targeted
with the effect that if a PR targeted only the patch branch (i.e. labeled `target: patch`
and targeting `11.2.x` in github)it would still expect to merge into both `11.2.x` and
`master`. This is now corrected to once again use the branch name, restoring to the
previous functionality.
PR Close#41523
Add a property, `targetLabelExemptScopes`, to the merge configuration allowing certain
scopes to be exempted from requirements for features and breaking changes only included
in PRs targetting certain labels.
PR Close#41459
Check commits in pull requests to ensure the pr can be merged into the target branch. Confirms
that prs targeting minor do not contain breaking changes, and prs targeting patch or lts do not
contain breaking changes or `feat` commits.
PR Close#41459
Check a range of commits by retrieving the log files to be parsed with the expected
format for the parser.
This change is in part of a larger set of changes making the process for obtaining
and parsing commits for release note creation and message validation consistent.
This consistency will make it easier to debug as well as ease the design of tooling
which is built on top of these processes.
PR Close#41341
Use conventional-commits-parser for parsing commits for validation, this is being done
in anticipation of relying on this parser for release note creation. Unifying how commits
are parsed will provide the most consistency in our tooling.
PR Close#41286
Add support for a `--no-branch-prompt` flag for the `ng-dev pr merge` tool. This
flag enables suppression of the confirmation prompt for which branches the specified
PR will merge into.
PR Close#40528
When multiple target labels are applied to a PR, it should be considered
invalid as our tooling does not support a single PR targetting multiple
trains/versions.
PR Close#40156
With the migration to husky@5, the environment variable used to skip husky changed from
`HUSKY_SKIP_HOOKS` to `HUSKY`. To continue skipping husky during the merge process as
expected, the environment variable used is updated.
PR Close#40025
Previously, when a PR which does not target the master branch in the Github UI was
merged it would not close automatically. This change detects when this case occurs
and closes the PR via the Github API.
For example:
A PR which targets the 11.0.x branch in the Github UI has the `target: patch` label
This PR is only pushed into the 11.0.x branch, which does not trigger Github's
reference based actions to close the PR.
PR Close#39979
When fetch is run in normal mode, the `git-fetch-pack` plumbing command
outputs progress to stderr. Since this is unnecessary progress
information for ng-dev usages, it should be suppressed instead.
PR Close#39503
Rather than running ng-dev via ts-node, going forward ng-dev is generated and run
locally via node. Additionally, the generated file is tested on each commit to
ensure that the local generated version stays up to date.
PR Close#39089
Perviously, it was not immediately clear what branches a PR would merge
into during the merge process. This prompt allows for caretakers to
understand and acknowledge where the PR will merge to.
PR Close#39333
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.
PR Close#39182
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
We initially added logic for determining active release trains into
the merge script. Given we now build more tools that rely on this
information, we move the logic into a more general "versioning" folder
that can contain common logic following the versioning document for the
Angular organization.
PR Close#38656
Exposes logic for dealing with LTS branches, so that the release
tool can re-use it for cutting LTS patch releases.
Eventually, we can move all of this logic to a more dedicated
folder instead of having it inside the merge folder.
PR Close#38656
Instead of maintaining multiple interface for grouping
owner name and repo name, we expose a shared interface
describing a Github repository.
One unfortunate downside is that the GraphQL Github
and Rest API diverge slightly with the key for the
repository name. i.e. rest uses `repo` for the name
of a repository, while GraphQL uses `name` for the name.
If that would be consistent, we could use the rest operator
to pass a repository to the Octokit REST or GraphQL API. This
does not work, so we have a small manual overhead as seen
in the `branches.ts` file.
PR Close#38656
Previously, the logic for determing the active release trains did not
return the resolved version of a release train. With the publish script
being created, we need this information and can just pass it through,
so that we do not need to fetch and parse the package.json of given
branches multiple times.
PR Close#38656
Instead of repeating the logic for adding the github token to
a repository git url, we add a shared function for automatically
computing the URls with token.
Additionally, URLs for updating/generating tokens have been moved
to a dedicated file in the `utils` folder. Also while being at it,
the yargs github token helper is also moved into the dedicated
Git/Github related util folder.
PR Close#38656
During the merge process, all validations have already been completed so git commit
hooks can be safely skipped. This additionally, prevents errors from occuring which
would be caused the commit hooks executing, such as when yarn updates and then yarn
commands are unable to run within the same process.
PR Close#38888
Creates a mixin for requiring a github token to be provided to a command. This mixin
allows for a centralized management of the requirement and handling of the github-token.
PR Close#38630
Currently the merge script default branch configuration throws an error
if an unexpected version branch is discovered. The error right now
assumes to much knowledge of the logic and the document outlining
the release trains conceptually.
We change it to something more easy to understand that doesn't require
full understanding of the versioning/labeling/branching document that
has been created for the Angular organization.
PR Close#38622
Creates a tool within ng-dev to checkout a pending PR from the upstream repository. This automates
an action that many developers on the Angular team need to do periodically in the process of testing
and reviewing incoming PRs.
Example usage:
ng-dev pr checkout <pr-number>
PR Close#38474
Previously, each Angular repository had its own strategy/configuration
for merging pull requests and cherry-picking. We worked out a new
strategy for labeling/branching/versioning that should be the canonical
strategy for all actively maintained projects in the Angular organization.
This PR provides a `ng-dev` merge configuration that implements the
labeling/branching/merging as per the approved proposal.
See the following document for the proposal this commit is based on
for the merge script labeling/branching: https://docs.google.com/document/d/197kVillDwx-RZtSVOBtPb4BBIAw0E9RT3q3v6DZkykU
The merge tool label configuration can be conveniently accesed
within each `.ng-dev` configuration, and can also be extended
if there are special labels on individual projects. This is one
of the reasons why the labels are not directly built into the
merge script. The script should remain unopinionated and flexible.
The configuration is conceptually powerful enough to achieve the
procedures as outlined in the versioning/branching/labeling proposal.
PR Close#38223
The merge tool provides a way for configurations to determine the branches
for a label lazily. This is supported because it allows labels to respect
the currently selected base branch through the Github UI. e.g. if `target: label`
is applied on a PR and the PR is based on the patch branch, then the change
could only go into the selected target branch, while if it would be based on
`master`, the change would be cherry-picked to `master` too. This allows for
convenient back-porting of changes if they did not apply cleanly to the primary
development branch (`master`).
We want to expand this function so that it is possible to report failures if an
invalid target label is appplied (e.g. `target: major` not allowed in
some situations), or if the Github base branch is not valid for the given target
label (e.g. if `target: lts` is used, but it's not based on a LTS branch).
PR Close#38223
The merge script currently accepts a configuration function that will
be invoked _only_ when the `ng-dev merge` command is executed. This
has been done that way because the merge tooling usually relies on
external requests to Git or NPM for constructing the branch configurations.
We do not want to perform these slow external queries on any `ng-dev` command
though, so this became a lazily invoked function.
This commit adds support for these configuration functions to run
asynchronously (by returning a Promise that will be awaited), so that
requests could also be made to the Github API. This is benefical as it
could avoid dependence on the local Git state and the HTTP requests
are more powerful/faster.
Additionally, in order to be able to perform Github API requests
with an authenticated instance, the merge tool will pass through
a `GithubClient` instance that uses the specified `--github-token`
(or from the environment). This ensures that all API requests use
the same `GithubClient` instance and can be authenticated (mitigating
potential rate limits).
PR Close#38223
The merge script uses `git cherry-pick` for both the API merge strategy
and the autosquash strategy. It uses cherry-pick to push commits to
different target branches (e.g. into the `10.0.x` branch).
Those commits never point to the commits that landed in the primary
Github branch though. For the autosquash strategy the pull request number
is always included, so there is a way to go back to the source. On the other
hand though, for commits cherry-picked in the API merge strategy, the
pull request number might not always be included (due to Github's
implementation of the rebase merge method).
e.g.
27f52711c0
For those cases we'd want to link the cherry-picked commits to the
original commits so that the corresponding PR is easier to track
down. This is not needed for the autosquash strategy (as outlined
before), but it would have been good for consistency. Unfortunately
though this would rather complicate the strategy as the autosquash
strategy cherry-picks directly from the PR head, so the SHAs that
are used in the primary branch are not known.
PR Close#37889
We recently added OAuth scope checking to the dev-infra Git client
and started leveraging it for the merge script. We set the `repo` scope
as required for running the merge script. We can loosen this requirement
as in the Angular org where the script is consumed, only pull requests on
public repositories are merged through the script.
This should help with reducing the risk with compromised tokens as no
access had to be granted on `repo:invite`, `repo_deployment` etc.
PR Close#37718
Scripts provided in the `ng-dev` command might use local `git`
commands. For such scripts, we keep track of the branch that
has been checked out before the command has been invoked.
We do this so that we can later (upon command completion)
restore back to the original branch. We do not want to
leave the Git repository in a dirty state.
It looks like this logic currently only deals with branches
but does not work properly when a command is invoked from
a detached head. We can make it work by just checking out
the previous revision (if no branch is checked out).
PR Close#37737
GitClient now uses GithubClient for github API interactions. GithubClient is
a class which extends Octokit and provides a member which allows for GraphQL
requests against the Github GraphQL api, as well as providing convenience methods
for common/repeated Github API requests.
PR Close#37593
Paul Gschwendtner
Joey Perrott
George Kalpakas
Alan Agius
Greg Magolan