Skip to content

[v12] build: Copy release artifacts to release directory#35242

Merged
camscale merged 1 commit intobranch/v12from
camh/v12/release-artifact-dir
Dec 1, 2023
Merged

[v12] build: Copy release artifacts to release directory#35242
camscale merged 1 commit intobranch/v12from
camh/v12/release-artifact-dir

Conversation

@camscale
Copy link
Copy Markdown
Contributor

@camscale camscale commented Dec 1, 2023

Copy the tarball and mac packages/dmgs release artifacts to a release
artifact directory so that the CI scripts do not have to. This makes it
clearer what is and is not a release artifact and puts the logic in the
Makefile instead of the CI yaml, so it can more easily be tested locally
and to make it easier to migrate to the next CI system.

Backport: #25460 (partial)
Companion: https://github.com/gravitational/teleport.e/pull/2804


This is a partial backport - just the first commit - of #25460.This PR
added universal mac builds which was not backported to v12. However it
also added the RELEASE_DIR to the Makefile which is needed for the
GitHub Actions workflow, so I'm backporting just that commit.

I will likely also update the e ref in the PR once the companion PR is merged.

@camscale camscale added the no-changelog Indicates that a PR does not require a changelog entry label Dec 1, 2023
@camscale camscale requested review from fheinecke, r0mant and tcsc December 1, 2023 05:54
@camscale camscale changed the title build: Copy release artifacts to release directory [v12] build: Copy release artifacts to release directory Dec 1, 2023
Copy the tarball and mac packages/dmgs release artifacts to a release
artifact directory so that the CI scripts do not have to. This makes it
clearer what is and is not a release artifact and puts the logic in the
Makefile instead of the CI yaml, so it can more easily be tested locally
and to make it easier to migrate to the next CI system.
@camscale camscale force-pushed the camh/v12/release-artifact-dir branch from 328f4e6 to fa1d345 Compare December 1, 2023 05:56
Comment thread Makefile
yarn install --frozen-lockfile
yarn build-term
yarn package-term -c.extraMetadata.version=$(VERSION)
if [ -n "$$CONNECT_TSH_APP_PATH" ]; then \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to check CONNECT_TSH_APP_PATH before copying the dmg? The comment above release-connect says that this target is used only in macOS, in which case the dmg should always be present after a successful yarn package-term.

Copy link
Copy Markdown
Contributor Author

@camscale camscale Dec 1, 2023

Choose a reason for hiding this comment

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

This is discriminating between CONNECT_TSH_APP_PATH and CONNECT_TSH_BIN_PATH. The former is set when we are building release packages. On a push build we don't care about saving the package, and that uses CONNECT_TSH_BIN_PATH.

https://github.com/gravitational/teleport.e/blob/branch/v12/.github/workflows/build-mac-amd64.yaml#L193 and the step after the highlighted line.

I will shortly be revisiting the whole copying artifact thing when we stop using Drone to do any of this. I may revise this logic then to be more straightforward. But for now I'll keep the branches in line with each other as much as I can.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops, I forgot that it does push builds too, I thought it's just for tag builds.

@camscale camscale added this pull request to the merge queue Dec 1, 2023
Merged via the queue into branch/v12 with commit 782f109 Dec 1, 2023
@camscale camscale deleted the camh/v12/release-artifact-dir branch December 1, 2023 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants