[v2] Cirrus-CI improvements, and other CI-related fixes, m68k

[PULL v2 00/15] Cirrus-CI improvements, and other CI-related fixes, m68k

Posted by Thomas Huth 5 years, 2 months ago

 Hi Peter,

the following changes since commit 887adde81d1f1f3897f1688d37ec6851b4fdad86:

  Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2020-09-01 22:50:23 +0100)

are available in the Git repository at:

  https://gitlab.com/huth/qemu.git tags/pull-request-2020-09-02

for you to fetch changes up to 5e560e07ca396e16150740ae3c46b35a85f59ba7:

  gitlab-ci.yml: Set artifacts expiration time (2020-09-02 16:23:55 +0200)

----------------------------------------------------------------
* Cirrus-CI improvements and fixes (compile with -Werror & fix for 1h problem)
* Two build system fixes to fix some failures the CI
* One m68k QOMification patch
* Some trivial qtest patches
* Some small improvements for the Gitlab CI
----------------------------------------------------------------

Gerd Hoffmann (1):
      meson: fix keymaps without qemu-keymap

Markus Armbruster (1):
      libqtest: Rename qmp_assert_error_class() to qmp_expect_error_and_unref()

Philippe Mathieu-Daudé (3):
      tests/qtest/ahci: Improve error handling (NEGATIVE_RETURNS)
      tests/qtest/tpm: Declare input buffers const and static
      tests/qtest/ipmi-kcs: Fix assert side-effect

Thomas Huth (10):
      configure: Fix atomic64 test for --enable-werror on macOS
      cirrus.yml: Compile FreeBSD with -Werror
      cirrus.yml: Compile macOS with -Werror
      cirrus.yml: Update the macOS jobs to Catalina
      cirrus.yml: Split FreeBSD job into two parts
      configure: Add system = 'linux' for meson when cross-compiling
      hw/m68k: QOMify the mcf5206 system integration module
      gitlab/travis: Rework the disabled features tests
      gitlab-ci.yml: Run check-qtest and check-unit at the end of the fuzzer job
      gitlab-ci.yml: Set artifacts expiration time

 .cirrus.yml                   | 43 ++++++++++++++++++++++++++++++++----------
 .gitlab-ci.yml                | 36 ++++++++++++++++++++++++++---------
 .travis.yml                   |  6 ------
 configure                     | 13 ++++++++-----
 hw/m68k/an5206.c              | 14 ++++++++++++--
 hw/m68k/mcf5206.c             | 44 ++++++++++++++++++++++++++++++++++---------
 include/hw/m68k/mcf.h         |  3 +--
 pc-bios/keymaps/meson.build   | 28 ++++++++++++++++++---------
 tests/qtest/ahci-test.c       |  1 +
 tests/qtest/drive_del-test.c  |  2 +-
 tests/qtest/ipmi-kcs-test.c   |  3 ++-
 tests/qtest/libqos/libqtest.h |  4 ++--
 tests/qtest/libqtest.c        |  2 +-
 tests/qtest/qmp-cmd-test.c    | 16 ++++++++--------
 tests/qtest/qmp-test.c        | 32 +++++++++++++++----------------
 tests/qtest/tpm-tests.c       |  4 ++--
 tests/qtest/tpm-util.c        | 10 +++++-----
 tests/test-qga.c              |  2 +-
 18 files changed, 174 insertions(+), 89 deletions(-)

Re: [PULL v2 00/15] Cirrus-CI improvements, and other CI-related fixes, m68k

Posted by Peter Maydell 5 years, 2 months ago

On Wed, 2 Sep 2020 at 16:49, Thomas Huth <thuth@redhat.com> wrote:
>
>  Hi Peter,
>
> the following changes since commit 887adde81d1f1f3897f1688d37ec6851b4fdad86:
>
>   Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2020-09-01 22:50:23 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/huth/qemu.git tags/pull-request-2020-09-02
>
> for you to fetch changes up to 5e560e07ca396e16150740ae3c46b35a85f59ba7:
>
>   gitlab-ci.yml: Set artifacts expiration time (2020-09-02 16:23:55 +0200)
>
> ----------------------------------------------------------------
> * Cirrus-CI improvements and fixes (compile with -Werror & fix for 1h problem)
> * Two build system fixes to fix some failures the CI
> * One m68k QOMification patch
> * Some trivial qtest patches
> * Some small improvements for the Gitlab CI
> ----------------------------------------------------------------

Failures on windows crossbuild and OSX:

