Currently the `GitClient` accepts a generic parameter for determining
whether the `githubToken` should be set or not. This worked fine so far
in terms of distinguishing between an authenticated and
non-authenticated git client instance, but if we intend to conditionally
show methods only for authenticated instances, the generic parameter
is not suitable.
This commit splits up the `GitClient` into two classes. One for
the base logic without any authorization, and a second class that
extends the base logic with authentication logic. i.e. the
`AuthenticatedGitClient`. This allows us to have specific methods only
for the authenticated instance. e.g.
* `hasOauthScopes` has been moved to only exist for authenticated
instances.
* the GraphQL functionality within `gitClient.github` is not
accessible for non-authenticated instances. GraphQL API requires
authentication as per Github.
The initial motiviation for this was that we want to throw if
`hasOAuthScopes` is called without the Octokit instance having
a token configured. This should help avoiding issues as within
3b434ed94d
that prevented the caretaker process momentarily.
Additionally, the Git client has moved from `index.ts` to
`git-client.ts` for better discoverability in the codebase.
PR Close#42468
In the dev-infra merge tool, a test fails locally if it runs on
a system with a different system locale. We can either make the
test more flexible, or update the merge tool to always print the date
in `en-US` format. The latter is more reasonable given our console
messages being in english anyway.
PR Close#42470
This change moves the `dev-infra/browsers` folder into `dev-infra/bazel`.
The browser folder is providing custom configuration for Bazel, so it
should live within the `bazel` folder for a more well-structured
`dev-infra` folder.
PR Close#42268
Removes the duplicate `hasLocalChanges` method from the Git client. We
already have `hasUncommittedChanges`. Also removes a TODO for adding
`assertNoLocalChanges` as it seems more flexible to manually check
(i.e. better messaging with context on the current tool; e.g. "cannot
perform rebase")
PR Close#42409
As per SemVer, deprecations should only be released if the
minor version is incremented. This means that we should error
if commits w/ deprecations target an upcoming patch, LTS release.
Additionally, we also error if deprecations are made as part of
an upcoming release-candidate.
PR Close#42454
Adds the JSDoc `@throws` annotation to functions that would throw
within in the ng-dev tool. We want to add `@throws` so clearly
communicate if the invocation of a function results in errors or not.
This helps if IDEs show the `@throws` annotation on invocation, or
if TS ever lands a feature like Java `throws`, with more fine-grained
and typed error handling.
PR Close#42454
If a commit message currently mentions the breaking change or
deprecation note keywords, the commit message parse logic
accidentally picks up the note. This could then accidentally
prevent the commit from being merged (e.g. if the commit targets
the patch branch but mentioned the `BREAKING CHANGE: ` marker).
This commit switches the commit message notes pattern to only
capture notes at the beginning of a line (also allowing accidental
whitespace). This matches with the format we describe in our
contribution guide, as well as with our commit message validation
logic that also assumes notes at the beginning of a line.
PR Close#42436
Currently the commit message validation tool from `ng-dev` validates
the `BREAKING CHANGE:` commit message notes. This commit adds a similar
check for `DEPRECATED:` commit message notes.
Additionally, the check for breaking changes is reworked slightly to
be more tolerant (i.e. if there is only a single line break after the
summary; this is acceptable as per the parser and commonly done in the
COMP repo). The checks have been updated to capture wrong keywords that
are commonly used instead of the correct one. e.g. if a commit message
uses `DEPRECATIONS:` instead of `DEPRECATED:`, the validation will fail.
This prevents changelog generation issues where breaking change notes,
or deprecations are missing. This happened in the COMP repo where
the `DEPRECATED:` keyword was used incorrectly. See:
99391e7939
PR Close#42436
Updates the outdated build-worker.js file. This file has
been accidentally not updated when we fixed the Octokit
authentication to unblock the caretaker.
We recently updated `@octokit/rest` to a more recent version. For this
the Github client had to be refactored to account for new types of
Octokit. With the refactorings the authentication of the Github
client broke as we used an incorrect option for setting the OAuth
token. The TS transpilation did not fail because the Octokit types
support arbitrary options in the constructor. This is not solvable
on our side, so we cannot prevent similar issues in the future
unfortunately. Adding tests for authentication is not a reasonable
option either.
Add a command to build the release output without stamping for release
and link via `yarn link` the generated builds to a project provided.
PR Close#42319
Move the release notes tooling under its own directory under release rather than
within publish, in preparation to have a release note generation command for ad-hoc
release note generation.
PR Close#42225
When `ng-dev format --check` is run, the ng-dev tool prints out
all files that are out-of-date. We recently updated the format
tool to also capture `stderr` for failed files. This broke the
console message as we did not unwrap the `FormatFailure` to
their file path when printing the "ng-dev format" fix command.
PR Close#42252
Currently if formatting for a file fails due a formatter error,
the `ng-dev` tool reports that formatting failed, but no actual
error (or involved file) is printed out. This commit prints out
the failed files with their error message.
PR Close#42202
Previously, the dev-infra release tool would publish major versions
directly to the NPM `@latest` dist tag. This is correct in theory, but
rather unpractical given that we want to publish packages first as
`@next` so that other dependent Angular packages can update too,
allowing us to publish all main Angular packages (from FW, COMP
and TOOL) at the same time to `@latest` on NPM.
This involves creating a new release action for re-tagging the
previously released major as `@latest` on NPM.
PR Close#42133
Instead of passing `string` in the release tool for NPM dist tags, we
should use a union string type that limits the tags to `latest`, `next`
and anything matching `v{number}-lts`. This avoids mistakes at
compilation-level if an invalid/unknown tag would be set by a release
action.
PR Close#42133
Currently the ng-dev release tool always run `bazel clean` before
calling the configured build release function. The clean is necessary
to ensure the release output is actually built; and not restored
from previous builds which could have different bazel workspace
status variables (which provide the NPM package version).
Instead of doing this as part of the release tool, the
actual script running to build the release output should
run the `bazel clean`. The release tool does not intend to
know about details on how the release output is built. This
is necessary because the build setup could vary between version
branches (especially for older ones; such as LTS version branches).
PR Close#42101
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
Previously by default GitClient would log the commands it was executing at the
INFO level. This change moves the default level of this logging to DEBUG, while
still allowing callers of the methods to set the log level back to INFO.
PR Close#41899
We recently added a sanity check that ensures that `python` is
available for Bazel. The check relies on `/usr/bin/python` to check
if python is available w/ the correct version. The problem is
that on Windows there is no guarantee that the `/usr/bin` folder
exists, so the `ng-dev release publish` command always fails.
We fix this by just accessing the `env` binary that is also consulted
by scripts executed within Bazel actions. The `env` binary can be
assumed exist in the shell's `$PATH` and can point us to Python
as if it would be executed within Bazel.
PR Close#41943
Currently we have a common utility method for running commands
in a child process. This method pipes all stdout and stderr, but sets
the `stdin` to `inherited`. This seemed to work as expected in terms of
allowing interactive commands being executed, but it messes with the
TTY in Windows (and potentially other platforms) so that colors and
prompts no longer work properly. See attached screenshot.
We fix this by not inheriting the stdin by default; but exposing
a dedicated method for interactive commands. This results in more
readable and obvious code too, so it's worth making this change
regardless of the TTY issues.
PR Close#41948
Bazel no longer recommends the use of `remote_execution_properties`
within `platform` definitions. Bazel intends to replace this attribute
with an object literal based attribute called `exec_properties`.
This simplifies the platform configuration and makes it more
readable. Additionally this make inheritance and overriding easier.
PR Close#41941
Currently if a major release-train in the `release-candidate`/`feature-freeze`
phase becomes `latest`, we intend to set the NPM LTS dist tag for all packages
of the previous major (as the old release train in `latest` moves into LTS phase).
The logic for this exists but the release tool sets the NPM dist tag for
all packages of the new major. This means that the script might error if
a new package is part of the new major; or it could cause a deleted
package to not receive the LTS tag properly.
PR Close#41946
Remove the getBaseDir method from GitClient as the baseDir is determined
once during construction and does not need to be rediscovered. The property
access should always be used.
PR Close#41898
This commit uses the correct property name (`prettier` vs `pretter`) to
check whether prettier is enabled via the config.
(It also fixes some typos in `dev-infra`.)
PR Close#41860
A test for the dev-infra publish tool currently relies on an external
http request by accident. This will break if we disallow internet
access within RBE containers.
PR Close#41767
Introduces a shared Bazel platform for remote execution builds
using Google cloud. Previously we used `bazel_toolchains` for
providing the platform w/ additional CPP and Java toolchains
`bazel_toolchains` no longer provides default toolchains with the
latest version, but provides a tool (linux and windows only) for
generating toolchain/platforms, which then need to be checked
into the repository. This is quite inconvenient and cumbersome
(especially with no macOS support), so we just provide our own
platform and CPP toolchain within `//dev-infra`. This is more
simple than all the effort we'd need to make the toolchain
generation tool work (while it would also increase the amount
of checked-in sources significantly; with more unused toolchains
for CPP or Java)
PR Close#41767
Since recently, the `githubToken` parameter passed to the `ReleaseTool`
constructor is no longer used. This commit removes the unused parameter
and also the corresponding argument from the `publish` script.
PR Close#41688
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
Paul Gschwendtner
Joey Perrott
Zach Arend
George Kalpakas
Alan Agius