[PATCH v1 0/2] tools/ocaml: support OCaml 5.x, drop support for <=4.05

[PATCH v1 0/2] tools/ocaml: support OCaml 5.x, drop support for <=4.05

[Edwin Török]

Fix building oxenstored with OCaml 5.x.
OCaml 5.x has removed some functions that have been deprecated for many years,
in order to support OCaml 5.x we need to drop support for OCaml 4.02.

Tested in gitlab CI (together with my other series):
https://gitlab.com/xen-project/people/edwintorok/xen/-/pipelines/1158302827

Edwin Török (2):
  oxenstored: fix build on OCaml 5.x
  tools/ocaml: bump minimum version to OCaml 4.05

 tools/configure               | 2 +-
 tools/configure.ac            | 2 +-
 tools/ocaml/xenstored/disk.ml | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH v1 1/2] oxenstored: fix build on OCaml 5.x

[Edwin Török]

Char.lowercase got removed in OCaml 5.0 (it has been deprecated since 2014).

Char.lowercase_ascii has existed since OCaml 4.03, so that is the new
minimum version for oxenstored.

(Given the choice between supporting a new release and dropping support
for an 8y+ old release, we drop support for OCaml <4.03)

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
---
 tools/configure               | 2 +-
 tools/configure.ac            | 2 +-
 tools/ocaml/xenstored/disk.ml | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/configure b/tools/configure
index 0135a0059a..5723efaa56 100755
--- a/tools/configure
+++ b/tools/configure
@@ -6836,7 +6836,7 @@ else
                      -e 's/[^0-9]//g'`
 
 
-  ax_compare_version_B=`echo "4.02.0" | sed -e 's/\([0-9]*\)/Z\1Z/g' \
+  ax_compare_version_B=`echo "4.03.0" | sed -e 's/\([0-9]*\)/Z\1Z/g' \
                      -e 's/Z\([0-9]\)Z/Z0\1Z/g' \
                      -e 's/Z\([0-9][0-9]\)Z/Z0\1Z/g' \
                      -e 's/Z\([0-9][0-9][0-9]\)Z/Z0\1Z/g' \
diff --git a/tools/configure.ac b/tools/configure.ac
index 618ef8c63f..c979c3de7c 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -336,7 +336,7 @@ AS_IF([test "x$ocamltools" = "xy"], [
             AC_MSG_ERROR([Ocaml tools enabled, but missing ocamlopt or ocamlfind])])
         ocamltools="n"
     ], [
-        AX_COMPARE_VERSION([$OCAMLVERSION], [lt], [4.02.0], [
+        AX_COMPARE_VERSION([$OCAMLVERSION], [lt], [4.03.0], [
             AS_IF([test "x$enable_ocamltools" = "xyes"], [
                 AC_MSG_ERROR([Your version of OCaml: $OCAMLVERSION is not supported])])
             ocamltools="n"
diff --git a/tools/ocaml/xenstored/disk.ml b/tools/ocaml/xenstored/disk.ml
index 91f945f2bd..ccaa048faf 100644
--- a/tools/ocaml/xenstored/disk.ml
+++ b/tools/ocaml/xenstored/disk.ml
@@ -30,7 +30,7 @@ let undec c =
   | _          -> raise (Failure "undecify")
 
 let unhex c =
-  let c = Char.lowercase c in
+  let c = Char.lowercase_ascii c in
   match c with
   | '0' .. '9' -> (Char.code c) - (Char.code '0')
   | 'a' .. 'f' -> (Char.code c) - (Char.code 'a') + 10
-- 
2.43.0

[PATCH v1 2/2] tools/ocaml: bump minimum version to OCaml 4.05

[Edwin Török]

We tried bumping to 4.06.1 [1] previously, but OSSTest was holding us
back.
So bump to OCaml 4.05 instead, which should match the version on
OSSTest?

[1]: https://patchwork.kernel.org/project/xen-devel/patch/ac885ce2b63159d26d857dc3e53cf8aa63ae3646.1659118200.git.edvin.torok@citrix.com/

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
---
 tools/configure    | 2 +-
 tools/configure.ac | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/configure b/tools/configure
index 5723efaa56..3d557234b3 100755
--- a/tools/configure
+++ b/tools/configure
@@ -6836,7 +6836,7 @@ else
                      -e 's/[^0-9]//g'`
 
 
