Skip to content

build: experimental stamping and snapshot repo substitutions#22491

Merged
alan-agius4 merged 3 commits intoangular:masterfrom
aspect-build:experimental-stamp
Jan 21, 2022
Merged

build: experimental stamping and snapshot repo substitutions#22491
alan-agius4 merged 3 commits intoangular:masterfrom
aspect-build:experimental-stamp

Conversation

@kormide
Copy link
Copy Markdown
Contributor

@kormide kormide commented Jan 12, 2022

@josephperrott @devversion @gregmagolan

The first commit performs stamping for experimental packages which get a different version format (e.g., 0.1302.0-next.1 vs 13.2.0-next.1). Basically any package with "experimental": true gets the alternative placeholder. If it becomes non-experimental then the version placeholder should also change. The two should be manually kept in sync.

The second commit performs substitutions for github snapshot repos in a snapshot build similar to the non-bazel snapshot build, so it stamps versions like github:angular/angular-devkit-architect-builds:174fe1056a9ca4987c9176f5f2d18faf0e71c92b.

Comment thread packages/angular/cli/models/version.ts
Comment thread tools/snapshot_repo_filter.bzl Outdated
load("//:constants.bzl", "SNAPSHOT_REPOS")

# jq filter that replaces package.json dependencies with snapshot repos
SNAPSHOT_REPO_JQ_FILTER = _generate_snapshot_repo_filter()
Copy link
Copy Markdown
Contributor Author

@kormide kormide Jan 12, 2022

Choose a reason for hiding this comment

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

If preferred, I could just hardcode this into a jq filter file. I thought it would be cleaner to generate it from SNAPSHOT_REPOS, but I'm on the fence.

Comment thread scripts/build.ts
Comment on lines +416 to +420
} else if ((obj[depName] as string).match(/\b0\.0\.0-EXPERIMENTAL-PLACEHOLDER\b/)) {
obj[depName] = (obj[depName] as string).replace(
/\b0\.0\.0-EXPERIMENTAL-PLACEHOLDER\b/,
v,
);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not ideal to add this extra condition here but this build script will be deleted after flipping to the bazel build script.

Copy link
Copy Markdown
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.

LGTM

@josephperrott josephperrott added area: build & ci Related the build and CI infrastructure of the project target: patch This PR is targeted for the next patch release labels Jan 19, 2022
@josephperrott josephperrott added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jan 19, 2022
Comment thread constants.bzl Outdated
Copy link
Copy Markdown
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM, just one small nit

@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 21, 2022
@alan-agius4 alan-agius4 merged commit 331c34c into angular:master Jan 21, 2022
@angular-automatic-lock-bot
Copy link
Copy Markdown

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 Feb 21, 2022
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: build & ci Related the build and CI infrastructure of the project target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants