diff mbox series

[1/2] tools: build golang tools if go compiler is present

Message ID c2d966b43313c9df64551b0ce31462c176445b70.1587599095.git.rosbrookn@ainfosec.com (mailing list archive)
State Superseded
Headers show
Series build golang tools | expand

Commit Message

Nick Rosbrook April 23, 2020, 12:25 a.m. UTC
By default, if the go compiler is found by the configure script, build
the golang tools. If the compiler is not found, and
--enable-golang_tools was not explicitly set, do not build to the golang
tools.

The new corresponding make variable is CONFIG_GOLANG_TOOLS. Remove
CONFIG_GOLANG from tools/Rules.mk since the new variable is set by
configure.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 config/Tools.mk.in |   1 +
 m4/golang.m4       |   4 ++
 tools/Makefile     |   2 +-
 tools/Rules.mk     |   2 -
 tools/configure    | 138 +++++++++++++++++++++++++++++++++++++++++++++
 tools/configure.ac |  12 ++++
 6 files changed, 156 insertions(+), 3 deletions(-)
 create mode 100644 m4/golang.m4

Comments

Wei Liu April 23, 2020, 10:25 a.m. UTC | #1
On Wed, Apr 22, 2020 at 08:25:25PM -0400, Nick Rosbrook wrote:
> By default, if the go compiler is found by the configure script, build
> the golang tools. If the compiler is not found, and
> --enable-golang_tools was not explicitly set, do not build to the golang

--enable-golang-tools here.

> tools.
> 
> The new corresponding make variable is CONFIG_GOLANG_TOOLS. Remove
> CONFIG_GOLANG from tools/Rules.mk since the new variable is set by
> configure.
> 
> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>

Acked-by: Wei Liu <wl@xen.org>

Note to self: fix commit message and maybe rerun autogen.sh.
George Dunlap April 23, 2020, 12:09 p.m. UTC | #2
On Thu, Apr 23, 2020 at 11:25 AM Wei Liu <wl@xen.org> wrote:

> On Wed, Apr 22, 2020 at 08:25:25PM -0400, Nick Rosbrook wrote:
> > By default, if the go compiler is found by the configure script, build
> > the golang tools. If the compiler is not found, and
> > --enable-golang_tools was not explicitly set, do not build to the golang
>
> --enable-golang-tools here.
>
> > tools.
> >
> > The new corresponding make variable is CONFIG_GOLANG_TOOLS. Remove
> > CONFIG_GOLANG from tools/Rules.mk since the new variable is set by
> > configure.
> >
> > Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
>
> Acked-by: Wei Liu <wl@xen.org>
>
> Note to self: fix commit message and maybe rerun autogen.sh.
>

It doesn't look like that's a typo -- if you want it to be `-` instead of
`_`, the patch needs to be changed (at least as far as I can tell w/ my
admittedly limited automake-foo).

 -George
Wei Liu April 23, 2020, 1:11 p.m. UTC | #3
On Thu, Apr 23, 2020 at 01:09:43PM +0100, George Dunlap wrote:
> On Thu, Apr 23, 2020 at 11:25 AM Wei Liu <wl@xen.org> wrote:
> 
> > On Wed, Apr 22, 2020 at 08:25:25PM -0400, Nick Rosbrook wrote:
> > > By default, if the go compiler is found by the configure script, build
> > > the golang tools. If the compiler is not found, and
> > > --enable-golang_tools was not explicitly set, do not build to the golang
> >
> > --enable-golang-tools here.
> >
> > > tools.
> > >
> > > The new corresponding make variable is CONFIG_GOLANG_TOOLS. Remove
> > > CONFIG_GOLANG from tools/Rules.mk since the new variable is set by
> > > configure.
> > >
> > > Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
> >
> > Acked-by: Wei Liu <wl@xen.org>
> >
> > Note to self: fix commit message and maybe rerun autogen.sh.
> >
> 
> It doesn't look like that's a typo -- if you want it to be `-` instead of
> `_`, the patch needs to be changed (at least as far as I can tell w/ my
> admittedly limited automake-foo).

My bad. The code indeed says --enable-golang_tools.

Wei.

> 
>  -George
George Dunlap April 23, 2020, 1:44 p.m. UTC | #4
On Thu, Apr 23, 2020 at 1:51 AM Nick Rosbrook <rosbrookn@gmail.com> wrote:

