[XEN PATCH v1 0/3] CI: Upgrade the Fedora container and fix build failure

[XEN PATCH v1 0/3] CI: Upgrade the Fedora container and fix build failure

[Javi Merino]

Hi,

Continuing Andrew's work on refreshing containers in GitLab CI.  With
Fedora 40, there was a build failure in the ocaml builds.  It is due
to an unused variable in an ocaml macro when building with "-Werror" .
Patch 2 solves the build failure, but I'm unsure if it is the best
approach.  Ideas welcome.

Andrew Cooper (1):
  CI: Refresh and upgrade the Fedora container

Javi Merino (2):
  automation: Fix URL to the gitlab container registry documentation
  ocaml/libs: Fill build failure due to unused variable in ocaml macro

 automation/build/README.md                    |  2 +-
 automation/build/fedora/29.dockerfile         | 46 -----------
 automation/build/fedora/40-x86_64.dockerfile  | 77 +++++++++++++++++++
 automation/gitlab-ci/build.yaml               |  8 +-
 automation/scripts/containerize               |  2 +-
 .../domain_getinfo_stubs_v1.c                 |  3 +
 6 files changed, 86 insertions(+), 52 deletions(-)
 delete mode 100644 automation/build/fedora/29.dockerfile
 create mode 100644 automation/build/fedora/40-x86_64.dockerfile

-- 
2.46.1

[XEN PATCH v1 1/3] automation: Fix URL to the gitlab container registry documentation

[Javi Merino]

The gitlab documentation is now at
https://docs.gitlab.com/ee/administration/packages/container_registry.html

Signed-off-by: Javi Merino <javi.merino@cloud.com>
---
 automation/build/README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/automation/build/README.md b/automation/build/README.md
index bd0c0e041804..ecc898680c91 100644
--- a/automation/build/README.md
+++ b/automation/build/README.md
@@ -110,7 +110,7 @@ make -C automation/build opensuse/tumbleweed-x86_64 PUSH=1
 
 [BuildKit]: https://docs.docker.com/build/buildkit/
 [registry]: https://gitlab.com/xen-project/xen/container_registry
-[registry help]: https://gitlab.com/help/user/project/container_registry
+[registry help]: https://docs.gitlab.com/ee/user/packages/container_registry/
 
 
 Building/Running container for a different architecture
-- 
2.46.1

Re: [XEN PATCH v1 1/3] automation: Fix URL to the gitlab container registry documentation

[Anthony PERARD]

On Thu, Oct 17, 2024 at 05:20:19PM +0100, Javi Merino wrote:
> The gitlab documentation is now at
> https://docs.gitlab.com/ee/administration/packages/container_registry.html

This link seems to be for self-managed instance of gitlab, but the link
in the patch looks fine.

> Signed-off-by: Javi Merino <javi.merino@cloud.com>

The patch description could be removed, so:
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

> ---
>  automation/build/README.md | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/automation/build/README.md b/automation/build/README.md
> index bd0c0e041804..ecc898680c91 100644
> --- a/automation/build/README.md
> +++ b/automation/build/README.md
> @@ -110,7 +110,7 @@ make -C automation/build opensuse/tumbleweed-x86_64 PUSH=1
>  
>  [BuildKit]: https://docs.docker.com/build/buildkit/
>  [registry]: https://gitlab.com/xen-project/xen/container_registry
> -[registry help]: https://gitlab.com/help/user/project/container_registry
> +[registry help]: https://docs.gitlab.com/ee/user/packages/container_registry/
>  
>  
>  Building/Running container for a different architecture

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [XEN PATCH v1 1/3] automation: Fix URL to the gitlab container registry documentation

[Stefano Stabellini]

On Thu, 17 Oct 2024, Anthony PERARD wrote:
> On Thu, Oct 17, 2024 at 05:20:19PM +0100, Javi Merino wrote:
> > The gitlab documentation is now at
> > https://docs.gitlab.com/ee/administration/packages/container_registry.html
> 
> This link seems to be for self-managed instance of gitlab, but the link
> in the patch looks fine.
> 
> > Signed-off-by: Javi Merino <javi.merino@cloud.com>
> 
> The patch description could be removed, so:
> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>


