[PATCH 1/2] golang/xenlight: do not hard code libxl dir in gengotypes.py

[PATCH 1/2] golang/xenlight: do not hard code libxl dir in gengotypes.py

[Nick Rosbrook]

Currently, in order to 'import idl' in gengotypes.py, we derive the path
of the libxl source directory from the XEN_ROOT environment variable, and
append that to sys.path so python can see idl.py. Since the the recent move of
libxl to tools/libs/light, this hard coding breaks the build.

Instead, check for the environment variable LIBXL_SRC_DIR, but move this
check to a try-except block (with empty except). This simply makes the
real error more visible, and does not strictly require that
LIBXL_SRC_DIR is used. Finally, update the Makefile to set LIBXL_SRC_DIR
rather than XEN_ROOT when calling gengotypes.py.

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/Makefile      | 2 +-
 tools/golang/xenlight/gengotypes.py | 9 ++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
index fd8e4893db..e394ef9b2b 100644
--- a/tools/golang/xenlight/Makefile
+++ b/tools/golang/xenlight/Makefile
@@ -16,7 +16,7 @@ all: build
 GOXL_GEN_FILES = types.gen.go helpers.gen.go
 
 %.gen.go: gengotypes.py $(LIBXL_SRC_DIR)/libxl_types.idl $(LIBXL_SRC_DIR)/idl.py
-	XEN_ROOT=$(XEN_ROOT) $(PYTHON) gengotypes.py $(LIBXL_SRC_DIR)/libxl_types.idl
+	LIBXL_SRC_DIR=$(LIBXL_SRC_DIR) $(PYTHON) gengotypes.py $(LIBXL_SRC_DIR)/libxl_types.idl
 
 # Go will do its own dependency checking, and not actuall go through
 # with the build if none of the input files have changed.
diff --git a/tools/golang/xenlight/gengotypes.py b/tools/golang/xenlight/gengotypes.py
index ebec938224..4ac181ae47 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -3,7 +3,14 @@
 import os
 import sys
 
-sys.path.append('{0}/tools/libxl'.format(os.environ['XEN_ROOT']))
+try:
+    sys.path.append(os.environ['LIBXL_SRC_DIR'])
+except:
+    # If we get here, then we expect the 'import idl'
+    # expression to fail. That error is more informative,
+    # so let it happen.
+    pass
+
 import idl
 
 # Go versions of some builtin types.
-- 
2.17.1

[PATCH 2/2] golang/xenlight: standardize generated code comment

[Nick Rosbrook]

There is a standard format for generated Go code header comments, as set
by [1]. Modify gengotypes.py to follow this standard, and use the
additional

  // source: <IDL file basename>

convention used by protoc-gen-go.

This change is motivated by the fact that since 41aea82de2, the comment
would include the absolute path to libxl_types.idl, therefore creating
unintended diffs when generating code across different machines. This
approach fixes that problem.

[1] https://github.com/golang/go/issues/13560

Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/golang/xenlight/gengotypes.py  | 10 ++++------
 tools/golang/xenlight/helpers.gen.go |  7 ++-----
 tools/golang/xenlight/types.gen.go   |  7 ++-----
 3 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/tools/golang/xenlight/gengotypes.py b/tools/golang/xenlight/gengotypes.py
index 4ac181ae47..3e40c3d5dc 100644
--- a/tools/golang/xenlight/gengotypes.py
+++ b/tools/golang/xenlight/gengotypes.py
@@ -731,13 +731,11 @@ if __name__ == '__main__':
         name = b.typename
         builtin_type_names[name] = xenlight_golang_fmt_name(name)
 
-    header_comment="""// DO NOT EDIT.
-//
-// This file is generated by:
-// {0}
-//
+    header_comment="""// Code generated by {}. DO NOT EDIT.
+// source: {}
 
-""".format(' '.join(sys.argv))
+""".format(os.path.basename(sys.argv[0]),
+           ' '.join([os.path.basename(a) for a in sys.argv[1:]]))
 
     xenlight_golang_generate_types(types=types,
                                    comment=header_comment)
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index 152c7e8e6b..c8605994e7 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1,8 +1,5 @@
-// DO NOT EDIT.
-//
-// This file is generated by:
-// gengotypes.py ../../libxl/libxl_types.idl
-//
+// Code generated by gengotypes.py. DO NOT EDIT.
+// source: libxl_types.idl
 
 package xenlight
 
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index 663c1e86b4..b4c5df0f2c 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -1,8 +1,5 @@
-// DO NOT EDIT.
-//
-// This file is generated by:
-// gengotypes.py ../../libxl/libxl_types.idl
-//
+// Code generated by gengotypes.py. DO NOT EDIT.
+// source: libxl_types.idl
 
 package xenlight
 
-- 
2.17.1

Re: [PATCH 1/2] golang/xenlight: do not hard code libxl dir in gengotypes.py

[George Dunlap]

> On Oct 12, 2020, at 12:31 AM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> 
> Currently, in order to 'import idl' in gengotypes.py, we derive the path
> of the libxl source directory from the XEN_ROOT environment variable, and
> append that to sys.path so python can see idl.py. Since the the recent move of
> libxl to tools/libs/light, this hard coding breaks the build.
> 
> Instead, check for the environment variable LIBXL_SRC_DIR, but move this
> check to a try-except block (with empty except). This simply makes the
> real error more visible, and does not strictly require that
> LIBXL_SRC_DIR is used. Finally, update the Makefile to set LIBXL_SRC_DIR
> rather than XEN_ROOT when calling gengotypes.py.
> 
> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>

Re: [PATCH 2/2] golang/xenlight: standardize generated code comment

[George Dunlap]

> On Oct 12, 2020, at 12:31 AM, Nick Rosbrook <rosbrookn@gmail.com> wrote:
> 
> There is a standard format for generated Go code header comments, as set
> by [1]. Modify gengotypes.py to follow this standard, and use the
> additional
> 
>  // source: <IDL file basename>
> 
> convention used by protoc-gen-go.
> 
> This change is motivated by the fact that since 41aea82de2, the comment
> would include the absolute path to libxl_types.idl, therefore creating
> unintended diffs when generating code across different machines. This
> approach fixes that problem.
> 
> [1] https://github.com/golang/go/issues/13560
> 
> Signed-off-by: Nick Rosbrook <rosbrookn@ainfosec.com>

Reviewed-by: George Dunlap <george.dunlap@citrix.com>