Automate SDK release from CI #2457

LaPeste · 2021-06-21T17:04:41Z

Description

This PR adds a workflow that automates the whole release process of the .NET SDK from Github Actions.

Fixes #2051

TODO

stop generating docs on every run

…workflow

.github/templates/main.yml

LaPeste · 2021-07-05T16:47:51Z

.github/workflows/main.yml

+      with:
+        name: Docs.zip
+        path: ${{ github.workspace }}/Realm/packages/Docs.zip
+        retention-days: ${{ github.event_name != 'pull_request' && 30 || 1 }}


I'm not sure the amount of days we'd like to retain the docs. So I just put the standard length we always use. Please, advise if 30 days are too many.

LaPeste · 2021-07-05T17:16:02Z

.github/workflows/release.yml

+        echo "RELEASE_VERSION=$version" | Out-File $env:GITHUB_ENV -Encoding utf8
+    - name: Upload to nuget
+      run: |
+        dotnet nuget push "Realm/packages/Realm.Fody.$env:RELEASE_VERSION/Realm.Fody.$env:RELEASE_VERSION.nupkg" --skip-duplicate --api-key ${{ secrets.NUGET_API_KEY }} --source https://api.nuget.org/v3/index.json


This folder within packages is because I decided to download all artifacts. When doing so, by specifications, a folder per artifact is created and each artifact is appropriately placed.

nirinchev · 2021-07-06T09:57:18Z

.github/workflows/release.yml

+    - name: Extract release name
+      run: |
+        $nupkgName = ls Realm/packages/Realm.Fody.$env:RELEASE_VERSION | select -expandproperty Name | Select-String Fody
+        $nupkgName -match "(?sm)\d{1,2}\.\d{1,2}\.\d{1,2}"


Will that regex (and the one above) match beta versions?

Good point. I'm gonna fix that.

nirinchev · 2021-07-06T09:57:57Z

.github/workflows/release.yml

+        $nupkgName = ls Realm/packages/Realm.Fody.$env:RELEASE_VERSION | select -expandproperty Name | Select-String Fody
+        $nupkgName -match "(?sm)\d{1,2}\.\d{1,2}\.\d{1,2}"
+        $version = $Matches[0]
+        echo "RELEASE_VERSION=$version" | Out-File $env:GITHUB_ENV -Encoding utf8


Rather than use env variables here, I think we should use set-output

I can do that but, what's the advantage? As a con it has longer syntax, as a pro maybe it's easier to understand where it's set (although a search in the editor achieves the same). What other pros do you see?

I generally dislike env variables as a way of passing data between steps. The step outputs are designed precisely for that and we should prefer to use them.

LaPeste · 2021-07-06T16:01:36Z

There's a thing I've been debating about: checking for error/warning in the executed commands. Generally we don't need to do any check on shell command return values, since when a command fails then the step automatically fails, too, consequently making the whole workflow fail, unless instructed otherwise. But there's a problem, if the command doesn't really fail but returns warnings or for some strange reason it "fails" but not enough to report a non-zero return value then it'll be hard to notice.
An example at hand is docfx, it doesn't fail but it reports tons of warnings which one can't see until the logs are checked. The result is a generated documentation that is half broken.
This is a bad example because I manually set it to be fine with duplicates, but for the sake of the conversion it's ok: Another example could be in nuget, when uploading a package that's already there and the option for skipping duplicate is set then a warning is given but GH Actions doesn't understand it and it goes buried in the logs.

So, the conclusion is: just sometimes GH runners don't understand warnings or errors aren't really non-zero return values (althogh I have no example for this second case). What should we do about this?

We could check on each single return code of each command (bad idea in my opinion as it'll make the code unreadable)
We could parse the output of each command in a custom function looking for a few keywords: E/e-rror-s, W/warning-s etc and release a general warning from the step so that it appears in the summary of the run workflow (maybe the best solution)
Do nothing and manually check the logs and results of each step we know it's delicate, e.g. so far it's been only a problem for the release workflow to my knowledge) (I suppose this is a still an acceptable solution).

What's your opinion on the matter?