> By default, if the go compiler is found by the configure script, build
> the golang tools. If the compiler is not found, and
> --enable-golang_tools was not explicitly set, do not build to the golang
> tools.
>
> The new corresponding make variable is CONFIG_GOLANG_TOOLS. Remove
> CONFIG_GOLANG from tools/Rules.mk since the new variable is set by
> configure.
>
> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
> ---
>  config/Tools.mk.in |   1 +
>  m4/golang.m4       |   4 ++
>  tools/Makefile     |   2 +-
>  tools/Rules.mk     |   2 -
>  tools/configure    | 138 +++++++++++++++++++++++++++++++++++++++++++++
>  tools/configure.ac |  12 ++++
>  6 files changed, 156 insertions(+), 3 deletions(-)
>  create mode 100644 m4/golang.m4
>
> diff --git a/config/Tools.mk.in b/config/Tools.mk.in
> index 189fda1596..2c219f5477 100644
> --- a/config/Tools.mk.in
> +++ b/config/Tools.mk.in
> @@ -55,6 +55,7 @@ CONFIG_QEMU_TRAD    := @qemu_traditional@
>  CONFIG_QEMU_XEN     := @qemu_xen@
>  CONFIG_QEMUU_EXTRA_ARGS:= @EXTRA_QEMUU_CONFIGURE_ARGS@
>  CONFIG_LIBNL        := @libnl@
> +CONFIG_GOLANG_TOOLS := @golang_tools@
>
>  CONFIG_SYSTEMD      := @systemd@
>  SYSTEMD_CFLAGS      := @SYSTEMD_CFLAGS@
> diff --git a/m4/golang.m4 b/m4/golang.m4
> new file mode 100644
> index 0000000000..0b4bd54ce0
> --- /dev/null
> +++ b/m4/golang.m4
> @@ -0,0 +1,4 @@
> +AC_DEFUN([AC_PROG_GO], [
> +    dnl Check for the go compiler
> +    AC_CHECK_TOOL([GO],[go],[no])
> +])
>

AFAICT this only checks for the existence of the binary.  Will the bindings
compile with all versions of go?  If not, should we try to check the
version here?

Thanks,
 -George
Nick Rosbrook April 23, 2020, 4:03 p.m. UTC | #5
On Thu, Apr 23, 2020 at 9:44 AM George Dunlap <dunlapg@umich.edu> wrote:
>
>
>
> On Thu, Apr 23, 2020 at 1:51 AM Nick Rosbrook <rosbrookn@gmail.com> wrote:
>>
>> By default, if the go compiler is found by the configure script, build
>> the golang tools. If the compiler is not found, and
>> --enable-golang_tools was not explicitly set, do not build to the golang
>> tools.
>>
>> The new corresponding make variable is CONFIG_GOLANG_TOOLS. Remove
>> CONFIG_GOLANG from tools/Rules.mk since the new variable is set by
>> configure.
>>
>> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
>> ---
>>  config/Tools.mk.in |   1 +
>>  m4/golang.m4       |   4 ++
>>  tools/Makefile     |   2 +-
>>  tools/Rules.mk     |   2 -
>>  tools/configure    | 138 +++++++++++++++++++++++++++++++++++++++++++++
>>  tools/configure.ac |  12 ++++
>>  6 files changed, 156 insertions(+), 3 deletions(-)
>>  create mode 100644 m4/golang.m4
>>
>> diff --git a/config/Tools.mk.in b/config/Tools.mk.in
>> index 189fda1596..2c219f5477 100644
>> --- a/config/Tools.mk.in
>> +++ b/config/Tools.mk.in
>> @@ -55,6 +55,7 @@ CONFIG_QEMU_TRAD    := @qemu_traditional@
>>  CONFIG_QEMU_XEN     := @qemu_xen@
>>  CONFIG_QEMUU_EXTRA_ARGS:= @EXTRA_QEMUU_CONFIGURE_ARGS@
>>  CONFIG_LIBNL        := @libnl@
>> +CONFIG_GOLANG_TOOLS := @golang_tools@
>>
>>  CONFIG_SYSTEMD      := @systemd@
>>  SYSTEMD_CFLAGS      := @SYSTEMD_CFLAGS@
>> diff --git a/m4/golang.m4 b/m4/golang.m4
>> new file mode 100644
>> index 0000000000..0b4bd54ce0
>> --- /dev/null
>> +++ b/m4/golang.m4
>> @@ -0,0 +1,4 @@
>> +AC_DEFUN([AC_PROG_GO], [
>> +    dnl Check for the go compiler
>> +    AC_CHECK_TOOL([GO],[go],[no])
>> +])
>
>
> AFAICT this only checks for the existence of the binary.  Will the bindings compile with all versions of go?  If not, should we try to check the version here?

