rethink external executable setup

Change-Id: I5293fea9b5404b82e72761407d325c408a2e45ca

5 files changed, 128 insertions, 47 deletions

diff --git a/solenv/gbuild/ComponentTarget.mk b/solenv/gbuild/ComponentTarget.mk
index 1b77e0eb76ff..866b07d3038c 100644
--- a/solenv/gbuild/ComponentTarget.mk
+++ b/solenv/gbuild/ComponentTarget.mk
@@ -53,7 +53,7 @@ $(call gb_ComponentTarget_get_clean_target,%) :
 
 $(call gb_ComponentTarget_get_target,%) : \
 		$(call gb_ComponentTarget_get_source,%) \
-		| $(call gb_ExternalExecutable_get_deps,xsltproc)
+		| $(call gb_ExternalExecutable_get_dependencies,xsltproc)
 	$(call gb_ComponentTarget__command,$@,$<,$*)
 
 # the .dir is for make 3.81, which ignores trailing /
diff --git a/solenv/gbuild/Configuration.mk b/solenv/gbuild/Configuration.mk
index 59e8a9652af6..db65dc582e5b 100644
--- a/solenv/gbuild/Configuration.mk
+++ b/solenv/gbuild/Configuration.mk
@@ -58,7 +58,7 @@ gb_Configuration__get_source = $(SRCDIR)/$(2)
 gb_Configuration_PRIMARY_REGISTRY_NAME := registry
 
 gb_Configuration_XSLTCOMMAND = $(call gb_ExternalExecutable_get_command,xsltproc)
-gb_Configuration_XSLTCOMMAND_DEPS = $(call gb_ExternalExecutable_get_deps,xsltproc)
+gb_Configuration_XSLTCOMMAND_DEPS = $(call gb_ExternalExecutable_get_dependencies,xsltproc)
 
 # XcsTarget class
 
diff --git a/solenv/gbuild/ExternalExecutable.mk b/solenv/gbuild/ExternalExecutable.mk
index bbdc190bfa8e..c62b4c6d407a 100644
--- a/solenv/gbuild/ExternalExecutable.mk
+++ b/solenv/gbuild/ExternalExecutable.mk
@@ -12,38 +12,42 @@
 # ExternalExecutable is a little helper for using executables that might
 # either come from system or be built internally.
 #
-# The configuration for an external executable is done in
-# RepositoryExternal.mk by defining function
-# gb_ExternalExecutable__register_EXECUTABLE, which should set up to 4
-# variables. The variables are (presuming the executable is named foo):
-
-# * gb_ExternalExecutable__foo_COMMAND
-#   Defines the complete command for the executable. This must include
-#   shell variables, if required. Defaults to
-#   $(gb_ExternalExecutable__foo_PRECOMMAND) $(ICECREAM_RUN) $(gb_ExternalExecutable__foo_TARGET)
-#   (if gb_ExternalExecutable__foo_TARGET is not defined, plain "foo" is substituted).
-#
-# * gb_ExternalExecutable__foo_DEPS
-#   The complete dependencies needed for running the executable. Only
-#   useful for internally built executables to make sure all data,
-#   config. files etc. are present. Defaults to
-#   $(gb_ExternalExecutable__foo_TARGET).
-#
-# * gb_ExternalExecutable__foo_PRECOMMAND
-#   Any variables that need to be set for running the executable.
-#   Typical example is use of $(gb_Helper_set_ld_path) for internally
-#   built executables.
-#
-# * gb_ExternalExecutable__foo_TARGET
-#   A full path to the executable. Typical definition for internally built
-#   executable is $(call gb_Executable_get_target_for_build,foo).
-#
-# Because of the particular choice of the defaults, there is typically
-# nothing that needs to be set for a system executable. 
+# === Setup ===
+#
+# An ExternalExecutable command consists of 4 parts:
+# * precommand: any command line variables that need to be set
+# * internal: unspecified command(s), possibly including calls of gdb,
+#   valgrind or icerun
+# * executable: the executable, with or without path
+# * arguments: command line arguments that are specific for either
+#   external or internal call, or that are common for _all_ uses of the
+#   executable
+#
+# The configuration is done in RepositoryExternal.mk by defining function
+# gb_ExternalExecutable__register_EXECUTABLE, which can call up to 4
+# functions:
+# * gb_ExternalExecutable_set_external / gb_ExternalExecutable_set_internal
+# * gb_ExternalExecutable_set_precommand
+# * gb_ExternalExecutable_add_dependencies
+# * gb_ExternalExecutable_add_arguments.
+# If neither gb_ExternalExecutable_set_external nor
+# gb_ExternalExecutable_set_internal is used, the executable defaults to
+# the ExternalExecutable's name. Due to that, nothing needs to be set
+# for an external executable in the typical case.
 #
 # All external executables must be registered (by listing the executable
 # name in gb_ExternalExecutable_register_executables call). This is done in
 # Repository.mk .
