[Makefile]: variable ENABLE_SYNCD_RPC is always empty string

[cytsai0409]

Description

I can not build the test image by setting the "ENABLE_SYNCD_RPC = y" in rules/config

Steps to reproduce the issue:

1. uncomment the line "ENABLE_SYNCD_RPC = y" in rules/config
2. execute "make target/sonic-broadcom.bin"
3. check the console output and the variable "ENABLE_SYNCD_RPC" is set to empty string

Build Configuration
"CONFIGURED_PLATFORM" : "broadcom"
"SONIC_CONFIG_PRINT_DEPENDENCIES" : ""
"SONIC_CONFIG_BUILD_JOBS" : "1"
"SONIC_CONFIG_MAKE_JOBS" : "4"
"DEFAULT_USERNAME" : "admin"
"DEFAULT_PASSWORD" : "YourPaSsWoRd"
"ENABLE_DHCP_GRAPH_SERVICE" : ""
"SHUTDOWN_BGP_ON_START" : ""
"SONIC_CONFIG_DEBUG" : ""
"ROUTING_STACK" : "quagga"
"ENABLE_SYNCD_RPC" : "" <------------------------ (always empty string)
"ENABLE_ORGANIZATION_EXTENSIONS" : "y"

Describe the results you received:

The variable "ENABLE_SYNCD_RPC" is always set to empty string and test image is not built

Describe the results you expected:

The variable "ENABLE_SYNCD_RPC" should be set to "y" and test image is built.

Root Cause

In Makefile, the variable ENABLE_SYNCD_RPC is not initialized and assigned as command argument to
SONIC_BUILD_INSTRUCTION as below:

SONIC_BUILD_INSTRUCTION := make
-f slave.mk
PLATFORM=$(PLATFORM)
BUILD_NUMBER=$(BUILD_NUMBER)
ENABLE_DHCP_GRAPH_SERVICE=$(ENABLE_DHCP_GRAPH_SERVICE)
SHUTDOWN_BGP_ON_START=$(SHUTDOWN_BGP_ON_START)
ENABLE_SYNCD_RPC=$(ENABLE_SYNCD_RPC) \ <----------- (not initialized)
PASSWORD=$(PASSWORD)
USERNAME=$(USERNAME)

The variable ENABLE_SYNCD_RPC in rules/config is ignored since command argument has higher precedence.

Suggestion

Remove the command argument ENABLE_SYNCD_RPC in Makefile to avoid variable overwriting as below:

SONIC_BUILD_INSTRUCTION := make
-f slave.mk
PLATFORM=$(PLATFORM)
BUILD_NUMBER=$(BUILD_NUMBER)
ENABLE_DHCP_GRAPH_SERVICE=$(ENABLE_DHCP_GRAPH_SERVICE)
SHUTDOWN_BGP_ON_START=$(SHUTDOWN_BGP_ON_START) \
PASSWORD=$(PASSWORD)
USERNAME=$(USERNAME)

[marian-pritsak]

#1032

This PR causes a problem. Variables had different names intentionally - to avoid overriding them with empty strings from entry point Makefile.

@cytsai0409 for now, as a workaround, I'd suggest to use following command:

make ENABLE_SYNCD_RPC=y target/sonic-broadcom.bin

[lguohan]

Can we revert #1032? @stcheng ?

[jleveque]

If I'm understanding this issue correctly, ENABLE_SYNCD_RPC is an empty string in the main Makefile. It is then passed as a command line argument to slave.mk. slave.mk includes rules/config which then will try to set the variable if it is defined there. However, with GNU Make, variables set via a command line argument take precedence over those set in a file, so the variable is not overridden with the value from rules/config.

If this is the case, we could possibly use the "override" directive in rules/config (https://www.gnu.org/software/make/manual/html_node/Override-Directive.html#Override-Directive). However, I think we ultimately want command line parameters to take precedence, as is the GNU make default behavior.

Therefore, I believe this can be resolved in the main Makefile. When calling slave.mk, we should only pass variables on the command line if they are already set (i.e., they were set on the command line when invoking the main Makefile. It might be a little ugly due to the nature of GNU Make conditionals, but I believe it will work, and if so, it's probably the best solution.