Overhaul Action for Documentation publishing

[nephros]

Continuing work from #447:

• Fix some non-fatal errors
• Overhaul build script output
• More verbose bot commit message

[nephros]

Okay this needed some additional tweaking, mainly adding permissions.

But it runs successfully now.

[Olf0]

Cool! As I am still interested in learning more about GH actions, I retraced your latest commit set. I already quickly looked once over the documentation of all three GH pages related actions by GitHub you use here, but missed that the two permissions required for the "deploy-pages" action are mentioned in its documentation. Even more explicitly in the section "security considerations", which is interesting to read in its entirety. The authors (GitHub employees) also suggest to split the workflow into multiple jobs, so these two permissions only have to be granted to the "deploy" job (they can be set outside of a job scope for all jobs or inside one for a specific job) for "better security" (I have no idea how the other actions called may (ab)use these permissions). Note that then one has to be sure that no concurrency between jobs occurs (IIRC the default is "off" anyway) and AFAIU concurrency: "gendoc" already prevents concurrency between multiple runs of this workflow.

[nephros]

Sorry for the noise, some improvements:

• split the jobs as suggested
• add a redirect page to the document root to avoid a 404.
• add option to specify source branch when run manually (might be useful here, it definitely is useful to have in development forks ;) )

This LGTM now, would merge if there are no objections.

[Olf0]

Sorry for the noise, some improvements:

Nice improvements!

^{Side note: I usually do not see the "noise" (i.e., many commits), because I only view the resulting diff to when I viewed a file the last time in GitHub's web-frontend. If unaware of this feature, it may be problematic, because one becomes easily confused to assume to view the diff against the original file (not only the changes since last viewed). IIRC this is triggered by setting the checkbox "Viewed" above each file in a PR. If aware of this, it is a really cool feature, and it just takes a few clicks to really see the diff against the original file, so this is not an issue, either.}

This LGTM now, would merge if there are no objections.

BTW, none of my comments, after I approved this changeset are "objections", they all were "suggestions". Currently I have too little time to come up with well founded objections, hence I do not dare to utter something such, here.

Consequently my "Beautify"-PR #5 against the doc-actions2 branch in your repository is only a suggestion, too.

[Olf0]

@nephros, a bit of nitpicking, as I just "stole" a few enhancements from your action, hence read it again (this time even more thoroughly) and wondered, if setting required: true and default: 'master' at the same time in inputs:SOURCE_BRANCH: makes sense: On first sight, I assume that a strictly required input does not need a default. If you concur, my suggestion is to set required: false instead. I did not check though, if the form for inputting this value in the Actions tab is still displayed when omitting required: true, but assume that it is. These references are not really conclusive:

• https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#onworkflow_callinputs
• https://docs.github.com/en/actions/learn-github-actions/contexts#inputs-context

[nephros]

The thinking was to not permit a user to specify no (SOURCE_BRANCH="") branch.

Of course, the check in https://github.com/sailfishos-patches/patchmanager/blob/master/.github/workflows/gendoc-create-html_qt56.yml#L31 does deal with that case so it would work.

[Olf0]

Oh, now I see that we are discussing two separate points.

1. These [documentation] references are not really conclusive

   I interpreted inputs:SOURCE_BRANCH:default: 'master' that the input (variable; but "variables" address only environment and configuration variables in the context of GitHub Actions) SOURCE_BRANCH is set to master, if it is not explicitly set to something else. At the GUI, when manually starting this workflow, the input field is prefilled accordingly: But I assume when another workflow calls this workflow, still the input SOURCE_BRANCH is set to master, if it is not explicitly set to something else.

   Consequently, unless the input SOURCE_BRANCH is explicitly set to '', it cannot be empty, and will always be set to master unless explicitly set to something else.

   Thus you could directly use inputs.SOURCE_BRANCH throughout your workflow instead checking and copying it to the environment variable DOC_SOURCE_BRANCH in line 31.

2. The thinking was to not permit a user to specify no (SOURCE_BRANCH="") branch.

   Of course, the check in https://github.com/sailfishos-patches/patchmanager/blob/master/.github/workflows/gendoc-create-html_qt56.yml#L31 does deal with that case so it would work.

   AFAICS, the check only ensures, that the SOURCE_BRANCH is set to master, even if the input was deliberately set empty (i.e., ''). IMO one should let the caller (be it a user or another workflow) run into an error when this is explicitly done, which an empty checkout reference in the next step nicely provides. One can think of other checks, as testing if this input value constitutes a valid branch name (though this would require some effort), but all these checks obscure an error made by the caller by silently "fixing" the wrong input. IMO a wrong input shall be accepted as is, as long it does not pose a security risk (which is not the case here AFAICS, because nothing is overwritten, which cannot be easily recreated: only the extant doc output).

In all cases (even when performing input sanitising for security reasons) the caller shall provided with the feedback (s)he/it deserves: "This input was nonsense" or silence, when the input was fine.

PR #455 depicts what I have in mind.

[Olf0]

… and now for something completely different (well, not really, but different locations: line 44 and line 45):

1. What does echo 'debconf debconf/frontend select Noninteractive' | sudo debconf-set-selections; do and why is this needed? I am sure willing to read for myself, but the debconf man-page is not at all conclusive, rather cryptic (well, written by Joey). In contrast to that DebianWiki:debconf is digestable, but does not tell much and The Debconf Programmer's Tutorial is extensive but just as cryptic (also written by Joey). Before I try to tediously dissect this information or wade through Stackoverflow / Stackexchange etc. for long, until I find one or two usable statements, I ask you kindly for a terse, concise statement, maybe with one or two web-links; if your opinion is, "get along with what you found", that is O.K., too. But I have the impression, that debconf is usually used for feeding interactive dialogues with the same replies again and again, which IMO is not necessary when providing the option -y to apt-get install; am I missing something? O.K., this was not too hard to find, e.g.:

   • https://github.com/phusion/baseimage-docker/issues/58#issuecomment-47995343 The whole thread is worth reading.
   • *stackoverflow*: Is it possible to answer dialog questions when installing under docker?** ⇒ DEBIAN_FRONTEND=noninteractive should yield the same and for apt-get it even does not seem to need exporting (to the environment).

   Background: It seems that very similar steps are required when deploying Docker in rootless mode (DIY, not by "Rootless Docker" Action) in order to enable caching.

2. sudo rm -f /etc/apt/sources.list.d/beineri-opt-qt561-trusty-trusty.list removes this sources.list, because the addressed package repo does not exist any longer (or provides wrong packages, but then "how come?")? Most likely irrelevant for my purpose.

[nephros]

Ad 2. Yes that repo is dead, removing it silences some error messages (and speeds up very slightly saving a timeout).

Ad 1. Well you found out. There are several ways to skin a cat here.
In a calssic Dockerfile I would prefer this persistent way over an env var, because seperate RUN lines lead to reusable filesystem fragments and hence speed up docker build invocations.
That does not apply here, but that's where it's coming from.