Clean up docker/build.sh #197
Conversation
| else | ||
| export PS4='(${0}:${LINENO}): - [$?] $ ' |
There was a problem hiding this comment.
Turns out that LINENO doesn't work in sh. How about switching to bash?
There was a problem hiding this comment.
We try by default ship sh scripts instead of bash, I don't think the LINENO is imporant enough to switch as this is only for CI and only in a legacy part of CI as this was used for gitlab-ci.
So in this case moving to bash is not worth it IMO, if you find another section where this makes sense I'm fine with (reconsider) moving. But to me the line numbers are not relevant when checking the CI log as this logging was only copied from the main repository but is not relevant for such a short script.
There was a problem hiding this comment.
Switching to bash would also allow hiding the section_start (and section_end) lines from the trace, but it's not a big deal. Currently it looks like this without PS4 (the "▸" lines are collapsed groups):
▶ Run cd docker
+ VERSION=8.2.3
+ NAMESPACE=domjudge
+ URL=https://www.domjudge.org/releases/domjudge-8.2.3.tar.gz
+ FILE=domjudge.tar.gz
+ section_start Download DOMjudge tarball
▶ Download DOMjudge tarball
+ section_start Build domserver container
▶ Build domserver container
+ section_start Build judgehost container (with intermediate image)
▸ Build judgehost container (with intermediate image)
+ section_start Build judgehost container (judging chroot)
▶ Build judgehost container (judging chroot)
+ section_start Push instructions
▶ Push instructions
(Another option is to only enable tracing between section_start and section_end, which would hide the section_start lines even in sh.)
There was a problem hiding this comment.
I removed PS4 (and put the variable assignments in a log group to make things look nicer).
- Move usage check to the top, to serve as documentation for people who read the script. - Remove `-x` from shebang line (#!), to only enable tracing in CI. - Fix `set -x` call (previously it was inside the `if` condition, which is wrong). - Remove PS4 variable (which adds info to the trace) because sh does not support LINENO, and the rest of the info is not that useful. - Inline `trace_off` into section_start and section_end to produce less noise in the trace. (The "section_start" lines will still be displayed though. Hiding them is tricky; see DOMjudge/domjudge/gitlab/integration.sh for an example, but note that it requires bash for `shopt -s expand_aliases`.) - Make the no-op placeholders for section_start and section_end produce less noise in the trace. - Redirect stderr to stdout as a workaround for a GitHub Actions issue that causes them to appear out-of-order. - Simplify initialisation of NAMESPACE variable to make the trace look nicer. - Put the variable assignments in a log group to make it look nicer. - Fix the invocation of build.sh in the build-domjudge-container-* workflows to use `./build.sh` rather than `sh ./build.sh` so that the options in the shebang line will be respected. Also remove unnecessary calls to `set -x` in the workflows.
tom93
force-pushed
the
pr/clean-up-docker-build-script
branch
from
6cd73e4 to
0a45274
Compare
Move usage check to the top, to serve as documentation for people who read the script.
Remove
-xfrom shebang line (#!), to only enable tracing in CI.Fix
set -xcall (previously it was inside theifcondition, which is wrong).Include line numbers etc. in traces (using PS4) on GitHub Actions (previously they were only included on other CI platforms).Update: sh doesn't support $LINENO, so remove PS4 entirely instead.
Inline
trace_offinto section_start and section_end to produce less noise in the trace.Make the no-op placeholders for section_start and section_end produce less noise in the trace.
Redirect stderr to stdout as a workaround for a GitHub Actions issue that causes them to appear out-of-order.
Simplify initialisation of NAMESPACE variable to make the trace look nicer.
Put the variable assignments in a log group to make it look nicer.
Fix the invocation of build.sh in the build-domjudge-container-* workflows to use
./build.shrather thansh ./build.shso that the options in the shebang line will be respected. Also remove unnecessary calls toset -xin the workflows.