diff mbox series

[v2,2/5] golang/xenlight: Get rid of GOPATH-based build artefacts

Message ID 20200526221612.900922-3-george.dunlap@citrix.com (mailing list archive)
State New, archived
Headers show
Series Golang build fixes / improvements | expand

Commit Message

George Dunlap May 26, 2020, 10:16 p.m. UTC 
The original build setup used a "fake GOPATH" in tools/golang to test
the mechanism of building from go package files installed on a
filesystem.  With the move to modules, this isn't necessary, and leads
to potentially confusing directories being created.  (I.e., it might
not be obvious that files under tools/golang/src shouldn't be edited.)

Get rid of the code that creates this (now unused) intermediate
directory.  Add direct dependencies from 'build' onto the source
files.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
---
v2:
- New

CC: Ian Jackson <ian.jackson@citrix.com>
CC: Nick Rosbrook <rosbrookn@ainfosec.com>
---
 tools/Rules.mk                 |  1 -
 tools/golang/Makefile          | 10 ----------
 tools/golang/xenlight/Makefile | 19 ++++---------------
 3 files changed, 4 insertions(+), 26 deletions(-)

Comments

Nick Rosbrook May 26, 2020, 11:57 p.m. UTC | #1 
> diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
> index 8ab4cb5665..f8d8047524 100644
> --- a/tools/golang/xenlight/Makefile
> +++ b/tools/golang/xenlight/Makefile
> @@ -14,17 +14,8 @@ LIBXL_SRC_DIR = ../../libxl
>  .PHONY: all
>  all: build
>  
> -.PHONY: package
> -package: $(XEN_GOPATH)$(GOXL_PKG_DIR)
> -
>  GOXL_GEN_FILES = types.gen.go helpers.gen.go
>  
> -$(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: xenlight.go $(GOXL_GEN_FILES)
> -	$(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR)
> -	$(INSTALL_DATA) xenlight.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
> -	$(INSTALL_DATA) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
> -	$(INSTALL_DATA) helpers.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
> -

I think the GOXL_PKG_DIR variable can be removed all together too. With
these changes it's only used to initialize GOXL_INSTALL_DIR.

-NR
Nick Rosbrook May 27, 2020, 12:03 a.m. UTC | #2 
On Tue, May 26, 2020 at 11:16:09PM +0100, George Dunlap wrote:
> The original build setup used a "fake GOPATH" in tools/golang to test
> the mechanism of building from go package files installed on a
> filesystem.  With the move to modules, this isn't necessary, and leads
> to potentially confusing directories being created.  (I.e., it might
> not be obvious that files under tools/golang/src shouldn't be edited.)
> 
> Get rid of the code that creates this (now unused) intermediate
> directory.  Add direct dependencies from 'build' onto the source
> files.
> 
> Signed-off-by: George Dunlap <george.dunlap@citrix.com>

If you want to just make that change on check-in,

Reviewed-by: Nick Rosbrook <rosbrookn@ainfosec.com>
diff mbox series

Patch

diff --git a/tools/Rules.mk b/tools/Rules.mk
index 59c72e7a88..76acaef988 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -35,7 +35,6 @@  XENSTORE_XENSTORED ?= y
 debug ?= y
 debug_symbols ?= $(debug)
 
-XEN_GOPATH        = $(XEN_ROOT)/tools/golang
 XEN_GOCODE_URL    = golang.xenproject.org
 
 ifeq ($(debug_symbols),y)
diff --git a/tools/golang/Makefile b/tools/golang/Makefile
index aba11ebc39..b022e2c5a3 100644
--- a/tools/golang/Makefile
+++ b/tools/golang/Makefile
@@ -1,16 +1,6 @@ 
 XEN_ROOT=$(CURDIR)/../..
 include $(XEN_ROOT)/tools/Rules.mk
 
-# In order to link against a package in Go, the package must live in a
-# directory tree in the way that Go expects.  To make this possible,
-# there must be a directory such that we can set GOPATH=${dir}, and
-# the package will be under $GOPATH/src/${full-package-path}.
-
-# So we set XEN_GOPATH to $XEN_ROOT/tools/golang.  The xenlight
-# "package build" directory ($PWD/xenlight) will create the "package
-# source" directory in the proper place.  Go programs can use this
-# package by setting GOPATH=$(XEN_GOPATH).
-
 SUBDIRS-y = xenlight
 
 .PHONY: build all
diff --git a/tools/golang/xenlight/Makefile b/tools/golang/xenlight/Makefile
index 8ab4cb5665..f8d8047524 100644
--- a/tools/golang/xenlight/Makefile
+++ b/tools/golang/xenlight/Makefile
@@ -14,17 +14,8 @@  LIBXL_SRC_DIR = ../../libxl
 .PHONY: all
 all: build
 
-.PHONY: package
-package: $(XEN_GOPATH)$(GOXL_PKG_DIR)
-
 GOXL_GEN_FILES = types.gen.go helpers.gen.go
 
-$(XEN_GOPATH)/src/$(XEN_GOCODE_URL)/xenlight/: xenlight.go $(GOXL_GEN_FILES)
-	$(INSTALL_DIR) $(XEN_GOPATH)$(GOXL_PKG_DIR)
-	$(INSTALL_DATA) xenlight.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
-	$(INSTALL_DATA) types.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
-	$(INSTALL_DATA) helpers.gen.go $(XEN_GOPATH)$(GOXL_PKG_DIR)
-
 # NOTE: This target is called from libxl/Makefile:all.  Since that
 # target must finish before golang/Makefile is called, this is
 # currently safe.  It must not be called from anywhere else in the
@@ -43,23 +34,21 @@  idl-gen: $(GOXL_GEN_FILES)
 # in the LDFLAGS; and thus we need to add -L$(XEN_XENLIGHT) here
 # so that it can find the actual library.
 .PHONY: build
-build: package
+build: xenlight.go $(GOXL_GEN_FILES)
 	CGO_CFLAGS="$(CFLAGS_libxenlight) $(CFLAGS_libxentoollog)" CGO_LDFLAGS="$(LDLIBS_libxenlight) $(LDLIBS_libxentoollog) -L$(XEN_XENLIGHT) -L$(XEN_LIBXENTOOLLOG)" $(GO) build -x
 
 .PHONY: install
 install: build
 	$(INSTALL_DIR) $(DESTDIR)$(GOXL_INSTALL_DIR)
-	$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)xenlight.go $(DESTDIR)$(GOXL_INSTALL_DIR)
-	$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)types.gen.go $(DESTDIR)$(GOXL_INSTALL_DIR)
-	$(INSTALL_DATA) $(XEN_GOPATH)$(GOXL_PKG_DIR)helpers.gen.go $(DESTDIR)$(GOXL_INSTALL_DIR)
+	$(INSTALL_DATA) xenlight.go $(DESTDIR)$(GOXL_INSTALL_DIR)
+	$(INSTALL_DATA) types.gen.go $(DESTDIR)$(GOXL_INSTALL_DIR)
+	$(INSTALL_DATA) helpers.gen.go $(DESTDIR)$(GOXL_INSTALL_DIR)
 
 .PHONY: uninstall
 	rm -rf $(DESTDIR)$(GOXL_INSTALL_DIR)
 
 .PHONY: clean
 clean:
-	$(RM) -r $(XEN_GOPATH)$(GOXL_PKG_DIR)
-	$(RM) $(XEN_GOPATH)/pkg/*/$(XEN_GOCODE_URL)/xenlight.a
 
 .PHONY: distclean
 distclean: clean