+#
+# === Usage ===
+#
+# The call site(s) should always use both of the following functions:
+# * gb_ExternalExecutable_get_command: the complete command for the
+#   executable
+# * gb_ExternalExecutable_get_dependencies: all run-time dependencies
+#   needed by the command.
+
+## Infrastructure functions
 
 # The list of registered executables.
 gb_ExternalExecutable_REGISTERED_EXECUTABLES :=
@@ -78,7 +82,9 @@ endef
 #
 # gb_ExternalExecutable_collect_registrations
 define gb_ExternalExecutable_collect_registrations
-$(foreach executable,$(gb_ExternalExecutable_REGISTERED_EXECUTABLES),$(call gb_ExternalExecutable__process_registration,$(executable)))
+$(eval $(foreach executable,$(gb_ExternalExecutable_REGISTERED_EXECUTABLES),\
+	$(call gb_ExternalExecutable__process_registration,$(executable)))
+)
 
 endef
 
@@ -89,13 +95,91 @@ $(if $(filter $(1),$(gb_ExternalExecutable_REGISTERED_EXECUTABLES)),,\
 
 endef
 
+## Setup functions
+
+# Set the executable as external
+#
+# Optionally set a specific executable call to use.
+# Example:
+# 	$(call gb_ExternalExecutable_set_external,python,$(PYTHON))
+#
+# gb_ExternalExecutable_set_external executable call?
+define gb_ExternalExecutable_set_external
+$(if $(2),gb_ExternalExecutable_$(1)_EXECUTABLE := $(2))
+
+endef
+
+define gb_ExternalExecutable__set_internal
+gb_ExternalExecutable_$(1)_EXECUTABLE := $(2)
+gb_ExternalExecutable_$(1)_DEPENDENCIES := $(2)
+gb_ExternalExecutable_$(1)_PRECOMMAND := $(gb_Helper_set_ld_path)
+
+endef
+
+# Set the executable as internal
+#
+# Optionally set a specific executable target to use (if the target
+# returned by gb_Executable_get_target_for_build is not suitable).
+#
+# gb_ExternalExecutable_set_internal executable call?
+define gb_ExternalExecutable_set_internal
+$(call gb_ExternalExecutable__set_internal,$(1),$(if $(strip $(2)),$(2),$(call gb_Executable_get_target_for_build,$(1))))
+
+endef
+
+# Set pre-command for the executable
+#
+# This call should set any command line variables needed for the
+# executable to run.
+#
+# gb_ExternalExecutable_set_precommand executable precommand
+define gb_ExternalExecutable_set_precommand
+gb_ExternalExecutable_$(1)_PRECOMMAND := $(2)
+
+endef
+
+# Add dependencies needed for running the executable
+#
+# Note that the dependencies should in most (if not all) cases be
+# _for_build targets, or there might be problems in cross-compilation
+# Specifically, not using _for_build target would mean either:
+# * the target is built before the command even if it is not necessary
+#   (not really a problem, but might be a nuisance)
+# * the build breaks because the target is not known. This might happen
+#   if there is a difference in configuration between build and host
+#   phases.
+#
+# gb_ExternalExecutable_add_dependencies executable dependencies
+define gb_ExternalExecutable_add_dependencies
+gb_ExternalExecutable_$(1)_DEPENDENCIES += $(2)
+
+endef
+
+# Add arguments needed for running the executable
+#
+# This should only contain arguments that differ between external and
+# internal executable call or that are common for all call sites.
+#
+# gb_ExternalExecutable_add_arguments executable arguments
+define gb_ExternalExecutable_add_arguments
+gb_ExternalExecutable_$(1)_ARGUMENTS += $(2)
+
+endef
+
+## User functions
+
+gb_ExternalExecutable__get_internal := $(ICECREAM_RUN)
+
+define gb_ExternalExecutable__get_executable
+$(if $(gb_ExternalExecutable_$(1)_EXECUTABLE),$(gb_ExternalExecutable_$(1)_EXECUTABLE),$(1))
+endef
+
 define gb_ExternalExecutable__get_command
 $(call gb_ExternalExecutale__check_registration,$(1))
-$(if $(filter undefined,$(origin gb_ExternalExecutable__$(1)_COMMAND)) \
-    ,$(gb_ExternalExecutable__$(1)_PRECOMMAND) $(ICECREAM_RUN) \
-    	$(if $(gb_ExternalExecutable__$(1)_TARGET),$(gb_ExternalExecutable__$(1)_TARGET),$(1)) \
-    ,$(gb_ExternalExecutable__$(1)_COMMAND) \
-)
+$(gb_ExternalExecutable_$(1)_PRECOMMAND) \
+	$(call gb_ExternalExecutable__get_internal,$(1)) \
+	$(call gb_ExternalExecutable__get_executable,$(1)) \
+	$(gb_ExternalExecutable_$(1)_ARGUMENTS)
 endef
 
 # Return the command for running an external executable.
@@ -109,19 +193,16 @@ define gb_ExternalExecutable_get_command
 $(strip $(call gb_ExternalExecutable__get_command,$(1)))
 endef
 
-define gb_ExternalExecutable__get_deps
+define gb_ExternalExecutable__get_dependencies
 $(call gb_ExternalExecutale__check_registration,$(1))
-$(if $(filter undefined,$(origin gb_ExternalExecutable__$(1)_DEPS)) \
-    ,$(gb_ExternalExecutable__$(1)_TARGET) \
-    ,$(gb_ExternalExecutable__$(1)_DEPS) \
-)
+$(gb_ExternalExecutable_$(1)_DEPENDENCIES)
 endef
 
-# Return the deps needed for running an external executable.
+# Return the dependencies needed for running an external executable.
 #
-# gb_ExternalExecutable_get_deps executable
-define gb_ExternalExecutable_get_deps
-$(strip $(call gb_ExternalExecutable__get_deps,$(1)))
+# gb_ExternalExecutable_get_dependencies executable
+define gb_ExternalExecutable_get_dependencies
+$(strip $(call gb_ExternalExecutable__get_dependencies,$(1)))
 endef
 
 # vim: set noet sw=4 ts=4:
diff --git a/solenv/gbuild/Rdb.mk b/solenv/gbuild/Rdb.mk
index 1a6786545e49..e4df44cf03b1 100644
--- a/solenv/gbuild/Rdb.mk
+++ b/solenv/gbuild/Rdb.mk
@@ -37,7 +37,7 @@ $(call gb_Helper_abbreviate_dirs,\
 	rm $(1).input)
 endef
 
-$(call gb_Rdb_get_target,%) :| $(call gb_ExternalExecutable_get_deps,xsltproc)
+$(call gb_Rdb_get_target,%) :| $(call gb_ExternalExecutable_get_dependencies,xsltproc)
 	$(call gb_Output_announce,$*,$(true),RDB,1)
 	$(call gb_Rdb__command,$@,$*,$?,$^)
 
diff --git a/solenv/gbuild/UnoApiTarget.mk b/solenv/gbuild/UnoApiTarget.mk
index 12e592dce42e..4f2f88d529f6 100644
--- a/solenv/gbuild/UnoApiTarget.mk
+++ b/solenv/gbuild/UnoApiTarget.mk
@@ -72,7 +72,7 @@ endef
 # idlc as well so their dummy rule fires if that changes.
 $(call gb_UnoApiPartTarget_get_target,%.done) : \
 		$(gb_UnoApiPartTarget_IDLCTARGET) \
-		| $(call gb_ExternalExecutable_get_deps,ucpp)
+		| $(call gb_ExternalExecutable_get_dependencies,ucpp)
 	$(call gb_UnoApiPartTarget__command,$@,$*,$(filter-out $(gb_UnoApiPartTarget_IDLCTARGET),$(if $(filter $(gb_UnoApiPartTarget_IDLCTARGET),$?),$^,$?)))
 
 ifeq ($(gb_FULLDEPS),$(true))