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(dev-infra): tool for staging and publishing releases #38656

Closed

Conversation

devversion
Copy link
Member

@devversion devversion commented Aug 31, 2020

Creates a tool for staging and publishing releases as per the
new branching and versioning that has been outlined in the following
document. The tool is intended to be used across the organization to
ensure consistent branching/versioning and labeling:

https://docs.google.com/document/d/197kVillDwx-RZtSVOBtPb4BBIAw0E9RT3q3v6DZkykU

The tool implements the actions as outlined in the following
initial plan: https://hackmd.io/2Le8leq0S6G_R5VEVTNK9A.

The implementation slightly differed in so far that it performs
staging and publishing together so that releasing is a single
convenient command. In case of errors for which re-running the
full command is not sufficient, we want to consider adding
recover functionality. e.g. when the staging completed, but the
actual NPM publishing aborted unexpectedly due to build errors.

@devversion devversion added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: dev-infra Issues related to Angular's own dev infra (build, test, CI, releasing) labels Aug 31, 2020
@ngbot ngbot bot added this to the needsTriage milestone Aug 31, 2020
dev-infra/release/publish/cli.ts Outdated Show resolved Hide resolved
dev-infra/release/publish/github-urls.ts Outdated Show resolved Hide resolved
dev-infra/release/publish/npm-publish.ts Outdated Show resolved Hide resolved
dev-infra/utils/config.ts Outdated Show resolved Hide resolved
dev-infra/pr/merge/defaults/branches.ts Outdated Show resolved Hide resolved
dev-infra/release/publish/actions.ts Show resolved Hide resolved
dev-infra/release/publish/actions.ts Outdated Show resolved Hide resolved
dev-infra/release/publish/actions.ts Show resolved Hide resolved
dev-infra/release/publish/actions.ts Outdated Show resolved Hide resolved
@josephperrott
Copy link
Member

@devversion I still have about 2/3s of the files to go through.

It might make sense to extract some of this into its own prefactor PR, particularly around configuration management between merge and publish

@devversion devversion force-pushed the feat/dev-infra-release-tool branch 4 times, most recently from 5e6abaf to b8b6cf3 Compare September 2, 2020 09:38
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

Finished a first past, sorry for the delays

A couple larger overall comments:

  • We should check the npm login status as part of the setup/startup process
  • Currently the design of this always has changes being brought into upstream via PR, this is not the model that framework uses, and personally I think isn't the model we should settle on. Most of these pieces are automated/trivial so adding in the PR process will make how long this release process takes grow crazily long.

dev-infra/utils/git/index.ts Show resolved Hide resolved
dev-infra/pr/merge/defaults/branches.ts Outdated Show resolved Hide resolved
dev-infra/pr/merge/defaults/branches.ts Outdated Show resolved Hide resolved
dev-infra/release/publish/actions.ts Show resolved Hide resolved
dev-infra/release/publish/actions.ts Show resolved Hide resolved
dev-infra/release/publish/npm-publish.ts Outdated Show resolved Hide resolved
dev-infra/release/publish/npm-publish.ts Outdated Show resolved Hide resolved
dev-infra/release/publish/test/cut-lts-patch.spec.ts Outdated Show resolved Hide resolved
dev-infra/release/publish/test/virtual-git-client.ts Outdated Show resolved Hide resolved
@devversion
Copy link
Member Author

Just for reference (based on our discussion in the dev-infra sync meeting): We decided to proceed with the concept of creating PRs instead of directly pushing staging changes to the upstream branches.

dev-infra/release/publish/npm-publish.ts Outdated Show resolved Hide resolved
dev-infra/utils/git/github.ts Show resolved Hide resolved
dev-infra/release/publish/actions/cut-next-prerelease.ts Outdated Show resolved Hide resolved
dev-infra/release/versioning/print-active-trains.ts Outdated Show resolved Hide resolved
dev-infra/release/build/cli.ts Show resolved Hide resolved
dev-infra/release/publish/actions.ts Outdated Show resolved Hide resolved
dev-infra/release/publish/index.ts Outdated Show resolved Hide resolved
@devversion
Copy link
Member Author

devversion commented Sep 8, 2020

@josephperrott Addressed all of your feedback. Pushed separate fixup commits (starting with 01b85de) to make review easier. It's a bit unfortunate how large this PR turned out, but I don't think there is a good way around it (since we build a rather complex tool where lots of parts need to play together well w/ iterations). Should be still review-able (given the fixup commits for feedback)

Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

I think we are in pretty good shape, just a left a couple of comments that are not a big deal.

I think our best bet is for you to address them (if appropriate) and then rebase the fixups you have created back into their commits, and we can have jelbourn@ start to review. That way he doesn't have to sift through all of the fixups. I will keep an eye out, but I think I am overall happy with where we are at.