There are no obvious pieces to me that won't compile on fairly recent
(i.e. >= 1.9) versions of go, but yes it would probably be best to
check for a minimum version. I will do some more work to figure out an
appropriate minimum version, but I think go 1.10 might be a reasonable
start.

Thanks,

-NR
Nick Rosbrook April 23, 2020, 4:06 p.m. UTC | #6
On Thu, Apr 23, 2020 at 6:25 AM Wei Liu <wl@xen.org> wrote:
>
> On Wed, Apr 22, 2020 at 08:25:25PM -0400, Nick Rosbrook wrote:
> > By default, if the go compiler is found by the configure script, build
> > the golang tools. If the compiler is not found, and
> > --enable-golang_tools was not explicitly set, do not build to the golang
>
> --enable-golang-tools here.
>
> > tools.
> >
> > The new corresponding make variable is CONFIG_GOLANG_TOOLS. Remove
> > CONFIG_GOLANG from tools/Rules.mk since the new variable is set by
> > configure.
> >
> > Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
>
> Acked-by: Wei Liu <wl@xen.org>

Thanks!

-NR
George Dunlap April 23, 2020, 5:27 p.m. UTC | #7
> On Apr 23, 2020, at 5:03 PM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> 
> On Thu, Apr 23, 2020 at 9:44 AM George Dunlap <dunlapg@umich.edu> wrote:
>> 
>> 
>> 
>> On Thu, Apr 23, 2020 at 1:51 AM Nick Rosbrook <rosbrookn@gmail.com> wrote:
>>> 
>>> By default, if the go compiler is found by the configure script, build
>>> the golang tools. If the compiler is not found, and
>>> --enable-golang_tools was not explicitly set, do not build to the golang
>>> tools.
>>> 
>>> The new corresponding make variable is CONFIG_GOLANG_TOOLS. Remove
>>> CONFIG_GOLANG from tools/Rules.mk since the new variable is set by
>>> configure.
>>> 
>>> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
>>> ---
>>> config/Tools.mk.in |   1 +
>>> m4/golang.m4       |   4 ++
>>> tools/Makefile     |   2 +-
>>> tools/Rules.mk     |   2 -
>>> tools/configure    | 138 +++++++++++++++++++++++++++++++++++++++++++++
>>> tools/configure.ac |  12 ++++
>>> 6 files changed, 156 insertions(+), 3 deletions(-)
>>> create mode 100644 m4/golang.m4
>>> 
>>> diff --git a/config/Tools.mk.in b/config/Tools.mk.in
>>> index 189fda1596..2c219f5477 100644
>>> --- a/config/Tools.mk.in
>>> +++ b/config/Tools.mk.in
>>> @@ -55,6 +55,7 @@ CONFIG_QEMU_TRAD    := @qemu_traditional@
>>> CONFIG_QEMU_XEN     := @qemu_xen@
>>> CONFIG_QEMUU_EXTRA_ARGS:= @EXTRA_QEMUU_CONFIGURE_ARGS@
>>> CONFIG_LIBNL        := @libnl@
>>> +CONFIG_GOLANG_TOOLS := @golang_tools@
>>> 
>>> CONFIG_SYSTEMD      := @systemd@
>>> SYSTEMD_CFLAGS      := @SYSTEMD_CFLAGS@
>>> diff --git a/m4/golang.m4 b/m4/golang.m4
>>> new file mode 100644
>>> index 0000000000..0b4bd54ce0
>>> --- /dev/null
>>> +++ b/m4/golang.m4
>>> @@ -0,0 +1,4 @@
>>> +AC_DEFUN([AC_PROG_GO], [
>>> +    dnl Check for the go compiler
>>> +    AC_CHECK_TOOL([GO],[go],[no])
>>> +])
>> 
>> 
>> AFAICT this only checks for the existence of the binary.  Will the bindings compile with all versions of go?  If not, should we try to check the version here?
> 
> There are no obvious pieces to me that won't compile on fairly recent
> (i.e. >= 1.9) versions of go, but yes it would probably be best to
> check for a minimum version. I will do some more work to figure out an
> appropriate minimum version, but I think go 1.10 might be a reasonable
> start.

OK — I didn’t mean you had to; I just thought it was worth bringing up.

The underscore thing though — I think it’s a bad idea to mix `-` and `_`; —enable-golang_tools just isn’t a good idea IMHO. :-)

Do you need _tools?  Could it just be —enable-golang?

 -George
