Nick Rosbrook writes ("Re: [PATCH for-4.14] tools: check go compiler version if
present"):
> On Fri, Jun 12, 2020 at 05:40:21PM +0100, Ian Jackson wrote:
> > Nick Rosbrook writes ("[PATCH for-4.14] tools: check go compiler version if
> > present"):
> > > Currently, no minimum go compiler version is required by the configure
> > > scripts. However, the go bindings actually will not build with some
> > > older versions of go. Add a check for a minimum go version of 1.11.1 in
> > > accordance with tools/golang/xenlight/go.mod.
...
> > I don't understand this code. Why are you checking $enable_golang in
> > your new code whereas the old code checks $golang ? I actually read
> > the generated code trying to see where $golang is set and AFAICT it is
> > only ever set to n ?
>
> For some background, in an attempt to be consistent with existing code
> here, I basically copied the logic for enabling the ocaml tools.
>
> The logic is setup in a way that (unless --disable-golang is set) if a
> suitable version of the go compiler is found, then golang is enabled by
> default. If, however, a suitable go compiler is not found (either go
> is not present at all, or it is too old), then golang is disabled. This
> part happens silently unless --enable-golang is _explicitly_ set by the
> user, in which case an error is thrown by ./configure. This is why
> $enable_golang is checked.
Thanks. Well, I have to say I still don't understand the code.
But as you note, the behaviour you describe is the one I would want.
And the confusingness doesn't seem to have been your fault. It would
probably be worse to have two different arrangements. Let's leave it
as it is for now.
> > This is all very weird. Surely golang should be enabled by default
> > when the proper compiler is present, and disabled by default
> > otherwise. I think this effect will be quite hard to achieve with
> > AX_ARG_DEFAULT_ENABLE. Probably you should be using AC_ARG_ENABLE
> > and doing the defaulting yourself so that you can use a computed
> > default etc.
>
> I believe the behavior you described here is the same as I described
> above (and is acheived in this patch). But, I'm happy to re-work the
> implementation if necessary so that the code is more clear.
Ideally at some point maybe we would make this clearer but not now.
Have you tested the situations you describe ? That might be a better
way of checking that it's right than the code inspection which is
obviously failing for me now...
I definitely think we want to fix this for 4.14. So thanks for your
continued help!
Ian.
