Re: [PATCH v1 0/6] tools/ocaml/xenstored: simplify code

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Christian Lindig <christian.lindig@citrix.com>,
	Edwin Torok <edvin.torok@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Ian Jackson <Ian.Jackson@citrix.com>,
	"dave@recoil.org" <dave@recoil.org>,  "wl@xen.org" <wl@xen.org>
Subject: Re: [PATCH v1 0/6] tools/ocaml/xenstored: simplify code
Date: Tue, 18 Aug 2020 13:40:18 +0100	[thread overview]
Message-ID: <cd17508f-b170-3e3c-d8d3-26e827f7492a@citrix.com> (raw)
In-Reply-To: <1597742707142.74318@citrix.com>

On 18/08/2020 10:25, Christian Lindig wrote:
> I see little reason to support old OCaml releases and requiring OCaml 4.06 would be fine with me but I assume that the project might have its own ideas about this.
>
> ________________________________________
> From: Edwin Torok
> Sent: 18 August 2020 08:28
> To: Christian Lindig; xen-devel@lists.xenproject.org
> Cc: Ian Jackson; dave@recoil.org; wl@xen.org
> Subject: Re: [PATCH v1 0/6] tools/ocaml/xenstored: simplify code
>
> On Mon, 2020-08-17 at 14:56 +0200, Christian Lindig wrote:
>> This all looks good - I left a small comment on one of the patches
>> and I agree that this needs testing. I also wonder about
>> compatibility with earlier OCaml releases that we support but I see
>> no real obstacles.
>>
> I've developed the series using OCaml 4.08.1. I think the newest
> feature I used was Map.update (OCaml 4.06, nearly 3 years ago).
> Looking through https://repology.org/project/ocaml/versions I'm not
> sure if we can require more than 4.05 though.
> The README in Xen doesn't specify a minimum version, but configure
> checks for >=4.02.
>
> I can try to backport my series to OCaml 4.05 (to use Map.find_opt
> instead of Map.update) and update the configure check to require 4.05.
> It would be possible to backport even further to 4.02 by introducing
> additional inefficiencies (Map.mem + Map.find would traverse the map
> twice, and Map.find on its own would raise an exception on Not found,
> which is more costly than returning None in Map.find_opt), I'd avoid
> doing that.
>
> Xen's CI from automation might need some updates to use latest stable
> versions:
> * Fedora 29 is EOL, should use at least Fedora 31
> * Debian Jessie is EOL. Stretch is present, but Buster is missing

We're working on the CI loop.

As maintainer, it is ultimately Christian's choice to as to if/when to
bump the minimum versions.

As a general rule, we don't want to be sufficiently bleeding edge to
rule out in-use distros.  I have no idea if 4.06 is ok there, or whether
it is too new.  Then again, the Ocaml components are strictly optional
so it is perhaps less important.

Whatever happens WRT version, the configure change should occur before
changes in the code which would fail on older versions.

~Andrew