| Message ID | 20240220140733.40980-1-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN] automation: Rework "build-each-commit-gcc" test | expand |
On Tue, 20 Feb 2024, Anthony PERARD wrote: > Current issues with this test are: > - when the job timeout, the log file is lost as there is no chance to > run the `mv` command. > - GitLab job log is limited in size, so one usually have to download > the artifacts, which may be missing. > > Use $GITLAB_CI to detect when the script is run as part of a GitLab > pipeline. GitLab will add "GITLAB_CI=true" in the environment > variables. > > When run as part of $GITLAB_CI, ignore "dirty" worktree to allow to > write "build-each-commit-gcc.log", which can then be grabbed as > artifacts, even when the job timeout. The `git clean` command is > changed to keep those build logs. > > When run as part of $GITLAB_CI, we will also store the build output in > a log file instead of writing it to stdout, because GitLab's job log > is limited. But we will write the log to stdout in case of error, so > we can find out more quickly why there's been an error. > > This patch also make use of a GitLab feature, "log sections", which we > will collapse by default. One section per commit been built. > > There's a bit of colour added to the logs. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- > > Notes: > Examples: > - on success: > https://gitlab.com/xen-project/people/anthonyper/xen/-/jobs/6212972041 > - on failure: > https://gitlab.com/xen-project/people/anthonyper/xen/-/jobs/6212993231 > > automation/gitlab-ci/build-each-commit.sh | 2 +- > automation/gitlab-ci/test.yaml | 4 +- > automation/scripts/build-test.sh | 55 ++++++++++++++++++++--- > 3 files changed, 50 insertions(+), 11 deletions(-) > > diff --git a/automation/gitlab-ci/build-each-commit.sh b/automation/gitlab-ci/build-each-commit.sh > index 19e337b468..08fddecbbe 100755 > --- a/automation/gitlab-ci/build-each-commit.sh > +++ b/automation/gitlab-ci/build-each-commit.sh > @@ -15,4 +15,4 @@ fi > echo "Building ${BASE}..${TIP}" > > NON_SYMBOLIC_REF=1 ./automation/scripts/build-test.sh ${BASE} ${TIP} \ > - bash -c "git clean -ffdx && ./automation/scripts/build" > + bash -c "git clean -ffdx -e '/build-*.log' && ./automation/scripts/build" > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml > index 3b27cc9f41..50056c1372 100644 > --- a/automation/gitlab-ci/test.yaml > +++ b/automation/gitlab-ci/test.yaml > @@ -136,9 +136,7 @@ build-each-commit-gcc: > XEN_TARGET_ARCH: x86_64 > CC: gcc > script: > - - BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}} TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee ../build-each-commit-gcc.log > - after_script: > - - mv ../build-each-commit-gcc.log . > + - BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}} TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee build-each-commit-gcc.log > artifacts: > paths: > - '*.log' > diff --git a/automation/scripts/build-test.sh b/automation/scripts/build-test.sh > index da643adc01..355d4bf7f6 100755 > --- a/automation/scripts/build-test.sh > +++ b/automation/scripts/build-test.sh > @@ -9,6 +9,37 @@ > # Set NON_SYMBOLIC_REF=1 if you want to use this script in detached HEAD state. > # This is currently used by automated test system. > > +# Colors with ANSI escape sequences > +txt_info='[32m' > +txt_err='[31m' > +txt_clr='[0m' > + > +# $GITLAB_CI should be "true" or "false". > +if [ "$GITLAB_CI" != true ]; then > + GITLAB_CI=false > +fi > + > +gitlab_log_section() { > + if $GITLAB_CI; then > + echo -n "[0Ksection_$1:$(date +%s):$2[0K" > + fi > + if [ $# -ge 3 ]; then > + echo "$3" > + fi > +} > +log_section_last= > +log_section_start() { > + log_section_last="${1%\[collapsed=true\]}" > + gitlab_log_section 'start' "$1" "${txt_info}$2${txt_clr}" > +} > +log_section_end() { > + if [ "$log_section_last" ]; then > + gitlab_log_section 'end' "$log_section_last" > + log_section_last= > + fi > +} > + > + > if test $# -lt 2 ; then > echo "Usage:" > echo " $0 <BASE> <TIP> [CMD]" > @@ -19,10 +50,12 @@ fi > > pushd `git rev-parse --show-toplevel` > > -status=`git status -s` > -if test -n "$status"; then > - echo "Tree is dirty, aborted" > - exit 1 > +if ! $GITLAB_CI; then > + status=`git status -s` > + if test -n "$status"; then > + echo "Tree is dirty, aborted" > + exit 1 > + fi > fi > > BASE=$1; shift > @@ -40,26 +73,34 @@ fi > > ret=1 > while read num rev; do > - echo "Testing $num $rev" > + log_section_start "commit_$rev[collapsed=true]" "Testing #$num $(git log -1 --abbrev=12 --format=tformat:'%h ("%s")' $rev)" > > git checkout $rev > ret=$? > if test $ret -ne 0; then > - echo "Failed to checkout $num $rev with $ret" > + log_section_end > + echo "${txt_err}Failed to checkout $num $rev with $ret${txt_clr}" > break > fi > > if test $# -eq 0 ; then > git clean -fdx && ./configure && make -j4 > + elif $GITLAB_CI; then > + "$@" > "build-$num.log" 2>&1 > else > "$@" > fi > ret=$? > if test $ret -ne 0; then > - echo "Failed at $num $rev with $ret" > + if $GITLAB_CI; then > + cat "build-$num.log" > + fi > + log_section_end > + echo "${txt_err}Failed at $num $rev with $ret${txt_clr}" > break > fi > echo > + log_section_end > done < <(git rev-list $BASE..$TIP | nl -ba | tac) > > echo "Restoring original HEAD" > -- > Anthony PERARD >
On 20.02.2024 15:07, Anthony PERARD wrote: > Current issues with this test are: > - when the job timeout, the log file is lost as there is no chance to > run the `mv` command. > - GitLab job log is limited in size, so one usually have to download > the artifacts, which may be missing. > > Use $GITLAB_CI to detect when the script is run as part of a GitLab > pipeline. GitLab will add "GITLAB_CI=true" in the environment > variables. > > When run as part of $GITLAB_CI, ignore "dirty" worktree to allow to > write "build-each-commit-gcc.log", which can then be grabbed as > artifacts, even when the job timeout. The `git clean` command is > changed to keep those build logs. > > When run as part of $GITLAB_CI, we will also store the build output in > a log file instead of writing it to stdout, because GitLab's job log > is limited. But we will write the log to stdout in case of error, so > we can find out more quickly why there's been an error. > > This patch also make use of a GitLab feature, "log sections", which we > will collapse by default. One section per commit been built. > > There's a bit of colour added to the logs. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> I was trying to commit this, but ... > --- a/automation/scripts/build-test.sh > +++ b/automation/scripts/build-test.sh > @@ -9,6 +9,37 @@ > # Set NON_SYMBOLIC_REF=1 if you want to use this script in detached HEAD state. > # This is currently used by automated test system. > > +# Colors with ANSI escape sequences > +txt_info='[32m' > +txt_err='[31m' > +txt_clr='[0m' > + > +# $GITLAB_CI should be "true" or "false". > +if [ "$GITLAB_CI" != true ]; then > + GITLAB_CI=false > +fi > + > +gitlab_log_section() { > + if $GITLAB_CI; then > + echo -n "[0Ksection_$1:$(date +%s):$2 > [0K" ... there was either corruption on transmit here, or there's an embedded newline that I don't know how to deal with. Jan
On Mon, Feb 26, 2024 at 10:23:37AM +0100, Jan Beulich wrote: > On 20.02.2024 15:07, Anthony PERARD wrote: > > --- a/automation/scripts/build-test.sh > > +++ b/automation/scripts/build-test.sh > > @@ -9,6 +9,37 @@ > > # Set NON_SYMBOLIC_REF=1 if you want to use this script in detached HEAD state. > > # This is currently used by automated test system. > > > > +# Colors with ANSI escape sequences > > +txt_info='[32m' > > +txt_err='[31m' > > +txt_clr='[0m' > > + > > +# $GITLAB_CI should be "true" or "false". > > +if [ "$GITLAB_CI" != true ]; then > > + GITLAB_CI=false > > +fi > > + > > +gitlab_log_section() { > > + if $GITLAB_CI; then > > + echo -n "[0Ksection_$1:$(date +%s):$2 > > [0K" > > ... there was either corruption on transmit here, or there's an embedded > newline that I don't know how to deal with. No corruption here, there is a \r in this string. There's also \e a few times. Most tools can deal with these characters just fine, so I didn't even think there could be an issue. If that byte is really an issue, I could rewrite the patch to use printf, and I think it would read as: printf "\e[0Ksection_$1:$(date +%s):$2\r\e[0K" Thanks,
On 26.02.2024 15:49, Anthony PERARD wrote: > On Mon, Feb 26, 2024 at 10:23:37AM +0100, Jan Beulich wrote: >> On 20.02.2024 15:07, Anthony PERARD wrote: >>> --- a/automation/scripts/build-test.sh >>> +++ b/automation/scripts/build-test.sh >>> @@ -9,6 +9,37 @@ >>> # Set NON_SYMBOLIC_REF=1 if you want to use this script in detached HEAD state. >>> # This is currently used by automated test system. >>> >>> +# Colors with ANSI escape sequences >>> +txt_info='[32m' >>> +txt_err='[31m' >>> +txt_clr='[0m' >>> + >>> +# $GITLAB_CI should be "true" or "false". >>> +if [ "$GITLAB_CI" != true ]; then >>> + GITLAB_CI=false >>> +fi >>> + >>> +gitlab_log_section() { >>> + if $GITLAB_CI; then >>> + echo -n "[0Ksection_$1:$(date +%s):$2 >>> [0K" >> >> ... there was either corruption on transmit here, or there's an embedded >> newline that I don't know how to deal with. > > No corruption here, there is a \r in this string. There's also \e a few > times. Most tools can deal with these characters just fine, so I didn't > even think there could be an issue. Okay, in an entirely different 2nd attempt I got it committed fine (I hope). It's probably too old fashioned of me / my scripts that I demand for every patch to pass a "patch --dry-run" first. Jan > If that byte is really an issue, I could rewrite the patch to use > printf, and I think it would read as: > > printf "\e[0Ksection_$1:$(date +%s):$2\r\e[0K" > > Thanks, >
diff --git a/automation/gitlab-ci/build-each-commit.sh b/automation/gitlab-ci/build-each-commit.sh index 19e337b468..08fddecbbe 100755 --- a/automation/gitlab-ci/build-each-commit.sh +++ b/automation/gitlab-ci/build-each-commit.sh @@ -15,4 +15,4 @@ fi echo "Building ${BASE}..${TIP}" NON_SYMBOLIC_REF=1 ./automation/scripts/build-test.sh ${BASE} ${TIP} \ - bash -c "git clean -ffdx && ./automation/scripts/build" + bash -c "git clean -ffdx -e '/build-*.log' && ./automation/scripts/build" diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml index 3b27cc9f41..50056c1372 100644 --- a/automation/gitlab-ci/test.yaml +++ b/automation/gitlab-ci/test.yaml @@ -136,9 +136,7 @@ build-each-commit-gcc: XEN_TARGET_ARCH: x86_64 CC: gcc script: - - BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}} TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee ../build-each-commit-gcc.log - after_script: - - mv ../build-each-commit-gcc.log . + - BASE=${BASE_SHA:-${CI_COMMIT_BEFORE_SHA}} TIP=${TIP_SHA:-${CI_COMMIT_SHA}} ./automation/gitlab-ci/build-each-commit.sh 2>&1 | tee build-each-commit-gcc.log artifacts: paths: - '*.log' diff --git a/automation/scripts/build-test.sh b/automation/scripts/build-test.sh index da643adc01..355d4bf7f6 100755 --- a/automation/scripts/build-test.sh +++ b/automation/scripts/build-test.sh @@ -9,6 +9,37 @@ # Set NON_SYMBOLIC_REF=1 if you want to use this script in detached HEAD state. # This is currently used by automated test system. +# Colors with ANSI escape sequences +txt_info='[32m' +txt_err='[31m' +txt_clr='[0m' + +# $GITLAB_CI should be "true" or "false". +if [ "$GITLAB_CI" != true ]; then + GITLAB_CI=false +fi + +gitlab_log_section() { + if $GITLAB_CI; then + echo -n "[0Ksection_$1:$(date +%s):$2 [0K" + fi + if [ $# -ge 3 ]; then + echo "$3" + fi +} +log_section_last= +log_section_start() { + log_section_last="${1%\[collapsed=true\]}" + gitlab_log_section 'start' "$1" "${txt_info}$2${txt_clr}" +} +log_section_end() { + if [ "$log_section_last" ]; then + gitlab_log_section 'end' "$log_section_last" + log_section_last= + fi +} + + if test $# -lt 2 ; then echo "Usage:" echo " $0 <BASE> <TIP> [CMD]" @@ -19,10 +50,12 @@ fi pushd `git rev-parse --show-toplevel` -status=`git status -s` -if test -n "$status"; then - echo "Tree is dirty, aborted" - exit 1 +if ! $GITLAB_CI; then + status=`git status -s` + if test -n "$status"; then + echo "Tree is dirty, aborted" + exit 1 + fi fi BASE=$1; shift @@ -40,26 +73,34 @@ fi ret=1 while read num rev; do - echo "Testing $num $rev" + log_section_start "commit_$rev[collapsed=true]" "Testing #$num $(git log -1 --abbrev=12 --format=tformat:'%h ("%s")' $rev)" git checkout $rev ret=$? if test $ret -ne 0; then - echo "Failed to checkout $num $rev with $ret" + log_section_end + echo "${txt_err}Failed to checkout $num $rev with $ret${txt_clr}" break fi if test $# -eq 0 ; then git clean -fdx && ./configure && make -j4 + elif $GITLAB_CI; then + "$@" > "build-$num.log" 2>&1 else "$@" fi ret=$? if test $ret -ne 0; then - echo "Failed at $num $rev with $ret" + if $GITLAB_CI; then + cat "build-$num.log" + fi + log_section_end + echo "${txt_err}Failed at $num $rev with $ret${txt_clr}" break fi echo + log_section_end done < <(git rev-list $BASE..$TIP | nl -ba | tac) echo "Restoring original HEAD"
Current issues with this test are: - when the job timeout, the log file is lost as there is no chance to run the `mv` command. - GitLab job log is limited in size, so one usually have to download the artifacts, which may be missing. Use $GITLAB_CI to detect when the script is run as part of a GitLab pipeline. GitLab will add "GITLAB_CI=true" in the environment variables. When run as part of $GITLAB_CI, ignore "dirty" worktree to allow to write "build-each-commit-gcc.log", which can then be grabbed as artifacts, even when the job timeout. The `git clean` command is changed to keep those build logs. When run as part of $GITLAB_CI, we will also store the build output in a log file instead of writing it to stdout, because GitLab's job log is limited. But we will write the log to stdout in case of error, so we can find out more quickly why there's been an error. This patch also make use of a GitLab feature, "log sections", which we will collapse by default. One section per commit been built. There's a bit of colour added to the logs. Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: Examples: - on success: https://gitlab.com/xen-project/people/anthonyper/xen/-/jobs/6212972041 - on failure: https://gitlab.com/xen-project/people/anthonyper/xen/-/jobs/6212993231 automation/gitlab-ci/build-each-commit.sh | 2 +- automation/gitlab-ci/test.yaml | 4 +- automation/scripts/build-test.sh | 55 ++++++++++++++++++++--- 3 files changed, 50 insertions(+), 11 deletions(-)