This is an archive of the discontinued LLVM Phabricator instance.

workflows: Add GitHub action for automating some release tasks
ClosedPublic

Authored by tstellar on Apr 1 2021, 5:30 PM.

Details

Summary

For each release tag, this action will create a new release on GitHub,
and for each -final tag, this action will build the documentation and
upload it to GitHub.

Diff Detail

Event Timeline

tstellar requested review of this revision.Apr 1 2021, 5:30 PM
tstellar created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2021, 5:30 PM
hans accepted this revision.Apr 7 2021, 1:13 AM

I'm not familiar with github actions so I'm just skimming the code, but it looks good as far as I can tell.

Any reason not to upload the docs also for release candidates? I think it's useful for people to be able to see the current state of release notes (usually to suggest they need improvement).

This revision is now accepted and ready to land.Apr 7 2021, 1:13 AM

I'm not familiar with github actions so I'm just skimming the code, but it looks good as far as I can tell.

Any reason not to upload the docs also for release candidates? I think it's useful for people to be able to see the current state of release notes (usually to suggest they need improvement).

No reason not to. I was just trying to match the old behavior, but I can easily make that change.

tstellar added a reviewer: kwk.Apr 8 2021, 2:08 PM
kwk added inline comments.Apr 9 2021, 12:09 PM
.github/workflows/release-tasks.yml
19

Do we have a way to ensure sane tagging? If not, then a tag like llvmorg-13.0.....1 would match the pattern as well. Do we want to make the versioning scheme more descriptive here? For example, do we always want MAJOR.MINOR.PATCH and optional -RC<0-9>? Then I suggest to change the grep expression to grep -e '^refs/tags/llvmorg-[0-9]\+\.[0-9]\+\.[0-9]\(-rc[0-9]\+\)\?$'. With explicit begin ^ and end $ this will actually result in non-matching tags to be discarded.

25

Shouldn't this be sudo apt-get install -y to run in non-interactive mode?

31

This line can be omitted because 1 is the default: https://github.com/actions/checkout#usage.

35

Remove ./

39

I don't see this script in https://github.com/llvm/llvm-project/tree/main/llvm/utils/release nor in this change. What am I missing?

tstellar updated this revision to Diff 443415.Jul 8 2022, 11:29 PM
tstellar marked 4 inline comments as done.

Adress review comments.

Herald added a project: Restricted Project. · View Herald TranscriptJul 8 2022, 11:29 PM
tstellar added inline comments.Jul 15 2022, 10:12 AM
.github/workflows/release-tasks.yml
39

This script has been committed to the tree.

kwk accepted this revision.Jul 26 2022, 12:04 AM

I don't see serious problems with this change. And my comments are just improvements. Good Work!

.github/workflows/release-tasks.yml
24

This is just a side question: Don't you have to apt-get update when in a fresh Ubuntu container? At least that is the case when starting it with podman or docker. Is that already done when you're running this with Github?

When I do this locally I get:

Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
E: Unable to locate package doxygen
E: Unable to locate package graphviz
E: Unable to locate package python3-github
E: Unable to locate package python3-recommonmark
E: Unable to locate package python3-sphinx
E: Unable to locate package ninja-build
E: Unable to locate package texlive-font-utils
29

I've looked at the build-docs.sh script and it lists the sphinx-common package for Ubuntu installations as a requirement:

apt-get install doxygen sphinx-common python3-recommonmark \
             ninja-build graphviz texlive-font-utils

pip3 install --user sphinx-markdown-tables

Is this installed implicitly by any of the other packages that you install here or is it resolved when running the pip3 command?

UPDATE: When I check with the packages that you've installed I can confirm that the dependency *is* installed:

# dpkg -s sphinx-common
Package: sphinx-common
Status: install ok installed
Priority: optional
Section: python
Installed-Size: 3110
Maintainer: Debian Python Team <team+python@tracker.debian.org>
Architecture: all
Multi-Arch: foreign
Source: sphinx
Version: 4.3.2-1
Replaces: python-sphinx (<< 1.1)
Provides: dh-sequence-sphinxdoc
Depends: libjs-sphinxdoc (= 4.3.2-1), perl:any
Recommends: python3-sphinx
Conflicts: python-sphinx (<< 1.1)
Description: documentation generator for Python projects - common data
 Sphinx is a tool for producing documentation for Python projects, using
 reStructuredText as markup language.
 .
 This package includes manual pages, templates, translations and other data
 files.
Homepage: https://www.sphinx-doc.org/

I have looked at this locally in a podman container and made a strange observation:

Setting up x11-common (1:7.7+23ubuntu2) ...
debconf: unable to initialize frontend: Dialog
debconf: (No usable dialog-like program is installed, so the dialog based frontend cannot be used. at /usr/share/perl5/Debconf/FrontEnd/Dialog.pm line 78.)
debconf: falling back to frontend: Readline

[... OTHER PACKAGES ...]

Setting up tzdata (2022a-0ubuntu1) ...
debconf: unable to initialize frontend: Dialog
debconf: (No usable dialog-like program is installed, so the dialog based frontend cannot be used. at /usr/share/perl5/Debconf/FrontEnd/Dialog.pm line 78.)
debconf: falling back to frontend: Readline
Configuring tzdata
------------------

Please select the geographic area in which you live. Subsequent configuration questions will narrow this down by presenting a list of cities, representing the time zones in which they are
located.

  1. Africa  2. America  3. Antarctica  4. Australia  5. Arctic Ocean  6. Asia  7. Atlantic Ocean  8. Europe  9. Indian Ocean  10. Pacific Ocean  11. US  12. None of the above
Geographic area:

The CLI asked for feedback even though I gave apt the assume-yes-switch. Let's just hope this doesn't happen in Github with the Ubuntu containers.


Sorry for the noise, I'm just documenting this to confirm a thorough review.

43

Why leave it up to the caller to decide about the interpreter? Shouldn't the script tell what to use with its shebang line? Right now the shebang line in build-docs.sh is #!/bin/sh. I suggest to change this to #!/bin/bash and get rid of the bash here.

tstellar added inline comments.Jul 26 2022, 12:11 AM
.github/workflows/release-tasks.yml
24

I'm not sure that the runners are using containers, they may be using virtual machines. Either way, there have been a lot of packages pre-installed, so apt-get update must have been run at some point.

29

You have to set DEBIAN_FRONTEND=noninteractive to skip these kind of questions (even with -y). The github runners do this: https://github.com/actions/virtual-environments/pull/366

tstellar updated this revision to Diff 447590.Jul 26 2022, 12:18 AM

Fix invocation of build-docs.sh and prevent this action from running on forks.

This revision was landed with ongoing or failed builds.Jul 26 2022, 3:43 PM
This revision was automatically updated to reflect the committed changes.