dev-infra/release/versioning/active-release-trains.ts Outdated Show resolved Hide resolved
dev-infra/release/versioning/active-release-trains.ts Outdated Show resolved Hide resolved
dev-infra/utils/child-process.ts Outdated Show resolved Hide resolved
dev-infra/utils/child-process.ts Show resolved Hide resolved
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.
The git client respects the `SpawnSyncOptions` when a command
is executed. Currently it does not hide the command info
messages when commands are run in silent mode.

We fix this as part of this commit, so that the command info
is only printed to `debug` if `stdio` is set to `ignore`.

Additonally, the github token is made public so that it can be
used by commands if other repositories like forks are targeted.
Sets up the NPM `ora` package in the project and in dev-infra,
so that we can show progress spinners when needed. This is useful
in the publish release script when we wait for a pull request to
be merged.
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.
The dev-infra package is currently built with Bazel and ts-node.
In Bazel, the shared tsconfig from the `packages/` folder is used.

This means that the code is built in strict mode, but IDEs and
ts-node do not know about the strictness. This is because the tsconfig
is part of the `packages` folder and not accessible from the
dev-infra package. We fix this by adding an IDE and ts-node specific
tsconfig to the dev-infra package.

This helps with spotting compilation failures before building
with Bazel / waiting for CI to check build state.
alxhub pushed a commit that referenced this pull request Sep 28, 2020
…38656)

Moves the existing `ng-dev release env-stamp` command into a
subfolder so that the staging/publish tool can have its own
dedicated folder (without being polluted by the env-stamp logic).

Every subcommand should be in its own folder.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Adds a command for building all release packages. This command
is primarily used by the release tool for building release output
in version branches. The release tool cannot build the release packages
configured in `master` as those packages could differ from the
packages available in a given version branch. Also, the build process
could have changed, so we want to have an API for building
release packages that is guaranteed to be consistent across branches.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Introduces a new command for `ng-dev release`, so that the NPM
dist tag can be set for all configured NPM packages. This command
can be useful in case a manual tag needs to be set, but it is
primarily used by the release tooling when a new stable version
is cut, and when the previous patch branch needs to be set as LTS
version through a `v{major}-lts` dist tag.

It is necessary to have this as a command so that the release tool
can execute it for old branches where other packages might have been
configured. This is similar to the separate `ng-dev build` command
that we created.

Note that we also added logic for spawning a process conveniently
with different "console output" modes. This will be useful for
other command invocations in the release tool and it's generally
better than directly using native `child_process` as that one doesn't
log to the dev-infra debug log file.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Creates a tool for staging and publishing releases as per the
new branching and versioning that has been outlined in the following
document. The tool is intended to be used across the organization to
ensure consistent branching/versioning and labeling:

https://docs.google.com/document/d/197kVillDwx-RZtSVOBtPb4BBIAw0E9RT3q3v6DZkykU/edit#heading=h.s3qlps8f4zq7dd

The tool implements the actions as outlined in the following
initial plan: https://hackmd.io/2Le8leq0S6G_R5VEVTNK9A.

The implementation slightly diverged in so far that it performs
staging and publishing together so that releasing is a single
convenient command. In case of errors for which re-running the
full command is not sufficient, we want to consider adding
recover functionality. e.g. when the staging completed, but the
actual NPM publishing aborted unexpectedly due to build errors.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
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
alxhub pushed a commit that referenced this pull request Sep 28, 2020
The git client respects the `SpawnSyncOptions` when a command
is executed. Currently it does not hide the command info
messages when commands are run in silent mode.

We fix this as part of this commit, so that the command info
is only printed to `debug` if `stdio` is set to `ignore`.

Additonally, the github token is made public so that it can be
used by commands if other repositories like forks are targeted.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Sets up the NPM `ora` package in the project and in dev-infra,
so that we can show progress spinners when needed. This is useful
in the publish release script when we wait for a pull request to
be merged.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
…8656)

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
alxhub pushed a commit that referenced this pull request Sep 28, 2020
The dev-infra package is currently built with Bazel and ts-node.
In Bazel, the shared tsconfig from the `packages/` folder is used.

This means that the code is built in strict mode, but IDEs and
ts-node do not know about the strictness. This is because the tsconfig
is part of the `packages` folder and not accessible from the
dev-infra package. We fix this by adding an IDE and ts-node specific
tsconfig to the dev-infra package.

This helps with spotting compilation failures before building
with Bazel / waiting for CI to check build state.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
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
alxhub pushed a commit that referenced this pull request Sep 28, 2020
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
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Introduces a new configuration for the `ng-dev release` command. This
configuration will be the source of truth for all release packages
and how they can be built.

Additionally, in a temporary manner where each project has its own
way of generating the changelog, the changelog generation can be
configured. This will be removed in the future when there is
canonical changelog generation in the dev-infra shared package.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
#38656)

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
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Cleans up outdated comments in the shared dev-infra Git
utilities. We also export the Graphql client for consistency
as we expose the `GithubClient` and `GitClient` too.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Adds logic for determining active LTS branches for a given
release configuration. The active LTS branches can be determined
by querying NPM and matching dist tags against a specific
pattern. i.e. `v{major}-lts`.

This logic will be useful for the release tool that supports
publishing of active LTS version branches.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Adds a method for printing active release trains for a configured
project. This is helpful for the release tool that will print
the active release trains. Also this can be useful for the
caretaker status command, where we could print the active
version branches (i.e. "is there currently a feature-freeze branch").

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
…38656)

Adds a new folder to dev-infra where shared testing utilities
could be placed in. This commit already adds initial testing
utilities for dealing with the `GitClient` and SemVer versions.

The `GitClient` in the testing utilities simulates actual Git
behavior in a virtual manner. It's not complete at all, but can
be extended based on our needs. The currently implemented commands
are the most basic ones that we'd need for our release tooling.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
…38656)

Moves the existing `ng-dev release env-stamp` command into a
subfolder so that the staging/publish tool can have its own
dedicated folder (without being polluted by the env-stamp logic).

Every subcommand should be in its own folder.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Adds a command for building all release packages. This command
is primarily used by the release tool for building release output
in version branches. The release tool cannot build the release packages
configured in `master` as those packages could differ from the
packages available in a given version branch. Also, the build process
could have changed, so we want to have an API for building
release packages that is guaranteed to be consistent across branches.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Introduces a new command for `ng-dev release`, so that the NPM
dist tag can be set for all configured NPM packages. This command
can be useful in case a manual tag needs to be set, but it is
primarily used by the release tooling when a new stable version
is cut, and when the previous patch branch needs to be set as LTS
version through a `v{major}-lts` dist tag.

It is necessary to have this as a command so that the release tool
can execute it for old branches where other packages might have been
configured. This is similar to the separate `ng-dev build` command
that we created.

Note that we also added logic for spawning a process conveniently
with different "console output" modes. This will be useful for
other command invocations in the release tool and it's generally
better than directly using native `child_process` as that one doesn't
log to the dev-infra debug log file.

PR Close #38656
alxhub pushed a commit that referenced this pull request Sep 28, 2020
Creates a tool for staging and publishing releases as per the
new branching and versioning that has been outlined in the following
document. The tool is intended to be used across the organization to
ensure consistent branching/versioning and labeling:

https://docs.google.com/document/d/197kVillDwx-RZtSVOBtPb4BBIAw0E9RT3q3v6DZkykU/edit#heading=h.s3qlps8f4zq7dd

The tool implements the actions as outlined in the following
initial plan: https://hackmd.io/2Le8leq0S6G_R5VEVTNK9A.

The implementation slightly diverged in so far that it performs
staging and publishing together so that releasing is a single
convenient command. In case of errors for which re-running the
full command is not sufficient, we want to consider adding
recover functionality. e.g. when the staging completed, but the
actual NPM publishing aborted unexpectedly due to build errors.

PR Close #38656
gkalpak added a commit to gkalpak/angular that referenced this pull request Oct 6, 2020
This commit addresses comments from [my review][1] on PR angular#38656 (which
was merged without comments addressed). The changes are mostly related
to code style and typos.

[1]: angular#38656 (review)
gkalpak added a commit to gkalpak/angular that referenced this pull request Oct 6, 2020
This commit addresses comments from [my review][1] on PR angular#38656 (which
was merged without comments addressed). The changes are mostly related
to code style and typos.

[1]: angular#38656 (review)
gkalpak added a commit to gkalpak/angular that referenced this pull request Oct 10, 2020
This commit addresses comments from [my review][1] on PR angular#38656 (which
was merged without comments addressed). The changes are mostly related
to code style and typos.

[1]: angular#38656 (review)
atscott pushed a commit that referenced this pull request Oct 12, 2020
This commit addresses comments from [my review][1] on PR #38656 (which
was merged without comments addressed). The changes are mostly related
to code style and typos.

[1]: #38656 (review)

PR Close #39135
atscott pushed a commit that referenced this pull request Oct 12, 2020
This commit addresses comments from [my review][1] on PR #38656 (which
was merged without comments addressed). The changes are mostly related
to code style and typos.

[1]: #38656 (review)

PR Close #39135
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: dev-infra Issues related to Angular's own dev infra (build, test, CI, releasing) cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants