[XEN PATCH] automation: Rework "build-each-commit-gcc" test

Anthony PERARD posted 1 patch 2 months, 1 week ago
Failed in applying to current master (apply log)
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(-)
[XEN PATCH] automation: Rework "build-each-commit-gcc" test
Posted by Anthony PERARD 2 months, 1 week ago
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(-)

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=''
+txt_err=''
+txt_clr=''
+
+# $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 "section_$1:$(date +%s):$2
"
+    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
Re: [XEN PATCH] automation: Rework "build-each-commit-gcc" test
Posted by Jan Beulich 2 months ago
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=''
> +txt_err=''
> +txt_clr=''
> +
> +# $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 "section_$1:$(date +%s):$2
> "

... there was either corruption on transmit here, or there's an embedded
newline that I don't know how to deal with.

Jan
Re: [XEN PATCH] automation: Rework "build-each-commit-gcc" test
Posted by Anthony PERARD 2 months ago
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=''
> > +txt_err=''
> > +txt_clr=''
> > +
> > +# $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 "section_$1:$(date +%s):$2
> > "
> 
> ... 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,

-- 
Anthony PERARD
Re: [XEN PATCH] automation: Rework "build-each-commit-gcc" test
Posted by Jan Beulich 2 months ago
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=''
>>> +txt_err=''
>>> +txt_clr=''
>>> +
>>> +# $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 "section_$1:$(date +%s):$2
>>> "
>>
>> ... 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,
>
Re: [XEN PATCH] automation: Rework "build-each-commit-gcc" test
Posted by Stefano Stabellini 2 months ago
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=''
> +txt_err=''
> +txt_clr=''
> +
> +# $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 "section_$1:$(date +%s):$2"
> +    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
>