Skip to content

build: snapshot builds incorrectly modify semver versions #20053

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

Merged

Conversation

devversion
Copy link
Member

@devversion devversion commented Jul 21, 2020

An interesting case that came up in v10.0.0 with the docs-content. The
snapshot release package and docs-content output had various
@breaking-change notes in the source code referring to v10.0.0 as
certain changes are planned to be made at that point.

The snapshot deploy scripts pick up the version from the package.json
file and replace it in the output packages with a more concrete version
that includes the SHA. This meant that we accidentally also overwrote
versions as in the @breaking-change notes (ultimately making it
difficult for us to use the latest docs-content for the v10 release as
it was incorrect).

Example snapshot commit: angular/material2-docs-content@e624c71

We fix this by not including the SHA as part of the deployment, but
rather including the SHA when building the NPM packages. At that point,
we can safely just replace instances of the 0.0.0-PLACEHOLDER without
having to worry about accidental version overriding.

To achieve this, we update our release stamping script to have two
modes. i.e. snapshot build mode and release mode. Framework does this
by checking the Git tag history but that seems less ideal as it makes
the release output building reliant on external factors while our
stamping is self-contained within the checked out code revision.

@devversion devversion added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent merge safe target: patch This PR is targeted for the next patch release labels Jul 21, 2020
@devversion devversion requested a review from a team as a code owner July 21, 2020 09:21
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 21, 2020
@devversion devversion force-pushed the build/fix-version-stamping-snapshot branch from bb2093a to ef43741 Compare July 21, 2020 10:07
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.

LGTM

Just one comment about wording for an error message

An interesting case that came up in v10.0.0 with the docs-content. The
snapshot release package and docs-content output had various
`@breaking-change` notes in the source code referring to `v10.0.0` as
certain changes are planned to be made at that point.

The snapshot deploy scripts pick up the version from the package.json
file and replace it in the output packages with a more concrete version
that includes the SHA. This meant that we accidentally also overwrote
versions as in the `@breaking-change` notes (ultimately making it
difficult for us to use the latest docs-content for the v10 release as
it was incorrect).

Example snapshot commit:
angular/material2-docs-content@e624c71

We fix this by not including the SHA as part of the deployment, but
rather including the SHA when building the NPM packages. At that point,
we can safely just replace instances of the `0.0.0-PLACEHOLDER` without
having to worry about accidental version overriding.

To achieve this, we update our release stamping script to have two
modes. i.e. snapshot build mode and release mode. Framework does this
by checking the Git tag history but that seems less ideal as it makes
the release output building reliant on external factors while our
stamping is self-contained within the checked out code revision.
@devversion devversion force-pushed the build/fix-version-stamping-snapshot branch from ef43741 to a558cf1 Compare July 22, 2020 08:33
@devversion devversion added the action: merge The PR is ready for merge by the caretaker label Jul 22, 2020
@wagnermaciel wagnermaciel merged commit 33b31b1 into angular:master Jul 22, 2020
wagnermaciel pushed a commit that referenced this pull request Jul 22, 2020
An interesting case that came up in v10.0.0 with the docs-content. The
snapshot release package and docs-content output had various
`@breaking-change` notes in the source code referring to `v10.0.0` as
certain changes are planned to be made at that point.

The snapshot deploy scripts pick up the version from the package.json
file and replace it in the output packages with a more concrete version
that includes the SHA. This meant that we accidentally also overwrote
versions as in the `@breaking-change` notes (ultimately making it
difficult for us to use the latest docs-content for the v10 release as
it was incorrect).

Example snapshot commit:
angular/material2-docs-content@e624c71

We fix this by not including the SHA as part of the deployment, but
rather including the SHA when building the NPM packages. At that point,
we can safely just replace instances of the `0.0.0-PLACEHOLDER` without
having to worry about accidental version overriding.

To achieve this, we update our release stamping script to have two
modes. i.e. snapshot build mode and release mode. Framework does this
by checking the Git tag history but that seems less ideal as it makes
the release output building reliant on external factors while our
stamping is self-contained within the checked out code revision.

(cherry picked from commit 33b31b1)
@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 Aug 22, 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 cla: yes PR author has agreed to Google's Contributor License Agreement P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent 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