Nick Rosbrook April 23, 2020, 5:36 p.m. UTC | #8
> The underscore thing though — I think it’s a bad idea to mix `-` and `_`; —enable-golang_tools just isn’t a good idea IMHO. :-)
>
> Do you need _tools?  Could it just be —enable-golang?

I agree, I don't like mixing the '-' and '_'. From what I could tell,
that was how the other options were named in tools/configure.ac and
Tools.mk.in (unless I missed a way to tell the --enable flag to just
use '-'), so I went with that. I can just do --enable-golang, but I
was trying to make it a bit more descriptive if possible.

-NR
diff mbox series

Patch

diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index 189fda1596..2c219f5477 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -55,6 +55,7 @@  CONFIG_QEMU_TRAD    := @qemu_traditional@
 CONFIG_QEMU_XEN     := @qemu_xen@
 CONFIG_QEMUU_EXTRA_ARGS:= @EXTRA_QEMUU_CONFIGURE_ARGS@
 CONFIG_LIBNL        := @libnl@
+CONFIG_GOLANG_TOOLS := @golang_tools@
 
 CONFIG_SYSTEMD      := @systemd@
 SYSTEMD_CFLAGS      := @SYSTEMD_CFLAGS@
diff --git a/m4/golang.m4 b/m4/golang.m4
new file mode 100644
index 0000000000..0b4bd54ce0
--- /dev/null
+++ b/m4/golang.m4
@@ -0,0 +1,4 @@ 
+AC_DEFUN([AC_PROG_GO], [
+    dnl Check for the go compiler
+    AC_CHECK_TOOL([GO],[go],[no])
+])
diff --git a/tools/Makefile b/tools/Makefile
index c10946e3b1..81be8db757 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -31,7 +31,7 @@  endif
 
 SUBDIRS-y += xenpmd
 SUBDIRS-y += libxl
-SUBDIRS-$(CONFIG_GOLANG) += golang
+SUBDIRS-$(CONFIG_GOLANG_TOOLS) += golang
 SUBDIRS-y += xl
 SUBDIRS-y += helpers
 SUBDIRS-$(CONFIG_X86) += xenpaging
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 9bac15c8d1..5b8cf748ad 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -35,8 +35,6 @@  XENSTORE_XENSTORED ?= y
 debug ?= y
 debug_symbols ?= $(debug)
 
-# Set CONFIG_GOLANG=y in .config (or in make) to build golang
-CONFIG_GOLANG ?= n
 XEN_GOPATH        = $(XEN_ROOT)/tools/golang
 XEN_GOCODE_URL    = golang.xenproject.org
 
diff --git a/tools/configure b/tools/configure
index 4fa5f7b937..a4199573fd 100755
--- a/tools/configure
+++ b/tools/configure
@@ -664,6 +664,7 @@  pyconfig
 PYTHONPATH
 CHECKPOLICY
 XENSTORED
+GO
 OCAMLFIND
 OCAMLBUILD
 OCAMLDOC
@@ -705,6 +706,7 @@  LD86
 AS86
 qemu_traditional
 LINUX_BACKEND_MODULES
+golang_tools
 seabios
 ovmf
 xsmpolicy
@@ -807,6 +809,7 @@  enable_ocamltools
 enable_xsmpolicy
 enable_ovmf
 enable_seabios
+enable_golang_tools
 with_linux_backend_modules
 enable_qemu_traditional
 enable_rombios
@@ -1493,6 +1496,7 @@  Optional Features:
   --disable-xsmpolicy     Disable XSM policy compilation (default is ENABLED)
   --enable-ovmf           Enable OVMF (default is DISABLED)
   --disable-seabios       Disable SeaBIOS (default is ENABLED)
+  --disable-golang_tools  Disable Go tools (default is ENABLED)
   --enable-qemu-traditional
                           Enable qemu traditional device model, (DEFAULT is on
                           for Linux or NetBSD x86, otherwise off)
@@ -3870,6 +3874,8 @@  esac
 
 
 
+
+
 
 
 
@@ -4200,6 +4206,29 @@  seabios=$ax_cv_seabios
 
 
 
+# Check whether --enable-golang_tools was given.
+if test "${enable_golang_tools+set}" = set; then :
+  enableval=$enable_golang_tools;
+fi
+
+
+if test "x$enable_golang_tools" = "xno"; then :
+
+    ax_cv_golang_tools="n"
+
+elif test "x$enable_golang_tools" = "xyes"; then :
+
+    ax_cv_golang_tools="y"
+
+elif test -z $ax_cv_golang_tools; then :
+
+    ax_cv_golang_tools="y"
+
+fi
+golang_tools=$ax_cv_golang_tools
+
+
+
 
 # Check whether --with-linux-backend-modules was given.
 if test "${with_linux_backend_modules+set}" = set; then :
