diff mbox series

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

Message ID 11cc8480e6e52d5c2dccc7d8d65e1362c7fba685.1706697216.git.edwin.torok@cloud.com (mailing list archive)
State New, archived
Headers show
Series tools/ocaml: support OCaml 5.x, drop support for <=4.05 | expand

Commit Message

Edwin Torok Jan. 31, 2024, 10:42 a.m. UTC
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(-)

Comments

Anthony PERARD Jan. 31, 2024, 4:36 p.m. UTC | #1
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,
Andrew Cooper Jan. 31, 2024, 5:17 p.m. UTC | #2
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
Edwin Torok Jan. 31, 2024, 5:34 p.m. UTC | #3
> 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
Andrew Cooper Jan. 31, 2024, 7:43 p.m. UTC | #4
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
diff mbox series

Patch

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"