build: use bazel for snapshot builds by kormide · Pull Request #22543 · angular/angular-cli

kormide · 2022-01-22T00:07:28Z

Here are the snapshot repo diffs caused by flipping to the bazel build.

Snapshot repo	Diff
@angular/cli	diff
@angular/pwa	diff
@angular-devkit/architect	diff
@angular-devkit/architect-cli	diff
@angular-devkit/build-angular	diff
@angular-devkit/build-webpack	diff
@angular-devkit/core	diff
@angular-devkit/schematics	diff
@angular-devkit/schematics-cli	diff
@ngtools/webpack	diff
@schematics/angular	diff

Note that some changes in these diffs may be caused by new things pushed to master of the "build" repos.

CI runs for the snapshot-test branch set up to test the ci. The build artifacts are published to the build repos under the same snapshot-test branch name.

kormide · 2022-01-28T21:29:18Z

    filter = ""
    for (i, pkg_name) in enumerate(SNAPSHOT_REPOS.keys()):
-        filter += "{sep}(..|objects|select(has(\"{pkg_name}\")))[\"{pkg_name}\"] |= \"github:{snapshot_repo}:BUILD_SCM_HASH-PLACEHOLDER\"\n".format(
+        filter += "{sep}(..|objects|select(has(\"{pkg_name}\")))[\"{pkg_name}\"] |= \"github:{snapshot_repo}#BUILD_SCM_HASH-PLACEHOLDER\"\n".format(


Bug in the github dependency format.

josephperrott

LGTM

kormide · 2022-01-28T22:01:23Z

I'd like to run a few more tests before we mark this as ready to merge.

kormide · 2022-01-29T00:49:22Z

I ran into an issue trying to manually install the snapshots. The bazel build stamps snapshot versions of dependencies like:

github:angular/angular-devkit-architect-builds#bb8bc80581417898b10bdc23ef4d9ee4295da2a9

It's using the stamp variable {BUILD_SCM_COMMIT_SHA} provided by the ng-dev tool.

However, legacy build stamps using an abbreviated version of the SHA1, and the snapshot script tags the commit in the build repo using the abbreviated version, so running an npm install of the snapshots fails.

I tried changing the snapshot script to tag using the longer hash, however git complains that the name is too long.

Something I could do is to change the dev-infra repo to output another variable with the abbreviated hash (e.g., {BUILD_SCM_COMMIT_SHA_ABBREV}) so that we can stamp that instead. Wdyt @josephperrott?

alan-agius4 · 2022-01-31T12:08:00Z

Couple of questions.

why is the version 13.2.0-next and not 14.0.0-next?
can we also turn off source-map generation?

kormide · 2022-01-31T16:41:31Z

* why is the version `13.2.0-next` and not `14.0.0-next`?

I think that's just because master in the build repo has a more recent snapshot that the branch I'm working off of. That should resolve itself the next time a snapshot is pushed on snapshot-test.

* can we also turn off source-map generation?

Unfortunately I don't think we can with ts_library. @josephperrott previously ok'ed this, and it's consistent with what the angular bazel build produces.

For reference here's the CI: https://app.circleci.com/pipelines/github/angular/angular-cli?branch=snapshot-test&filter=all

alan-agius4 · 2022-01-31T17:07:44Z

Unfortunately I don't think we can with ts_library. @josephperrott previously ok'ed this, and it's consistent with what the angular bazel build produces.

This is not a big of deal, although sourcemaps are a bit redundant here and kinda of extra mbs over the wire and disk in this case. Unlike for angular/angular were they are useful for app developers.

But yeah, I don't think there is a away to disable sourcemaps in ts_library.

kormide · 2022-01-31T20:07:43Z

I'm now able to install the snapshot build locally. I'm getting a failure on the e2e windows tests, which I've seen in other ci runs so it appears unrelated to my changes. Is that test suite known to be flaky? https://app.circleci.com/pipelines/github/angular/angular-cli/20377/workflows/dc026fcf-1783-4d3d-9ac9-9ec9854c1eb3/jobs/286136

alan-agius4 · 2022-01-31T20:18:33Z

@kormide, looks like our Node.Js v12 CI is red.

Will investigate tomorrow, however it’s nothing related to this change.

kormide · 2022-01-31T21:28:49Z

@kormide, looks like our Node.Js v12 CI is red.

Will investigate tomorrow, however it’s nothing related to this change.

In that case, I think this is ready to go, unless you can think of anything else I should test first.

alan-agius4 · 2022-01-31T21:49:30Z

Do you want to wait for @gregmagolan’s review?

kormide · 2022-01-31T21:53:19Z

Do you want to wait for @gregmagolan’s review?

Your and @josephperrott's review is sufficient. @gregmagolan reviewed previously and is in the loop, but hes out today.

alan-agius4

LGTM

dgp1130 · 2022-02-02T01:19:28Z

There was a trivial conflict with package.json / yarn.lock in 13.2.x. So I merged this to master and did a manual cherry pick to 13.2.x in 491e210.

angular-automatic-lock-bot · 2022-03-05T08:04:15Z

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

gregmagolan approved these changes Jan 24, 2022

View reviewed changes

kormide commented Jan 28, 2022

View reviewed changes

Comment thread lib/packages.ts

kormide commented Jan 28, 2022

View reviewed changes

Comment thread .circleci/config.yml

kormide commented Jan 28, 2022

View reviewed changes

kormide requested review from gregmagolan and josephperrott January 28, 2022 21:41

josephperrott approved these changes Jan 28, 2022

View reviewed changes

kormide added 2 commits January 31, 2022 11:49

build: add missing dependency for tar/repo substitution

10adcea

build: use bazel for snapshot builds

8c65ad1

alan-agius4 added the target: patch This PR is targeted for the next patch release label Jan 31, 2022

alan-agius4 removed the request for review from gregmagolan January 31, 2022 21:54

alan-agius4 added the action: merge The PR is ready for merge by the caretaker label Jan 31, 2022

alan-agius4 approved these changes Jan 31, 2022

View reviewed changes

dgp1130 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 Feb 2, 2022

dgp1130 merged commit 82971c7 into angular:master Feb 2, 2022

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Mar 5, 2022

Conversation

kormide commented Jan 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kormide Jan 28, 2022

Choose a reason for hiding this comment

Uh oh!

josephperrott left a comment

Choose a reason for hiding this comment

Uh oh!

kormide commented Jan 28, 2022

Uh oh!

kormide commented Jan 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alan-agius4 commented Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kormide commented Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alan-agius4 commented Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kormide commented Jan 31, 2022

Uh oh!

alan-agius4 commented Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kormide commented Jan 31, 2022

Uh oh!

alan-agius4 commented Jan 31, 2022

Uh oh!

kormide commented Jan 31, 2022

Uh oh!

alan-agius4 left a comment

Choose a reason for hiding this comment

Uh oh!

dgp1130 commented Feb 2, 2022

Uh oh!

angular-automatic-lock-bot Bot commented Mar 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kormide commented Jan 22, 2022 •

edited

Loading

kormide commented Jan 29, 2022 •

edited

Loading

alan-agius4 commented Jan 31, 2022 •

edited

Loading

kormide commented Jan 31, 2022 •

edited

Loading

alan-agius4 commented Jan 31, 2022 •

edited

Loading

alan-agius4 commented Jan 31, 2022 •

edited

Loading