@@ -6694,6 +6723,115 @@  fi
 
 fi
 
+if test "x$golang_tools" = "xy"; then :
+
+
+        if test -n "$ac_tool_prefix"; then
+  # Extract the first word of "${ac_tool_prefix}go", so it can be a program name with args.
+set dummy ${ac_tool_prefix}go; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_GO+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  if test -n "$GO"; then
+  ac_cv_prog_GO="$GO" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+    for ac_exec_ext in '' $ac_executable_extensions; do
+  if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+    ac_cv_prog_GO="${ac_tool_prefix}go"
+    $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+  done
+IFS=$as_save_IFS
+
+fi
+fi
+GO=$ac_cv_prog_GO
+if test -n "$GO"; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $GO" >&5
+$as_echo "$GO" >&6; }
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+fi
+if test -z "$ac_cv_prog_GO"; then
+  ac_ct_GO=$GO
+  # Extract the first word of "go", so it can be a program name with args.
+set dummy go; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_prog_ac_ct_GO+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  if test -n "$ac_ct_GO"; then
+  ac_cv_prog_ac_ct_GO="$ac_ct_GO" # Let the user override the test.
+else
+as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+  IFS=$as_save_IFS
+  test -z "$as_dir" && as_dir=.
+    for ac_exec_ext in '' $ac_executable_extensions; do
+  if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+    ac_cv_prog_ac_ct_GO="go"
+    $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+    break 2
+  fi
+done
+  done
+IFS=$as_save_IFS
+
+fi
+fi
+ac_ct_GO=$ac_cv_prog_ac_ct_GO
+if test -n "$ac_ct_GO"; then
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_ct_GO" >&5
+$as_echo "$ac_ct_GO" >&6; }
+else
+  { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+  if test "x$ac_ct_GO" = x; then
+    GO="no"
+  else
+    case $cross_compiling:$ac_tool_warned in
+yes:)
+{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: using cross tools not prefixed with host triplet" >&5
+$as_echo "$as_me: WARNING: using cross tools not prefixed with host triplet" >&2;}
+ac_tool_warned=yes ;;
+esac
+    GO=$ac_ct_GO
+  fi
+else
+  GO="$ac_cv_prog_GO"
+fi
+
+
+    if test "x$GO" = "xno"; then :
+
+        if test "x$enable_golang_tools" =  "xyes"; then :
+
+            as_fn_error $? "Go tools enabled, but missing go compiler" "$LINENO" 5
+
+fi
+        golang_tools="n"
+
+fi
+
+fi
+
 
 
 
diff --git a/tools/configure.ac b/tools/configure.ac
index ea0272766f..69e0894638 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -73,6 +73,7 @@  m4_include([../m4/fetcher.m4])
 m4_include([../m4/ax_compare_version.m4])
 m4_include([../m4/paths.m4])
 m4_include([../m4/systemd.m4])
+m4_include([../m4/golang.m4])
 
 AX_XEN_EXPAND_CONFIG()
 
@@ -84,6 +85,7 @@  AX_ARG_DEFAULT_ENABLE([ocamltools], [Disable Ocaml tools])
 AX_ARG_DEFAULT_ENABLE([xsmpolicy], [Disable XSM policy compilation])
 AX_ARG_DEFAULT_DISABLE([ovmf], [Enable OVMF])
 AX_ARG_DEFAULT_ENABLE([seabios], [Disable SeaBIOS])
+AX_ARG_DEFAULT_ENABLE([golang_tools], [Disable Go tools])
 
 AC_ARG_WITH([linux-backend-modules],
     AS_HELP_STRING([--with-linux-backend-modules="mod1 mod2"],
@@ -320,6 +322,16 @@  AS_IF([test "x$ocamltools" = "xy"], [
     ])
 ])
 
+AS_IF([test "x$golang_tools" = "xy"], [
+    AC_PROG_GO
+    AS_IF([test "x$GO" = "xno"], [
+        AS_IF([test "x$enable_golang_tools" =  "xyes"], [
+            AC_MSG_ERROR([Go tools enabled, but missing go compiler])
+        ])
+        golang_tools="n"
+    ])
+])
+
 m4_include([../m4/xenstored.m4])
 AX_XENSTORE_OPTIONS
 AX_XENSTORE_SET