Acked-by: Stefano Stabellini <sstabellini@kernel.org>

 
> > ---
> >  automation/build/README.md | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/automation/build/README.md b/automation/build/README.md
> > index bd0c0e041804..ecc898680c91 100644
> > --- a/automation/build/README.md
> > +++ b/automation/build/README.md
> > @@ -110,7 +110,7 @@ make -C automation/build opensuse/tumbleweed-x86_64 PUSH=1
> >  
> >  [BuildKit]: https://docs.docker.com/build/buildkit/
> >  [registry]: https://gitlab.com/xen-project/xen/container_registry
> > -[registry help]: https://gitlab.com/help/user/project/container_registry
> > +[registry help]: https://docs.gitlab.com/ee/user/packages/container_registry/
> >  
> >  
> >  Building/Running container for a different architecture
> 
> -- 
> 
> Anthony Perard | Vates XCP-ng Developer
> 
> XCP-ng & Xen Orchestra - Vates solutions
> 
> web: https://vates.tech
> 

[XEN PATCH v1 2/3] ocaml/libs: Fill build failure due to unused variable in ocaml macro

[Javi Merino]

On Fedora 40, the build fails with:

    gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -Werror -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE   -fPIC -I/usr/lib64/ocaml -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -D__XEN_TOOLS__ -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include  -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
    In file included from domain_getinfo_stubs_v1.c:10:
    domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
    /usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
      275 |   struct caml__roots_block *caml__frame = *caml_local_roots_ptr
          |                             ^~~~~~~~~~~
    domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
      48 |         CAMLparam0();
         |         ^~~~~~~~~~
    cc1: all warnings being treated as errors

The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:

    #define CAMLparam0()                                                    \
      struct caml__roots_block** caml_local_roots_ptr =                     \
        (DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0,           \
         &CAML_LOCAL_ROOTS);                                                \
      struct caml__roots_block *caml__frame = *caml_local_roots_ptr

We can't modify the macro.  Mark the xsd_glue_failwith() function with
ignore "-Wunused-variable" to prevent gcc from failing the build due
to the unused variable.

Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
Signed-off-by: Javi Merino <javi.merino@cloud.com>
---

While it is not ideal, this is the most effective option I have found
that avoids treating warnings as errors.  I can add a comment above
the pragma explaining why if we decide that this is the way forward.

 .../domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c         | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c b/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c
index 7be386f4d4c2..df2b3c74c727 100644
--- a/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c
+++ b/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/domain_getinfo_stubs_v1.c
@@ -42,6 +42,8 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
 	.compare_ext = custom_compare_ext_default, /* Can't compare     */
 };
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-variable"
 static void Noreturn xsd_glue_failwith(
 	xc_interface *xch, const char *func, unsigned int line)
 {
@@ -70,6 +72,7 @@ static void Noreturn xsd_glue_failwith(
 
 	caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
 }
+#pragma GCC diagnostic pop
 #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
 
 CAMLprim value stub_xsd_glue_xc_interface_open(value unit)
-- 
2.46.1

Re: [XEN PATCH v1 2/3] ocaml/libs: Fill build failure due to unused variable in ocaml macro

[Andrew Cooper]

On 17/10/2024 5:20 pm, Javi Merino wrote:
> On Fedora 40, the build fails with:
>
>     gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -Werror -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE   -fPIC -I/usr/lib64/ocaml -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -D__XEN_TOOLS__ -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include  -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
>     In file included from domain_getinfo_stubs_v1.c:10:
>     domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
>     /usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
>       275 |   struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>           |                             ^~~~~~~~~~~
>     domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
>       48 |         CAMLparam0();
>          |         ^~~~~~~~~~
>     cc1: all warnings being treated as errors
>
> The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
>
>     #define CAMLparam0()                                                    \
>       struct caml__roots_block** caml_local_roots_ptr =                     \
>         (DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0,           \
>          &CAML_LOCAL_ROOTS);                                                \
>       struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>
> We can't modify the macro.  Mark the xsd_glue_failwith() function with
> ignore "-Wunused-variable" to prevent gcc from failing the build due
> to the unused variable.
>
> Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
> Signed-off-by: Javi Merino <javi.merino@cloud.com>

That's horrible...

I note the Ocaml manual even says:

"Some C compilers give bogus warnings about unused variables
caml__dummy_xxx at each use of CAMLparam and CAMLlocal. You should
ignore them."  which a brave claim to make...


The problem with pragma gcc is that we support Clang too.

Having had a play, this works too

@@ -45,7 +45,9 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
 static void Noreturn xsd_glue_failwith(
        xc_interface *xch, const char *func, unsigned int line)
 {
+#define caml__frame __attribute__((unused)) caml__frame
        CAMLparam0();
+#undef caml__frame
        CAMLlocal1(msg);
        const xc_error *error = xch ? xc_get_last_error(xch) : NULL;
        char *str = NULL;

and is rather more selective.


However, looking through other bits of memory.h, there's this gem:

#define CAMLnoreturn ((void) caml__frame)

which has existed since db3745919 "suppression des warnings "unused
variable" de gcc" in 2004.

So, I think this is a better fix:

@@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
        free(str);
 
        caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
+       CAMLnoreturn;
 }
 #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
 
and F40 seem happy with the result.

~Andrew

Re: [XEN PATCH v1 2/3] ocaml/libs: Fill build failure due to unused variable in ocaml macro

[Andrew Cooper]

On 17/10/2024 6:47 pm, Andrew Cooper wrote:
> On 17/10/2024 5:20 pm, Javi Merino wrote:
>> On Fedora 40, the build fails with:
>>
>>     gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -Werror -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE   -fPIC -I/usr/lib64/ocaml -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -D__XEN_TOOLS__ -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include  -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
>>     In file included from domain_getinfo_stubs_v1.c:10:
>>     domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
>>     /usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
>>       275 |   struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>>           |                             ^~~~~~~~~~~
>>     domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
>>       48 |         CAMLparam0();
>>          |         ^~~~~~~~~~
>>     cc1: all warnings being treated as errors
>>
>> The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
>>
>>     #define CAMLparam0()                                                    \
>>       struct caml__roots_block** caml_local_roots_ptr =                     \
>>         (DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0,           \
>>          &CAML_LOCAL_ROOTS);                                                \
>>       struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>>
>> We can't modify the macro.  Mark the xsd_glue_failwith() function with
>> ignore "-Wunused-variable" to prevent gcc from failing the build due
>> to the unused variable.
>>
>> Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
>> Signed-off-by: Javi Merino <javi.merino@cloud.com>
> That's horrible...
>
> I note the Ocaml manual even says:
>
> "Some C compilers give bogus warnings about unused variables
> caml__dummy_xxx at each use of CAMLparam and CAMLlocal. You should
> ignore them."  which a brave claim to make...
>
>
> The problem with pragma gcc is that we support Clang too.
>
> Having had a play, this works too
>
> @@ -45,7 +45,9 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
>  static void Noreturn xsd_glue_failwith(
>         xc_interface *xch, const char *func, unsigned int line)
>  {
> +#define caml__frame __attribute__((unused)) caml__frame
>         CAMLparam0();
> +#undef caml__frame
>         CAMLlocal1(msg);
>         const xc_error *error = xch ? xc_get_last_error(xch) : NULL;
>         char *str = NULL;
>
> and is rather more selective.
>
>
> However, looking through other bits of memory.h, there's this gem:
>
> #define CAMLnoreturn ((void) caml__frame)
>
> which has existed since db3745919 "suppression des warnings "unused
> variable" de gcc" in 2004.
>
> So, I think this is a better fix:
>
> @@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
>         free(str);
>  
>         caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
> +       CAMLnoreturn;
>  }
>  #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
>  
> and F40 seem happy with the result.

And https://github.com/ocaml/ocaml/pull/13561 to update Ocaml's
documentation on the matter.

~Andrew

Re: [XEN PATCH v1 2/3] ocaml/libs: Fill build failure due to unused variable in ocaml macro

[Christian Lindig]

> On 17 Oct 2024, at 18:47, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> 
> So, I think this is a better fix:
> 
> @@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
>         free(str);
>  
>         caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
> +       CAMLnoreturn;
>  }
>  #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
>  
> and F40 seem happy with the result.

Acked-by: Christian Lindig <christian.lindig@cloud.com>


I like the effort to remove warnings.

— C

Re: [XEN PATCH v1 2/3] ocaml/libs: Fill build failure due to unused variable in ocaml macro

[Javi Merino]

On Thu, Oct 17, 2024 at 06:47:44PM +0100, Andrew Cooper wrote:
> On 17/10/2024 5:20 pm, Javi Merino wrote:
> > On Fedora 40, the build fails with:
> >
> >     gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -Werror -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE   -fPIC -I/usr/lib64/ocaml -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -D__XEN_TOOLS__ -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include  -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
> >     In file included from domain_getinfo_stubs_v1.c:10:
> >     domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
> >     /usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
> >       275 |   struct caml__roots_block *caml__frame = *caml_local_roots_ptr
> >           |                             ^~~~~~~~~~~
> >     domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
> >       48 |         CAMLparam0();
> >          |         ^~~~~~~~~~
> >     cc1: all warnings being treated as errors
> >
> > The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
> >
> >     #define CAMLparam0()                                                    \
> >       struct caml__roots_block** caml_local_roots_ptr =                     \
> >         (DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0,           \
> >          &CAML_LOCAL_ROOTS);                                                \
> >       struct caml__roots_block *caml__frame = *caml_local_roots_ptr
> >
> > We can't modify the macro.  Mark the xsd_glue_failwith() function with
> > ignore "-Wunused-variable" to prevent gcc from failing the build due
> > to the unused variable.
> >
> > Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
> > Signed-off-by: Javi Merino <javi.merino@cloud.com>
> 
> That's horrible...
> 
> I note the Ocaml manual even says:
> 
> "Some C compilers give bogus warnings about unused variables
> caml__dummy_xxx at each use of CAMLparam and CAMLlocal. You should
> ignore them."  which a brave claim to make...
> 
> 
> The problem with pragma gcc is that we support Clang too.
> 
> Having had a play, this works too
> 
> @@ -45,7 +45,9 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
>  static void Noreturn xsd_glue_failwith(
>         xc_interface *xch, const char *func, unsigned int line)
>  {
> +#define caml__frame __attribute__((unused)) caml__frame
>         CAMLparam0();
> +#undef caml__frame
>         CAMLlocal1(msg);
>         const xc_error *error = xch ? xc_get_last_error(xch) : NULL;
>         char *str = NULL;
> 
> and is rather more selective.
> 
> 
> However, looking through other bits of memory.h, there's this gem:
> 
> #define CAMLnoreturn ((void) caml__frame)
> 
> which has existed since db3745919 "suppression des warnings "unused
> variable" de gcc" in 2004.
> 
> So, I think this is a better fix:
> 
> @@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
>         free(str);
>  
>         caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
> +       CAMLnoreturn;
>  }
>  #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
>  

Right!  I actually tried put a (void)caml__frame to silence the
warning, but I didn't think of looking for a macro that would do
exactly that.

Do I have to resend the series with your patch, or can you commit it
directly?

Cheers,
Javi

Re: [XEN PATCH v1 2/3] ocaml/libs: Fill build failure due to unused variable in ocaml macro

[Andrew Cooper]

On 18/10/2024 9:50 am, Javi Merino wrote:
> On Thu, Oct 17, 2024 at 06:47:44PM +0100, Andrew Cooper wrote:
>> On 17/10/2024 5:20 pm, Javi Merino wrote:
>>> On Fedora 40, the build fails with:
>>>
>>>     gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -Werror -O2 -fomit-frame-pointer -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__ -MMD -MP -MF .domain_getinfo_stubs_v1.o.d -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE   -fPIC -I/usr/lib64/ocaml -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs -I /build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../libs/xsd_glue -I/build/tools/ocaml/libs/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include -D__XEN_TOOLS__ -I/build/tools/ocaml/libs\/xsd_glue/domain_getinfo_plugin_v1/../../../../../tools/include  -c -o domain_getinfo_stubs_v1.o domain_getinfo_stubs_v1.c
>>>     In file included from domain_getinfo_stubs_v1.c:10:
>>>     domain_getinfo_stubs_v1.c: In function 'xsd_glue_failwith':
>>>     /usr/lib64/ocaml/caml/memory.h:275:29: error: unused variable 'caml__frame' [-Werror=unused-variable]
>>>       275 |   struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>>>           |                             ^~~~~~~~~~~
>>>     domain_getinfo_stubs_v1.c:48:9: note: in expansion of macro 'CAMLparam0'
>>>       48 |         CAMLparam0();
>>>          |         ^~~~~~~~~~
>>>     cc1: all warnings being treated as errors
>>>
>>> The CAMLparam0 macro is defined in /usr/lib64/ocaml/caml/memory.h:255 as:
>>>
>>>     #define CAMLparam0()                                                    \
>>>       struct caml__roots_block** caml_local_roots_ptr =                     \
>>>         (DO_CHECK_CAML_STATE ? Caml_check_caml_state() : (void)0,           \
>>>          &CAML_LOCAL_ROOTS);                                                \
>>>       struct caml__roots_block *caml__frame = *caml_local_roots_ptr
>>>
>>> We can't modify the macro.  Mark the xsd_glue_failwith() function with
>>> ignore "-Wunused-variable" to prevent gcc from failing the build due
>>> to the unused variable.
>>>
>>> Fixes: a6576011a4d2 ("ocaml/libs: Implement a dynamically-loaded plugin for Xenctrl.domain_getinfo")
>>> Signed-off-by: Javi Merino <javi.merino@cloud.com>
>> That's horrible...
>>
>> I note the Ocaml manual even says:
>>
>> "Some C compilers give bogus warnings about unused variables
>> caml__dummy_xxx at each use of CAMLparam and CAMLlocal. You should
>> ignore them."  which a brave claim to make...
>>
>>
>> The problem with pragma gcc is that we support Clang too.
>>
>> Having had a play, this works too
>>
>> @@ -45,7 +45,9 @@ static struct custom_operations xsd_glue_xenctrl_ops = {
>>  static void Noreturn xsd_glue_failwith(
>>         xc_interface *xch, const char *func, unsigned int line)
>>  {
>> +#define caml__frame __attribute__((unused)) caml__frame
>>         CAMLparam0();
>> +#undef caml__frame
>>         CAMLlocal1(msg);
>>         const xc_error *error = xch ? xc_get_last_error(xch) : NULL;
>>         char *str = NULL;
>>
>> and is rather more selective.
>>
>>
>> However, looking through other bits of memory.h, there's this gem:
>>
>> #define CAMLnoreturn ((void) caml__frame)
>>
>> which has existed since db3745919 "suppression des warnings "unused
>> variable" de gcc" in 2004.
>>
>> So, I think this is a better fix:
>>
>> @@ -69,6 +69,7 @@ static void Noreturn xsd_glue_failwith(
>>         free(str);
>>  
>>         caml_raise_with_arg(*caml_named_value("xsg.error_v1"), msg);
>> +       CAMLnoreturn;
>>  }
>>  #define xsd_glue_failwith(xch) xsd_glue_failwith(xch, __func__, __LINE__)
>>  
> Right!  I actually tried put a (void)caml__frame to silence the
> warning, but I didn't think of looking for a macro that would do
> exactly that.

:)  It's Ocaml.  I had a hunch there'd be something.

> Do I have to resend the series with your patch, or can you commit it
> directly?

I'll just fold it on commit.  No point spending more time on this.

In the meantime, Ocaml upstream have already merged my docs PR about this.

~Andrew

[XEN PATCH v1 3/3] CI: Refresh and upgrade the Fedora container

[Javi Merino]

From: Andrew Cooper <andrew.cooper3@citrix.com>

Fedora 29 is long out of date.  Move forward 5 years to Fedora 40.

Include all the usual improvements.  Rework the container to be non-root, use
heredocs for legibility, and switch to the new naming scheme.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Javi Merino <javi.merino@cloud.com>
---
 automation/build/fedora/29.dockerfile        | 46 ------------
 automation/build/fedora/40-x86_64.dockerfile | 77 ++++++++++++++++++++
 automation/gitlab-ci/build.yaml              |  8 +-
 automation/scripts/containerize              |  2 +-
 4 files changed, 82 insertions(+), 51 deletions(-)
 delete mode 100644 automation/build/fedora/29.dockerfile
 create mode 100644 automation/build/fedora/40-x86_64.dockerfile

diff --git a/automation/build/fedora/29.dockerfile b/automation/build/fedora/29.dockerfile
deleted file mode 100644
index d5f017ac723d..000000000000
--- a/automation/build/fedora/29.dockerfile
+++ /dev/null
@@ -1,46 +0,0 @@
-# syntax=docker/dockerfile:1
-FROM --platform=linux/amd64 fedora:29
-LABEL maintainer.name="The Xen Project" \
-      maintainer.email="xen-devel@lists.xenproject.org"
-
-# install Xen depends
-RUN dnf -y install \
-        clang \
-        gcc \
-        gcc-c++ \
-        ncurses-devel \
-        zlib-devel \
-        python-devel \
-        python3-devel \
-        libuuid-devel \
-        pkgconfig \
-        flex \
-        bison \
-        libaio-devel \
-        glib2-devel \
-        yajl-devel \
-        pixman-devel \
-        glibc-devel \
-        make \
-        binutils \
-        git \
-        wget \
-        acpica-tools \
-        python-markdown \
-        patch \
-        checkpolicy \
-        dev86 \
-        xz-devel \
-        bzip2 \
-        nasm \
-        ocaml \
-        ocaml-findlib \
-        golang \
-        # QEMU
-        ninja-build \
-    && dnf clean all && \
-    rm -rf /var/cache/dnf
-
-RUN useradd --create-home user
-USER user
-WORKDIR /build
diff --git a/automation/build/fedora/40-x86_64.dockerfile b/automation/build/fedora/40-x86_64.dockerfile
new file mode 100644
index 000000000000..7d4d4cc2ac0a
--- /dev/null
+++ b/automation/build/fedora/40-x86_64.dockerfile
@@ -0,0 +1,77 @@
+# syntax=docker/dockerfile:1
+FROM --platform=linux/amd64 fedora:40
+LABEL maintainer.name="The Xen Project"
+LABEL maintainer.email="xen-devel@lists.xenproject.org"
+
+RUN <<EOF
+    set -e
+
+    useradd --create-home user
+
+    dnf -y update
+
+    DEPS=(
+        # Xen
+        binutils
+        gcc
+        make
+        python3
+        # Kconfig
+        bison
+        flex
+        # Flask
+        checkpolicy
+
+        # Tools (general)
+        git-core
+        gzip
+        patch
+        perl-interpreter
+        perl-File-Find
+        pkgconfig
+        wget
+        # libxenguest dombuilder
+        bzip2-devel
+        libzstd-devel
+        lzo-devel
+        xz-devel
+        zlib-devel
+        # libacpi
+        acpica-tools
+        # libxl
+        libuuid-devel
+        yajl-devel
+        # xen-foreign
+        diffutils
+        # RomBIOS
+        dev86
+        # Header Check
+        gcc-c++
+        # xentop
+        ncurses-devel
+        # Python bindings
+        python3-devel
+        python3-setuptools
+        # Ocaml bindings/oxenstored
+        ocaml
+        ocaml-findlib
+        # Golang bindings
+        golang
+
+        # Stubdom download/extract
+        bzip2
+
+        # Qemu build
+        glib2-devel
+        pixman-devel
+        ninja-build
+    )
+
+    dnf -y --setopt=install_weak_deps=False install "${DEPS[@]}"
+
+    dnf clean all
+    rm -rf /var/cache/dnf
+EOF
+
+USER user
+WORKDIR /build
diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
index c2db69ecf86c..09dd9e6ccbd0 100644
--- a/automation/gitlab-ci/build.yaml
+++ b/automation/gitlab-ci/build.yaml
@@ -579,15 +579,15 @@ debian-bookworm-32-gcc-debug:
   variables:
     CONTAINER: debian:bookworm-i386
 
-fedora-gcc:
+fedora-40-x86_64-gcc:
   extends: .gcc-x86-64-build
   variables:
-    CONTAINER: fedora:29
+    CONTAINER: fedora:40-x86_64
 
-fedora-gcc-debug:
+fedora-40-x86_64-gcc-debug:
   extends: .gcc-x86-64-build-debug
   variables:
-    CONTAINER: fedora:29
+    CONTAINER: fedora:40-x86_64
 
 ubuntu-16.04-x86_64-clang:
   extends: .clang-x86-64-build
diff --git a/automation/scripts/containerize b/automation/scripts/containerize
index 7607b78f7666..6ac02c42d124 100755
--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -28,7 +28,7 @@ case "_${CONTAINER}" in
     _alpine-arm64v8) CONTAINER="${BASE}/alpine:3.18-arm64v8" ;;
     _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;;
     _centos7) CONTAINER="${BASE}/centos:7" ;;