Generating qemu-version.h with a meson_exe.py custom command
Generating ar with a custom command
cp: '../../pc-bios/keymaps/ar' and 'pc-bios/keymaps/ar' are the same file
Generating bepo with a custom command
cp: '../../pc-bios/keymaps/bepo' and 'pc-bios/keymaps/bepo' are the same file
Makefile.ninja:5177: recipe for target 'pc-bios/keymaps/ar.stamp' failed
make: *** [pc-bios/keymaps/ar.stamp] Error 1
make: *** Waiting for unfinished jobs....
Makefile.ninja:5179: recipe for target 'pc-bios/keymaps/bepo.stamp' failed
make: *** [pc-bios/keymaps/bepo.stamp] Error 1
Generating cz with a custom command
cp: '../../pc-bios/keymaps/cz' and 'pc-bios/keymaps/cz' are the same file
Makefile.ninja:5181: recipe for target 'pc-bios/keymaps/cz.stamp' failed
make: *** [pc-bios/keymaps/cz.stamp] Error 1
make: Leaving directory '/home/petmay01/qemu-for-merges/build/w64'

On x86 linux a lot of this kind of warning from meson:

../../pc-bios/keymaps/meson.build:58: WARNING: Custom target input
'ar' can't be converted to File object(s).
This will become a hard error in the future.
../../pc-bios/keymaps/meson.build:58: WARNING: Custom target input
'bepo' can't be converted to File object(s).
This will become a hard error in the future.
../../pc-bios/keymaps/meson.build:58: WARNING: Custom target input
'cz' can't be converted to File object(s).
This will become a hard error in the future.

and then a failure later on:

make[1]: Nothing to be done for 'all'.
make: *** No rule to make target '../../pc-bios/keymaps/ar', needed by
'pc-bios/keymaps/ar.stamp'. Stop.
make: *** Waiting for unfinished jobs....

This is the same set of errors I got from when Gerd put
the "meson: fix keymaps witout qemu-keymap" patch in his UI pullreq:
  https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg00149.html
so that seems like the problem.

thanks
-- PMM

Re: [PULL v2 00/15] Cirrus-CI improvements, and other CI-related fixes, m68k

Posted by Thomas Huth 5 years, 2 months ago

On 02/09/2020 22.19, Peter Maydell wrote:
> On Wed, 2 Sep 2020 at 16:49, Thomas Huth <thuth@redhat.com> wrote:
>>
>>  Hi Peter,
>>
>> the following changes since commit 887adde81d1f1f3897f1688d37ec6851b4fdad86:
>>
>>   Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' into staging (2020-09-01 22:50:23 +0100)
>>
>> are available in the Git repository at:
>>
>>   https://gitlab.com/huth/qemu.git tags/pull-request-2020-09-02
>>
>> for you to fetch changes up to 5e560e07ca396e16150740ae3c46b35a85f59ba7:
>>
>>   gitlab-ci.yml: Set artifacts expiration time (2020-09-02 16:23:55 +0200)
>>
>> ----------------------------------------------------------------
>> * Cirrus-CI improvements and fixes (compile with -Werror & fix for 1h problem)
>> * Two build system fixes to fix some failures the CI
>> * One m68k QOMification patch
>> * Some trivial qtest patches
>> * Some small improvements for the Gitlab CI
>> ----------------------------------------------------------------
> 
> Failures on windows crossbuild and OSX:
> 
> Generating qemu-version.h with a meson_exe.py custom command
> Generating ar with a custom command
> cp: '../../pc-bios/keymaps/ar' and 'pc-bios/keymaps/ar' are the same file
> Generating bepo with a custom command
> cp: '../../pc-bios/keymaps/bepo' and 'pc-bios/keymaps/bepo' are the same file
> Makefile.ninja:5177: recipe for target 'pc-bios/keymaps/ar.stamp' failed
[...]
> This is the same set of errors I got from when Gerd put
> the "meson: fix keymaps witout qemu-keymap" patch in his UI pullreq:
>   https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg00149.html
> so that seems like the problem.

Darn. I've added Gerd's patch since it is needed to fix the acceptance
tests in the Gitlab-CI. Could we maybe revert the patch that introduced
the regression instead, as long as no other proper fix is available? The
failing CI is really bugging me.

 Thomas

Re: [PULL v2 00/15] Cirrus-CI improvements, and other CI-related fixes, m68k

Posted by Gerd Hoffmann 5 years, 2 months ago

  Hi,

> Darn. I've added Gerd's patch since it is needed to fix the acceptance
> tests in the Gitlab-CI. Could we maybe revert the patch that introduced
> the regression instead, as long as no other proper fix is available? The
> failing CI is really bugging me.

Well, ddcf607fa3d6 fixes another nasty issue (builds modifying the
source tree), so reverting that isn't really an option.

We could have configure remove the symlink in case is present.  That way
old build trees with the symlink already created should work too.  Right
now only build trees created with ddcf607fa3d6 present are working
properly.

Untested patch below.

take care,
  Gerd