-  ax_compare_version_B=`echo "4.03.0" | sed -e 's/\([0-9]*\)/Z\1Z/g' \
+  ax_compare_version_B=`echo "4.05.0" | sed -e 's/\([0-9]*\)/Z\1Z/g' \
                      -e 's/Z\([0-9]\)Z/Z0\1Z/g' \
                      -e 's/Z\([0-9][0-9]\)Z/Z0\1Z/g' \
                      -e 's/Z\([0-9][0-9][0-9]\)Z/Z0\1Z/g' \
diff --git a/tools/configure.ac b/tools/configure.ac
index c979c3de7c..851887080c 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -336,7 +336,7 @@ AS_IF([test "x$ocamltools" = "xy"], [
             AC_MSG_ERROR([Ocaml tools enabled, but missing ocamlopt or ocamlfind])])
         ocamltools="n"
     ], [
-        AX_COMPARE_VERSION([$OCAMLVERSION], [lt], [4.03.0], [
+        AX_COMPARE_VERSION([$OCAMLVERSION], [lt], [4.05.0], [
             AS_IF([test "x$enable_ocamltools" = "xyes"], [
                 AC_MSG_ERROR([Your version of OCaml: $OCAMLVERSION is not supported])])
             ocamltools="n"
-- 
2.43.0

Re: [PATCH v1 2/2] tools/ocaml: bump minimum version to OCaml 4.05

[Anthony PERARD]

On Wed, Jan 31, 2024 at 10:42:49AM +0000, Edwin Török wrote:
> We tried bumping to 4.06.1 [1] previously, but OSSTest was holding us
> back.
> So bump to OCaml 4.05 instead, which should match the version on
> OSSTest?

Yes, it's looks that's the version osstest can currently use.
I've started an osstest flight with this patch series and your other
ocaml patch series, and so far osstest seems happy with it. The flight
isn't finished but all build jobs succeed, and a lot of the tests jobs
as well.

So:
Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

Re: [PATCH v1 2/2] tools/ocaml: bump minimum version to OCaml 4.05

[Andrew Cooper]

On 31/01/2024 4:36 pm, Anthony PERARD wrote:
> On Wed, Jan 31, 2024 at 10:42:49AM +0000, Edwin Török wrote:
>> We tried bumping to 4.06.1 [1] previously, but OSSTest was holding us
>> back.
>> So bump to OCaml 4.05 instead, which should match the version on
>> OSSTest?
> Yes, it's looks that's the version osstest can currently use.
> I've started an osstest flight with this patch series and your other
> ocaml patch series, and so far osstest seems happy with it. The flight
> isn't finished but all build jobs succeed, and a lot of the tests jobs
> as well.
>
> So:
> Acked-by: Anthony PERARD <anthony.perard@citrix.com>

A question, while I think about it.

I understand why we want patch 1.  The 4.02 -> 4.03 bump is necessary to
also compile with 5.0

But why this 4.03 -> 4.05 bump?  There is no other change in this patch.

If it's "just because", then why should we take it?  All it's doing is
moving a baseline which doesn't need appear to need to move.

~Andrew

Re: [PATCH v1 2/2] tools/ocaml: bump minimum version to OCaml 4.05

[Edwin Torok]

[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]



> On 31 Jan 2024, at 17:17, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> 
> On 31/01/2024 4:36 pm, Anthony PERARD wrote:
>> On Wed, Jan 31, 2024 at 10:42:49AM +0000, Edwin Török wrote:
>>> We tried bumping to 4.06.1 [1] previously, but OSSTest was holding us
>>> back.
>>> So bump to OCaml 4.05 instead, which should match the version on
>>> OSSTest?
>> Yes, it's looks that's the version osstest can currently use.
>> I've started an osstest flight with this patch series and your other
>> ocaml patch series, and so far osstest seems happy with it. The flight
>> isn't finished but all build jobs succeed, and a lot of the tests jobs
>> as well.
>> 
>> So:
>> Acked-by: Anthony PERARD <anthony.perard@citrix.com>
> 
> A question, while I think about it.
> 
> I understand why we want patch 1.  The 4.02 -> 4.03 bump is necessary to
> also compile with 5.0
> 
> But why this 4.03 -> 4.05 bump?  There is no other change in this patch.


The oldest supported Debian has 4.05, and I can’t find a non-EOL distro with 4.03 or 4.04 here: https://repology.org/project/ocaml/versions
I also have another series (that I haven’t sent out yet) which would use Dune 1.x in an attempt to use Dune in a way that works on OSSTest, and the oldest release I can test this on is Debian 10.

We could keep the minimum at 4.03, but would anything in the CI actually be able to test that? 

Best regards,
—Edwin

> 
> If it's "just because", then why should we take it?  All it's doing is
> moving a baseline which doesn't need appear to need to move.
> 
> ~Andrew


[-- Attachment #2: Type: text/html, Size: 2270 bytes --]

Re: [PATCH v1 2/2] tools/ocaml: bump minimum version to OCaml 4.05

[Andrew Cooper]

On 31/01/2024 5:34 pm, Edwin Torok wrote:
>
>
>> On 31 Jan 2024, at 17:17, Andrew Cooper <andrew.cooper3@citrix.com>
>> wrote:
>>
>> On 31/01/2024 4:36 pm, Anthony PERARD wrote:
>>> On Wed, Jan 31, 2024 at 10:42:49AM +0000, Edwin Török wrote:
>>>> We tried bumping to 4.06.1 [1] previously, but OSSTest was holding us
>>>> back.
>>>> So bump to OCaml 4.05 instead, which should match the version on
>>>> OSSTest?
>>> Yes, it's looks that's the version osstest can currently use.
>>> I've started an osstest flight with this patch series and your other
>>> ocaml patch series, and so far osstest seems happy with it. The flight
>>> isn't finished but all build jobs succeed, and a lot of the tests jobs
>>> as well.
>>>
>>> So:
>>> Acked-by: Anthony PERARD <anthony.perard@citrix.com>
>>
>> A question, while I think about it.
>>
>> I understand why we want patch 1.  The 4.02 -> 4.03 bump is necessary to
>> also compile with 5.0
>>
>> But why this 4.03 -> 4.05 bump?  There is no other change in this patch.
>
>
> The oldest supported Debian has 4.05, and I can’t find a non-EOL
> distro with 4.03 or 4.04 here: https://repology.org/project/ocaml/versions
> I also have another series (that I haven’t sent out yet) which would
> use Dune 1.x in an attempt to use Dune in a way that works on OSSTest,
> and the oldest release I can test this on is Debian 10.
>
> We could keep the minimum at 4.03, but would anything in the CI
> actually be able to test that?

Nah - that's a good enough reason to go to 4.05.

However, the two patches ought to be folded together, with both parts of
the justification given in one commit message.

Otherwise to anyone doing git blame, you've entirely hidden the 5.0
build fix with something that just looks like 4.03->4.05

I can sort this out on commit.

~Andrew

Re: [PATCH v1 0/2] tools/ocaml: support OCaml 5.x, drop support for <=4.05

[Christian Lindig]

> On 31 Jan 2024, at 10:42, Edwin Török <edwin.torok@cloud.com> wrote:
> 
> Fix building oxenstored with OCaml 5.x.
> OCaml 5.x has removed some functions that have been deprecated for many years,
> in order to support OCaml 5.x we need to drop support for OCaml 4.02.
> 
> Tested in gitlab CI (together with my other series):
> https://gitlab.com/xen-project/people/edwintorok/xen/-/pipelines/1158302827
> 
> Edwin Török (2):
>  oxenstored: fix build on OCaml 5.x
>  tools/ocaml: bump minimum version to OCaml 4.05
> 
> tools/configure               | 2 +-
> tools/configure.ac            | 2 +-
> tools/ocaml/xenstored/disk.ml | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
> 
> -- 
> 2.43.0
> 

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

Re: [PATCH v1 0/2] tools/ocaml: support OCaml 5.x, drop support for <=4.05

[Andrew Cooper]

On 31/01/2024 10:44 am, Christian Lindig wrote:
>> On 31 Jan 2024, at 10:42, Edwin Török <edwin.torok@cloud.com> wrote:
>>
>> Fix building oxenstored with OCaml 5.x.
>> OCaml 5.x has removed some functions that have been deprecated for many years,
>> in order to support OCaml 5.x we need to drop support for OCaml 4.02.
>>
>> Tested in gitlab CI (together with my other series):
>> https://gitlab.com/xen-project/people/edwintorok/xen/-/pipelines/1158302827
>>
>> Edwin Török (2):
>>  oxenstored: fix build on OCaml 5.x
>>  tools/ocaml: bump minimum version to OCaml 4.05
>>
>> tools/configure               | 2 +-
>> tools/configure.ac            | 2 +-
>> tools/ocaml/xenstored/disk.ml | 2 +-
>> 3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> -- 
>> 2.43.0
>>
> Acked-by: Christian Lindig <christian.lindig@cloud.com>

It occurs to me that this is the kind of thing which should get a
CHANGELOG.md entry these days.  Something like:

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1f55c9c72d10..fd7c8f5c6b82 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ The format is based on [Keep a
Changelog](https://keepachangelog.com/en/1.0.0/)
 ### Changed
  - Changed flexible array definitions in public I/O interface headers
to not
    use "1" as the number of array elements.
+ - The minimum supported Ocaml toolchain version is now 4.05
  - On x86:
    - HVM PIRQs are disabled by default.
    - Reduce IOMMU setup time for hardware domain.


ought to do.

Have we checked to see whether this drops Ocaml from any of the build
containers ?

~Andrew

Re: [PATCH v1 0/2] tools/ocaml: support OCaml 5.x, drop support for <=4.05

[Edwin Torok]

> On 31 Jan 2024, at 10:55, Andrew Cooper <Andrew.Cooper3@citrix.com> wrote:
> 
> On 31/01/2024 10:44 am, Christian Lindig wrote:
>>> On 31 Jan 2024, at 10:42, Edwin Török <edwin.torok@cloud.com> wrote:
>>> 
>>> Fix building oxenstored with OCaml 5.x.
>>> OCaml 5.x has removed some functions that have been deprecated for many years,
>>> in order to support OCaml 5.x we need to drop support for OCaml 4.02.
>>> 
>>> Tested in gitlab CI (together with my other series):
>>> https://gitlab.com/xen-project/people/edwintorok/xen/-/pipelines/1158302827
>>> 
>>> Edwin Török (2):
>>> oxenstored: fix build on OCaml 5.x
>>> tools/ocaml: bump minimum version to OCaml 4.05
>>> 
>>> tools/configure               | 2 +-
>>> tools/configure.ac            | 2 +-
>>> tools/ocaml/xenstored/disk.ml | 2 +-
>>> 3 files changed, 3 insertions(+), 3 deletions(-)
>>> 
>>> -- 
>>> 2.43.0
>>> 
>> Acked-by: Christian Lindig <christian.lindig@cloud.com>
> 
> It occurs to me that this is the kind of thing which should get a
> CHANGELOG.md entry these days.  Something like:
> 
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index 1f55c9c72d10..fd7c8f5c6b82 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -9,6 +9,7 @@ The format is based on [Keep a
> Changelog](https://keepachangelog.com/en/1.0.0/)
>  ### Changed
>   - Changed flexible array definitions in public I/O interface headers
> to not
>     use "1" as the number of array elements.
> + - The minimum supported Ocaml toolchain version is now 4.05
>   - On x86:
>     - HVM PIRQs are disabled by default.
>     - Reduce IOMMU setup time for hardware domain.

Sounds good.

Should this be mentioned in https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Sending_a_Patch_Series?

Best regards,
—Edwin

> 
> 
> ought to do.
> 
> Have we checked to see whether this drops Ocaml from any of the build
> containers ?


I can look into this later, haven’t tried rebuilding the containers (the gitlab CI passed though)

Best regards,
—Edwin

> 
> ~Andrew