-    _fedora) CONTAINER="${BASE}/fedora:29";;
+    _fedora) CONTAINER="${BASE}/fedora:40-x86_64";;
     _bullseye-ppc64le) CONTAINER="${BASE}/debian:11-ppc64le" ;;
     _bookworm-ppc64le) CONTAINER="${BASE}/debian:12-ppc64le" ;;
     _bullseye-riscv64) CONTAINER="${BASE}/debian:11-riscv64" ;;
-- 
2.46.1

Re: [XEN PATCH v1 3/3] CI: Refresh and upgrade the Fedora container

[Anthony PERARD]

On Thu, Oct 17, 2024 at 05:20:21PM +0100, Javi Merino wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Fedora 29 is long out of date.  Move forward 5 years to Fedora 40.
> 
> Include all the usual improvements.  Rework the container to be non-root, use
> heredocs for legibility, and switch to the new naming scheme.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Signed-off-by: Javi Merino <javi.merino@cloud.com>

Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

Re: [XEN PATCH v1 3/3] CI: Refresh and upgrade the Fedora container

[Stefano Stabellini]

On Thu, 17 Oct 2024, Anthony PERARD wrote:
> On Thu, Oct 17, 2024 at 05:20:21PM +0100, Javi Merino wrote:
> > From: Andrew Cooper <andrew.cooper3@citrix.com>
> > 
> > Fedora 29 is long out of date.  Move forward 5 years to Fedora 40.
> > 
> > Include all the usual improvements.  Rework the container to be non-root, use
> > heredocs for legibility, and switch to the new naming scheme.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Signed-off-by: Javi Merino <javi.merino@cloud.com>
> 
> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Assuming it was tested successfully:

Acked-by: Stefano Stabellini <sstabellini@kernel.org>