diff --git a/configure b/configure
index b1e11397a827..493b4e86da62 100755
--- a/configure
+++ b/configure
@@ -8107,6 +8107,7 @@ LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
 LINKS="$LINKS tests/acceptance tests/data"
 LINKS="$LINKS tests/qemu-iotests/check"
 LINKS="$LINKS python"
+UNLINK="pc-bios/keymaps"
 for bios_file in \
     $source_path/pc-bios/*.bin \
     $source_path/pc-bios/*.elf \
@@ -8127,6 +8128,11 @@ for f in $LINKS ; do
         symlink "$source_path/$f" "$f"
     fi
 done
+for f in $UNLINK ; do
+    if [ -L "$f" ]; then
+        rm -f "$f"
+    fi
+done
 
 (for i in $cross_cc_vars; do
   export $i

Re: [PULL v2 00/15] Cirrus-CI improvements, and other CI-related fixes, m68k

Posted by Peter Maydell 5 years, 2 months ago

On Thu, 3 Sep 2020 at 06:47, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > Darn. I've added Gerd's patch since it is needed to fix the acceptance
> > tests in the Gitlab-CI. Could we maybe revert the patch that introduced
> > the regression instead, as long as no other proper fix is available? The
> > failing CI is really bugging me.
>
> Well, ddcf607fa3d6 fixes another nasty issue (builds modifying the
> source tree), so reverting that isn't really an option.
>
> We could have configure remove the symlink in case is present.  That way
> old build trees with the symlink already created should work too.  Right
> now only build trees created with ddcf607fa3d6 present are working
> properly.
>
> Untested patch below.
>
> take care,
>   Gerd
>
> diff --git a/configure b/configure
> index b1e11397a827..493b4e86da62 100755
> --- a/configure
> +++ b/configure
> @@ -8107,6 +8107,7 @@ LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
>  LINKS="$LINKS tests/acceptance tests/data"
>  LINKS="$LINKS tests/qemu-iotests/check"
>  LINKS="$LINKS python"
> +UNLINK="pc-bios/keymaps"
>  for bios_file in \
>      $source_path/pc-bios/*.bin \
>      $source_path/pc-bios/*.elf \
> @@ -8127,6 +8128,11 @@ for f in $LINKS ; do
>          symlink "$source_path/$f" "$f"
>      fi
>  done
> +for f in $UNLINK ; do
> +    if [ -L "$f" ]; then
> +        rm -f "$f"
> +    fi
> +done

If this is for back-compat with old trees only we should add a
comment that documents that in the final version of this patch.

thanks
-- PMM

Re: [PULL v2 00/15] Cirrus-CI improvements, and other CI-related fixes, m68k

Posted by Thomas Huth 5 years, 2 months ago

On 03/09/2020 12.48, Peter Maydell wrote:
> On Thu, 3 Sep 2020 at 06:47, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>
>>   Hi,
>>
>>> Darn. I've added Gerd's patch since it is needed to fix the acceptance
>>> tests in the Gitlab-CI. Could we maybe revert the patch that introduced
>>> the regression instead, as long as no other proper fix is available? The
>>> failing CI is really bugging me.
>>
>> Well, ddcf607fa3d6 fixes another nasty issue (builds modifying the
>> source tree), so reverting that isn't really an option.
>>
>> We could have configure remove the symlink in case is present.  That way
>> old build trees with the symlink already created should work too.  Right
>> now only build trees created with ddcf607fa3d6 present are working
>> properly.
>>
>> Untested patch below.
>>
>> take care,
>>   Gerd
>>
>> diff --git a/configure b/configure
>> index b1e11397a827..493b4e86da62 100755
>> --- a/configure
>> +++ b/configure
>> @@ -8107,6 +8107,7 @@ LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
>>  LINKS="$LINKS tests/acceptance tests/data"
>>  LINKS="$LINKS tests/qemu-iotests/check"
>>  LINKS="$LINKS python"
>> +UNLINK="pc-bios/keymaps"
>>  for bios_file in \
>>      $source_path/pc-bios/*.bin \
>>      $source_path/pc-bios/*.elf \
>> @@ -8127,6 +8128,11 @@ for f in $LINKS ; do
>>          symlink "$source_path/$f" "$f"
>>      fi
>>  done
>> +for f in $UNLINK ; do
>> +    if [ -L "$f" ]; then
>> +        rm -f "$f"
>> +    fi
>> +done
> 
> If this is for back-compat with old trees only we should add a
> comment that documents that in the final version of this patch.

Ok, I can try to respin my pull request with this patch and the
following comment added:

# UNLINK is used to remove symlinks from older development
# versions that might get into the way when doing "git update"
# without doing a "make distclean" in between.

Does that sound ok?

I wasn't able to reproduce the problem locally so far, so I hope it's ok
if I "abuse" your merge test for this, Peter?

 Thomas