Fix doc deployment to netlify for releases #38220

kwankyu · 2024-06-14T10:17:03Z

Fix a bug that deployed a PR doc preview to the production site https://sagemath.netlify.app, which is supposed to serve the doc preview for a release.

The reason of the behavior seems that if PR number is missing somehow, the doc is deployed to the production site, as seen here

It started so at least. Now this PR ended in fixing a few things in doc-publish and doc-build-pdf, and overhauling doc-publish logic.

Tests:

on pull request: https://github.com/kwankyu/sage/actions/runs/9635036620
deployed site: https://doc-pr-42--sagemath-test.netlify.app

on push branch develop: https://github.com/kwankyu/sage/actions/runs/9640827756
deployed site: https://doc-develop--sagemath-test.netlify.app

on push tag: https://github.com/kwankyu/sage/actions/runs/9641539263
deployed site: https://doc-10-4-beta29--sagemath-test.netlify.app
deployed site: https://doc-release--sagemath-test.netlify.app

on pull request, build-doc-pdf: https://github.com/kwankyu/sage/actions/runs/9634975196

📝 Checklist

The title is concise and informative.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation and checked the documentation preview.

⌛ Dependencies

github-actions · 2024-06-14T11:20:02Z

Documentation preview for this PR (built with commit b5ec091; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

.ci/create-changes-html.sh

mkoeppe · 2024-06-14T18:17:00Z

In #36730, I have a change that builds the live doc (and only the live doc) on pushed release tags instead of on the "develop" branch. Would you agree with this change?

kwankyu · 2024-06-14T22:15:07Z

No problem. Then this test github.ref == 'refs/heads/develop' should be changed appropriately.

kwankyu · 2024-06-14T22:26:48Z

Perhaps I missed the point of your question. Do you mean that with your change, docs (which is not livedoc) will not be uploaded on pushed release tags?

kwankyu · 2024-06-14T22:49:23Z

So with your change, 'https://preview-release--sagemath.netlify.app' (that is 'https://sagemath.netlify.app') won't exist, only 'https://livedoc--sagemath.netlify.app' will.

I think I prefer to have 'https://preview-release--sagemath.netlify.app', which is better suited for visual comparison with PR doc preview (which is not livedoc)

mkoeppe · 2024-06-20T07:10:03Z

Then let's do this bit, part of the change in #36730?

diff --git a/.github/workflows/doc-build.yml b/.github/workflows/doc-build.yml
index 9dfb8d7d221..57058ac8566 100644
--- a/.github/workflows/doc-build.yml
+++ b/.github/workflows/doc-build.yml
@@ -4,6 +4,14 @@ on:
   pull_request:
   merge_group:
   push:
+    tags:
+      # Match all release tags including beta, rc
+      - '[0-9]+.[0-9]+'
+      - '[0-9]+.[0-9]+.[0-9]+'
+      - '[0-9]+.[0-9]+.beta[0-9]+'
+      - '[0-9]+.[0-9]+.[0-9]+.beta[0-9]+'
+      - '[0-9]+.[0-9]+.rc[0-9]+'
+      - '[0-9]+.[0-9]+.[0-9]+.rc[0-9]+'
     branches:
       - master
       - develop
@@ -114,11 +122,13 @@ jobs:
                      --workdir $(pwd) \
                      ${{ env.BUILD_IMAGE }} /bin/sh
 
-      # Docs
+      #
+      # On PRs and pushes to branches master/develop
+      #
 
       - name: Store old docs
         id: worktree
-        if: (success() || failure()) && steps.container.outcome == 'success'
+        if: (success() || failure()) && steps.container.outcome == 'success' && !startsWith(github.ref, 'refs/tags/')
         run: |
           git config --global --add safe.directory $(pwd)
           git config --global user.email "[email protected]"
@@ -145,7 +155,7 @@ jobs:
 
       - name: Build docs
         id: docbuild
-        if: (success() || failure()) && steps.worktree.outcome == 'success'
+        if: (success() || failure()) && steps.worktree.outcome == 'success' && !startsWith(github.ref, 'refs/tags/')
         # Always non-incremental because of the concern that
         # incremental docbuild may introduce broken links (inter-file references) though build succeeds
         run: |
@@ -182,9 +192,13 @@ jobs:
           name: docs
           path: docs.zip
 
+      #
+      # On release tags: live doc and wheels
+      #
+
       - name: Build live doc
         id: buildlivedoc
-        if: (success() || failure()) && steps.copy.outcome == 'success' && github.repository == 'sagemath/sage' && github.ref == 'refs/heads/develop'
+        if: (success() || failure()) && steps.container.outcome == 'success' && startsWith(github.ref, 'refs/tags/')
         run: |
           export MAKE="make -j5 --output-sync=recurse" SAGE_NUM_THREADS=5
           export PATH="build/bin:$PATH"

kwankyu · 2024-06-20T07:55:27Z

The change of if in Build docs seems not necessary?

If Store old docs step didn't run, Build docs step won't run. Right?

kwankyu · 2024-06-20T08:08:48Z

Not harmful. OK.

mkoeppe · 2024-06-20T21:22:04Z

For the tags, can we now arrange for the live doc to be deposited in versioned URLs?

kwankyu · 2024-06-20T22:40:53Z

For comparison purpose with the docs (non-live) for PRs, the regular (non-live) doc would be better.

regular doc or live doc for tags?

My suggestion: For tags, we may deploy regular doc to release--sagemath.netlify.app together with release-tag--sagemath.netlify.app. And live doc for the latest stable release to sagemath.netlify.app.

mkoeppe · 2024-06-20T22:56:56Z

For comparison purpose with the docs (non-live) for PRs, the regular (non-live) doc would be better.

You mean for preparing the diff, or for something else? The diff is always made against the documentation that is in the container image, and it unrelated to what is deployed to netlify, right?

kwankyu · 2024-06-20T23:09:25Z

For comparison purpose with the docs (non-live) for PRs, the regular (non-live) doc would be better.

You mean for preparing the diff, or for something else? The diff is always made against the documentation that is in the container image, and it unrelated to what is deployed to netlify, right?

Right.

But the "changes.html" page uses the deployed netlify doc site for online comparison with the doc for a PR.

mkoeppe · 2024-06-20T23:20:09Z

OK, but that's not actually the correct version of the documentation to link, right?

The container image is made on pushes to release tags, each of which is also pushed to develop.
So for these also the regular doc is built because it is a push to the develop branch.

kwankyu · 2024-06-20T23:31:44Z

OK, but that's not actually the correct version of the documentation to link, right?

Why not? It is the latest release.

The container image is made on pushes to release tags, each of which is also pushed to develop. So for these also the regular doc is built because it is a push to the develop branch.

Ah, I see.

mkoeppe · 2024-06-20T23:34:42Z

For tags, we may deploy regular doc to release--sagemath.netlify.app together with release-tag--sagemath.netlify.app. And live doc for the latest stable release to sagemath.netlify.app.

In any case I would suggest to include the word "doc" in all of the URLs because nothing else in the URL explains that it is documentation.

On push to the "develop" branch... perhaps deploy to doc-develop--sagemath.netlify.app
On push to tags, deploy to livedoc-TAG--sagemath.netlify.app and additionally to livedoc-develop--sagemath.netlify.app; if a stable tag, additionally deploy to livedoc--sagemath.netlify.app

kwankyu · 2024-06-21T00:11:03Z

In any case I would suggest to include the word "doc" in all of the URLs because nothing else in the URL explains that it is documentation.

Another possibility is to change the domain name from "sagemath.netlify.app" to "sagemath-doc.netlify.app"or "sagedoc.netlify.app"

.github/workflows/doc-build.yml

.github/workflows/doc-publish.yml

kwankyu · 2024-06-24T01:17:20Z

"Deploy Documentation" fails (https://github.com/sagemath/sage/actions/runs/9637964820/job/26577952721) naturally because doc-build.yml and doc-publish.yml do not work in harmony here.

mkoeppe · 2024-06-24T06:12:09Z

doc-build.yml and doc-publish.yml do not work in harmony here.

Just because of the upload/download-artifact versions or other changes too?

kwankyu · 2024-06-24T06:16:47Z

I changed all "docs" to "doc" to be consistent with "livedoc".

"doc-build.yml" coming from this PR uploads "doc.zip" while "doc-publish.yml" coming from the develop branch downloads "docs.zip". So there is a mismatch. See https://github.com/sagemath/sage/actions/runs/9637964820

Of course, "doc-publish.yml" from this PR will work well.

mkoeppe · 2024-06-24T06:25:23Z

a cleaner migration path would be to make doc-publish accept either version

kwankyu · 2024-06-24T06:28:15Z

The friction will just last briefly. Few unfortunate PRs will be affected. Maybe none. I think "cleaner migration path" is unnecessary hassle.

mkoeppe · 2024-06-24T21:46:57Z

OK.

mkoeppe · 2024-06-24T21:47:21Z

Let's merge it.

kwankyu · 2024-06-24T22:52:53Z

Thanks.

Ideally CI should be much more robust than the target software. I expect that with this PR, our doc CI will be robust enough, at least for some time. But there are some warnings that some actions used in our CI will be deprecated soon, which would break CI again...

kwankyu · 2024-06-24T23:23:54Z

Specifically,

netlify/actions/cli@master:

Warning: The `set-output` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

This is minor. Perhaps netlify will fix it.

potiuk/get-workflow-origin@v1_5:

[@octokit/rest] `const Octokit = require("@octokit/rest")` is deprecated. Use `const { Octokit } = require("@octokit/rest")` instead

According to the author, get-workflow-origin is not maintained. So this is more serious.

broken by careless code in sagemath#38213: ``` # Restore the new doc from changes by "wipe out" (cd docs && git checkout -f) ``` which also wipes out anchors planted by `.ci/create-changes-html.sh`! To reviewer: changes over sagemath#38220 is in https://github.com/sagemath/sage/ pull/38274/commits/d2d1514148f46a4ceeea650eab1fd0a0800ebbc9 Tested in : https://github.com/kwankyu/sage/actions/runs/9659812609/job/26643993813. Try to open https://doc-pr-43--sagemath- test.netlify.app/html/en/reference/references/#hunk2 to check ### 📝 Checklist  - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies    sagemath#38220 URL: sagemath#38274 Reported by: Kwankyu Lee Reviewer(s):

as sagemath#38220 allows us to implement the version selector to sage doc. Preview : https://doc-pr-53--sagemath- test.netlify.app/html/en/reference/ The script `sage-update-version`(used by the release manager) updates src/doc/website/versions.txt. The file will be accessible from https://doc-release--sagemath.netlify.app/html/en/versions.txt, after 10.4 release. The Preview is using https://doc-release--sagemath- test.netlify.app/html/en/versions.txt for test purpose. ### 📝 Checklist  - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies    sagemath#38274 URL: sagemath#38285 Reported by: Kwankyu Lee Reviewer(s): Kwankyu Lee, Matthias Köppe

kwankyu changed the title ~~Fix netlify deployment for releases~~ Fix doc deployment to netlify for releases Jun 14, 2024

kwankyu mentioned this pull request Jun 14, 2024

Fix regex to remove spurious diffs in doc preview #38213

Merged

5 tasks

kwankyu marked this pull request as ready for review June 14, 2024 11:34

kwankyu added c: documentation s: needs review labels Jun 14, 2024

github-actions bot added the v: minimal label Jun 14, 2024

kwankyu force-pushed the p/fix-netlify-deployment branch from 8cc5a3f to 21001f6 Compare June 14, 2024 11:43

kwankyu requested a review from mkoeppe June 14, 2024 15:27

mkoeppe reviewed Jun 14, 2024

View reviewed changes

.ci/create-changes-html.sh Outdated Show resolved Hide resolved

kwankyu added s: needs work s: needs review c: scripts and removed s: needs review s: needs work labels Jun 14, 2024

mkoeppe reviewed Jun 23, 2024

View reviewed changes

.github/workflows/doc-build.yml Outdated Show resolved Hide resolved

kwankyu commented Jun 23, 2024

View reviewed changes

.github/workflows/doc-build.yml Outdated Show resolved Hide resolved

Only pushes to develop

b7bd5d0

mkoeppe reviewed Jun 23, 2024

View reviewed changes

.github/workflows/doc-publish.yml Show resolved Hide resolved

mkoeppe reviewed Jun 23, 2024

View reviewed changes

.github/workflows/doc-publish.yml Show resolved Hide resolved

kwankyu added 2 commits June 24, 2024 01:43

Add a comment

ea093f1

Add more comments

a74cc41

kwankyu added the p: critical / 2 label Jun 24, 2024

Fix random error by replacing rm by mv

87161b1

Deploy twice to have a way to refer to doc latest

501573e

mkoeppe added this to the sage-10.4 milestone Jun 24, 2024

mkoeppe added s: positive review and removed s: needs review labels Jun 24, 2024

This was referenced Jun 25, 2024

Fix broken anchors in CHANGES.html of doc preview #38274

Merged

Add version selector to doc #38285

Merged

mkoeppe added p: blocker / 1 and removed p: critical / 2 labels Jun 29, 2024

vbraun merged commit 668da90 into sagemath:develop Jul 4, 2024
24 of 27 checks passed

github-actions bot removed the s: positive review label Jul 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix doc deployment to netlify for releases #38220

Fix doc deployment to netlify for releases #38220

kwankyu commented Jun 14, 2024 •

edited

Loading

github-actions bot commented Jun 14, 2024 •

edited

Loading

mkoeppe commented Jun 14, 2024

kwankyu commented Jun 14, 2024

kwankyu commented Jun 14, 2024 •

edited

Loading

kwankyu commented Jun 14, 2024 •

edited

Loading

mkoeppe commented Jun 20, 2024

kwankyu commented Jun 20, 2024 •

edited

Loading

kwankyu commented Jun 20, 2024 •

edited

Loading

mkoeppe commented Jun 20, 2024

kwankyu commented Jun 20, 2024 •

edited

Loading

mkoeppe commented Jun 20, 2024

kwankyu commented Jun 20, 2024 •

edited

Loading

mkoeppe commented Jun 20, 2024

kwankyu commented Jun 20, 2024

mkoeppe commented Jun 20, 2024

kwankyu commented Jun 21, 2024

kwankyu commented Jun 24, 2024 •

edited

Loading

mkoeppe commented Jun 24, 2024

kwankyu commented Jun 24, 2024 •

edited

Loading

mkoeppe commented Jun 24, 2024

kwankyu commented Jun 24, 2024 •

edited

Loading

mkoeppe commented Jun 24, 2024

mkoeppe commented Jun 24, 2024

kwankyu commented Jun 24, 2024

kwankyu commented Jun 24, 2024

Fix doc deployment to netlify for releases #38220

Fix doc deployment to netlify for releases #38220

Conversation

kwankyu commented Jun 14, 2024 • edited Loading

📝 Checklist

⌛ Dependencies

github-actions bot commented Jun 14, 2024 • edited Loading

mkoeppe commented Jun 14, 2024

kwankyu commented Jun 14, 2024

kwankyu commented Jun 14, 2024 • edited Loading

kwankyu commented Jun 14, 2024 • edited Loading

mkoeppe commented Jun 20, 2024

kwankyu commented Jun 20, 2024 • edited Loading

kwankyu commented Jun 20, 2024 • edited Loading

mkoeppe commented Jun 20, 2024

kwankyu commented Jun 20, 2024 • edited Loading

mkoeppe commented Jun 20, 2024

kwankyu commented Jun 20, 2024 • edited Loading

mkoeppe commented Jun 20, 2024

kwankyu commented Jun 20, 2024

mkoeppe commented Jun 20, 2024

kwankyu commented Jun 21, 2024

kwankyu commented Jun 24, 2024 • edited Loading

mkoeppe commented Jun 24, 2024

kwankyu commented Jun 24, 2024 • edited Loading

mkoeppe commented Jun 24, 2024

kwankyu commented Jun 24, 2024 • edited Loading

mkoeppe commented Jun 24, 2024

mkoeppe commented Jun 24, 2024

kwankyu commented Jun 24, 2024

kwankyu commented Jun 24, 2024

kwankyu commented Jun 14, 2024 •

edited

Loading

github-actions bot commented Jun 14, 2024 •

edited

Loading

kwankyu commented Jun 14, 2024 •

edited

Loading

kwankyu commented Jun 14, 2024 •

edited

Loading

kwankyu commented Jun 20, 2024 •

edited

Loading

kwankyu commented Jun 20, 2024 •

edited

Loading

kwankyu commented Jun 20, 2024 •

edited

Loading

kwankyu commented Jun 20, 2024 •

edited

Loading

kwankyu commented Jun 24, 2024 •

edited

Loading

kwankyu commented Jun 24, 2024 •

edited

Loading

kwankyu commented Jun 24, 2024 •

edited

Loading