Improve build-ui Makefile target and fix dependencies (#2573)

* Add explicit dep of build-ui with jaeger-query Signed-off-by: albertteoh <[email protected]> * Add back explanation of build-ui and cleanup Makefile Signed-off-by: albertteoh <[email protected]> * Refactor Makefile to use file targets Signed-off-by: albertteoh <[email protected]> * Clean build dir Signed-off-by: albertteoh <[email protected]> * Add require dependency Signed-off-by: albertteoh <[email protected]> * Remove build-ui explanation from Makefile Signed-off-by: albertteoh <[email protected]>
jaegertracing · Oct 20, 2020 · 3d99871 · 3d99871
1 parent bd59f13
commit 3d99871
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 10 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -27,11 +27,14 @@ cd jaeger
 Then install dependencies and run the tests:
 
 ```
+# Adds the jaeger-ui submodule
 git submodule update --init --recursive
+
+# Installs required tools
 make install-tools
+
+# Runs all unit tests
 make test
-# if you wish to build platform binaries locally - the step below is needed.
-make build-ui
 ```
 
 ### Running local build with the UI
@@ -40,6 +43,23 @@ make build-ui
 $ make run-all-in-one
 ```
 
+#### What does this command do?
+
+The `jaeger-ui` submodule, which was added from the Pre-requisites step above, contains
+the source code for the UI assets (requires Node.js 6+).
+
+The assets must be compiled first with `make build-ui`, which runs Node.js build and then
+packages the assets into a Go file that is `.gitignore`-ed.
+
+The packaged assets can be enabled by providing a build tag `ui`, for example:
+
+```
+$ go run -tags ui ./cmd/all-in-one/main.go
+```
+
+`make run-all-in-one` essentially runs Jaeger all-in-one by combining both of the above
+steps into a single `make` command.
+
 ## Project Structure
 
 These are general guidelines on how to organize source code in this repository.

diff --git a/Makefile b/Makefile
@@ -87,7 +87,8 @@ md-to-godoc-gen:
 
 .PHONY: clean
 clean:
-	rm -rf cover.out .cover/ cover.html lint.log fmt.log
+	rm -rf cover.out .cover/ cover.html lint.log fmt.log \
+		jaeger-ui/packages/jaeger-ui/build
 
 .PHONY: test
 test: go-gen test-otel
@@ -222,29 +223,36 @@ docker-hotrod:
 	docker build -t $(DOCKER_NAMESPACE)/example-hotrod:${DOCKER_TAG} ./examples/hotrod --build-arg TARGETARCH=$(GOARCH)
 
 .PHONY: run-all-in-one
-run-all-in-one:
+run-all-in-one: build-ui
 	go run -tags ui ./cmd/all-in-one --log-level debug
 
 .PHONY: build-ui
-build-ui:
+build-ui: cmd/query/app/ui/actual/gen_assets.go cmd/query/app/ui/placeholder/gen_assets.go
+	# Do nothing. If you need to force a rebuild of UI assets, run `make clean`.
+
+jaeger-ui/packages/jaeger-ui/build/index.html:
 	cd jaeger-ui && yarn install --frozen-lockfile && cd packages/jaeger-ui && yarn build
+
+cmd/query/app/ui/actual/gen_assets.go: jaeger-ui/packages/jaeger-ui/build/index.html
 	esc -pkg assets -o cmd/query/app/ui/actual/gen_assets.go -prefix jaeger-ui/packages/jaeger-ui/build jaeger-ui/packages/jaeger-ui/build
+
+cmd/query/app/ui/placeholder/gen_assets.go: cmd/query/app/ui/placeholder/public/index.html
 	esc -pkg assets -o cmd/query/app/ui/placeholder/gen_assets.go -prefix cmd/query/app/ui/placeholder/public cmd/query/app/ui/placeholder/public
 
 .PHONY: build-all-in-one-linux
-build-all-in-one-linux: build-ui
+build-all-in-one-linux:
 	GOOS=linux $(MAKE) build-all-in-one
 
 .PHONY: build-all-in-one
-build-all-in-one: elasticsearch-mappings
+build-all-in-one: build-ui elasticsearch-mappings
 	$(GOBUILD) -tags ui -o ./cmd/all-in-one/all-in-one-$(GOOS)-$(GOARCH) $(BUILD_INFO) ./cmd/all-in-one/main.go
 
 .PHONY: build-agent
 build-agent:
 	$(GOBUILD) -o ./cmd/agent/agent-$(GOOS)-$(GOARCH) $(BUILD_INFO) ./cmd/agent/main.go
 
 .PHONY: build-query
-build-query:
+build-query: build-ui
 	$(GOBUILD) -tags ui -o ./cmd/query/query-$(GOOS)-$(GOARCH) $(BUILD_INFO) ./cmd/query/main.go
 
 .PHONY: build-collector
@@ -264,15 +272,15 @@ build-otel-ingester:
 	cd ${OTEL_COLLECTOR_DIR}/cmd/ingester && $(GOBUILD) -o ./opentelemetry-ingester-$(GOOS)-$(GOARCH) $(BUILD_INFO) main.go
 
 .PHONY: build-otel-all-in-one
-build-otel-all-in-one:
+build-otel-all-in-one: build-ui
 	cd ${OTEL_COLLECTOR_DIR}/cmd/all-in-one && $(GOBUILD) -tags ui -o ./opentelemetry-all-in-one-$(GOOS)-$(GOARCH) $(BUILD_INFO) main.go
 
 .PHONY: build-ingester
 build-ingester:
 	$(GOBUILD) -o ./cmd/ingester/ingester-$(GOOS)-$(GOARCH) $(BUILD_INFO) ./cmd/ingester/main.go
 
 .PHONY: docker
-docker: build-ui build-binaries-linux docker-images-only
+docker: build-binaries-linux docker-images-only
 
 .PHONY: build-binaries-linux
 build-binaries-linux:
@@ -354,6 +362,8 @@ include crossdock/rules.mk
 # Crossdock tests do not require fully functioning UI, so we skip it to speed up the build.
 .PHONY: build-crossdock-ui-placeholder
 build-crossdock-ui-placeholder:
+	mkdir -p jaeger-ui/packages/jaeger-ui/build/
+	cp cmd/query/app/ui/placeholder/public/index.html jaeger-ui/packages/jaeger-ui/build/index.html
 	mkdir -p cmd/query/app/ui/actual
 	[ -e cmd/query/app/ui/actual/gen_assets.go ] || cp cmd/query/app/ui/placeholder/gen_assets.go cmd/query/app/ui/actual/gen_assets.go