Re: [PATCH for-4.14] tools: check go compiler version if present

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

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.