* master: Update the AppConfiguration.Logger obsolete message (#2495) Add more serializer types to the preserved ones (#2489) Update changelog from Core (#2488) Build an xcframework for iOS (#2475) Prepare for vNext (#2487) Prepare for 10.2.1 (#2484) Fix TableKey / ObjKey conversion on Android x86 (#2477) Update the build manager locations (#2474) Clean up native resources on Unity application exit (#2467) Fix some native warnings (#2483) Don't try to get the app from a removed user (#2480) Use delete_files from core when deleting a Realm. (#2401) Fixes for the sync datatype tests (#2461) Clean up the package.json and add some docs (#2452) Run iOS tests on CI (#2405) Verify that objects do not belong to different realm when added to collection (#2465) Updated README instructions (#2459)

nirinchev

Added 2 minor comments and some explanation for the changes I made to get the generated docs to resolve references correctly.

nirinchev · 2021-07-06T23:46:06Z

.github/templates/Readme.md

 1. Download the executable for your platform from the [releases page](https://github.com/vmware-tanzu/carvel-ytt/releases).
 1. Rename it to `ytt`.
 1. Place it in your PATH.

 ## Building the docs
-1. In `$SolutionDir/.github/templates` run `ytt -f main.yml --output-files ../workflows/`.
+1. `cd $SolutionDir/.github/templates`
+1. `ytt -f . --output-files ../workflows/` ==> to target all files<br/>


There's no need for <br/> or is there?

In markdown we can insert a line break in 2 ways:

Specifically adding a line break tag (like a did)

Ending a line with 2 or more spaces and a new line char

I thought that in the option number 2 it would be harder to notice the new line. But maybe it's nicer to read? Which approach do you recommend?

.github/templates/main.yml

nirinchev · 2021-07-06T23:49:35Z

.github/templates/main.yml

  - name: Checkout code
    uses: actions/checkout@v2
    with:
-      submodules: recursive
+      submodules: #@ submodules
+      ref: ${{ github.event.pull_request.head.sha }}


This changes the behavior of checkout@v2 to checkout the HEAD rather than the merge commit - this is useful in regular PRs, but especially important for releases, where we don't want to build the merged thing.

I want to double check that I got this right:
Our main workflow is triggered with push on master/main and on pull_request (with events opened, synchronize, or reopened). Since ref defaults to the reference or SHA of the event that triggered the workflow, you're saying that we need to avoid to fetch a push to master, which generally happens to be a merge of a pull request, but we always need to carefully fetch the head of the current PR which could be lacking the latest merged PRs that master has, which is a good thing in this case.
Did I get this right?

The default behavior for checkout on a PR trigger is to checkout the merge commit (i.e. have the PR branch merged into the base branch and use that). This is why when there are merge conflicts, our GHA workflows don't run. This is the change suggested in the checkout action docs to make sure that PR builds actually build and test the code that is in the PR itself.

While for PR builds against master, checking out the merge commit is not a big deal - and in some cases may even be useful - for release PRs it's plain wrong and may result in very surprising behavior where we actually push a nuget package containing code that was not present in the release branch.

Now it all makes sense. I wasn't fully aware of the default behaviour for the checkout on PRs, that's why I got confused. Thank you for the explanation. On top of this, the change of line 74 will stop "there are merge conflicts with master" that prevent the workflow to run; which I'm actually rather happy about.

nirinchev · 2021-07-06T23:53:43Z

.github/templates/main.yml

+  - name: Check Docfx cache
+    if: #@ docsCondition
+    id: check-docfx-cache
+    uses: actions/cache@v2
+    with:
+      path: 'C:\docfx'
+      key: docfx
+  - name: Download docfx
+    if: #@ docsCondition + " && steps.check-docfx-cache.outputs.cache-hit != 'true'"
+    run: |
+      Invoke-WebRequest -Uri https://github.com/dotnet/docfx/releases/download/v2.58/docfx.zip -OutFile C:\docfx.zip
+      Expand-Archive -Path C:\docfx.zip -DestinationPath C:\docfx
+    shell: powershell


Rather than use choco, we're caching docfx to avoid downloading it every time. This seems to cut docs build times from 70 to 35 seconds.

nirinchev · 2021-07-06T23:55:31Z

.github/templates/main.yml

      - #@ template.replace(fetchWrapperBinaries())
      - #@ template.replace(buildPackages())
      - #@ findPackageVersion()
+      - #@ template.replace(buildDocs())


This was the important change that fixed the docs issues - moving it to after the package was built allows docfx to pick up the correct references.

🤦🏻.... makes sense

nirinchev · 2021-07-07T00:04:46Z

.github/workflows/release.yml

+        $nupkgName = ls Realm/packages/Realm.Fody.$env:RELEASE_VERSION | select -expandproperty Name | Select-String Fody
+        $nupkgName -match "(?sm)\d{1,2}\.\d{1,2}\.\d{1,2}"
+        $version = $Matches[0]
+        echo "RELEASE_VERSION=$version" | Out-File $env:GITHUB_ENV -Encoding utf8


I generally dislike env variables as a way of passing data between steps. The step outputs are designed precisely for that and we should prefer to use them.

LaPeste and others added 20 commits June 15, 2021 16:54

Added first version on the publish workflow

dbf7400

Move to work from windows

e6b55cc

Merge branch 'master' into ac/ci-publish-nupkg

e21f709

Some more work done

e186ef7

Added apexes around nupkg

da17089

First "close to final" release workflow

e936664

Rename AWS secrets and cleaned templates/

4bf8118

Cleaned-up PR and version from nupkg when on release branch

33e6b21

Corrected env vars names

d7825bd

Generate docs in release branch from main and upload them in release …

38c1e5d

…workflow

Fix for contains in main.yml

0d0010e

Test docs

9848ff9

Test docs - 0

9a9cd6e

Test docs - 1

3571f63

Test docs - 2

a3fc1b6

Test docs - 3

ea2214b

Test docs - 4

3598d87

Docs creation works

0919697

Docs creation works - 0

652fb51

Minor fixes before review

b296c37

LaPeste commented Jul 5, 2021

View reviewed changes

.github/templates/main.yml Show resolved Hide resolved

LaPeste commented Jul 5, 2021

View reviewed changes

Fix some errors in release workflow

8ce7465

LaPeste commented Jul 5, 2021

View reviewed changes

nirinchev reviewed Jul 6, 2021

View reviewed changes

LaPeste added 2 commits July 6, 2021 17:24

Applied fixes from things learned from the test repo

e1090d8

Added negation to wrong file

8a02492

LaPeste marked this pull request as ready for review July 6, 2021 15:42

nirinchev added 7 commits July 7, 2021 02:15

Build docs immediately after a package has been built

9a2f219

Build PR HEAD rather than the merge commit

491df05

Try to cache docfx

d80346b

Don't always checkout recursive submodules

6e306c7

Clean up some logging

b327cc7

Try #2

8c0a3fd

Try #3

36b5358

nirinchev reviewed Jul 7, 2021

View reviewed changes

nirinchev and others added 7 commits July 7, 2021 17:46

Update regex and comment-out nuget publishing for now

17b42c3

Removed <br/> from readme

a3d9617

Changed from env vars to outputs

6c812b7

Added apexes to avoid problems in interpretation of string

d4e1607

Clean up some step names

357b341

Remove accidental space

816fc7b

Inverse doc condition

fb552e5

nirinchev merged commit bd60f17 into master Jul 7, 2021

nirinchev deleted the ac/ci-publish-nupkg branch July 7, 2021 17:35

LaPeste mentioned this pull request Aug 16, 2021

Generate and publish API docs on release #2054

Closed

github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automate SDK release from CI #2457

Automate SDK release from CI #2457

LaPeste commented Jun 21, 2021 •

edited by nirinchev

Loading

LaPeste Jul 5, 2021 •

edited

Loading

LaPeste Jul 5, 2021

nirinchev Jul 6, 2021

LaPeste Jul 6, 2021

nirinchev Jul 6, 2021

LaPeste Jul 6, 2021 •

edited

Loading

nirinchev Jul 7, 2021

LaPeste commented Jul 6, 2021 •

edited

Loading

nirinchev left a comment

nirinchev Jul 6, 2021

LaPeste Jul 7, 2021 •

edited

Loading

nirinchev Jul 6, 2021

LaPeste Jul 7, 2021 •

edited

Loading

nirinchev Jul 7, 2021

LaPeste Jul 8, 2021 •

edited

Loading

nirinchev Jul 6, 2021

nirinchev Jul 6, 2021

LaPeste Jul 7, 2021 •

edited

Loading

nirinchev Jul 7, 2021

Automate SDK release from CI #2457

Automate SDK release from CI #2457

Conversation

LaPeste commented Jun 21, 2021 • edited by nirinchev Loading

Description

TODO

LaPeste Jul 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LaPeste Jul 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LaPeste commented Jul 6, 2021 • edited Loading

nirinchev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LaPeste Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LaPeste Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LaPeste Jul 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LaPeste Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LaPeste commented Jun 21, 2021 •

edited by nirinchev

Loading

LaPeste Jul 5, 2021 •

edited

Loading

LaPeste Jul 6, 2021 •

edited

Loading

LaPeste commented Jul 6, 2021 •

edited

Loading

LaPeste Jul 7, 2021 •

edited

Loading

LaPeste Jul 7, 2021 •

edited

Loading

LaPeste Jul 8, 2021 •

edited

Loading

LaPeste Jul 7, 2021 •

edited

Loading