Re: [PATCH] Makefile: Add support for user makefile params

[Chuck Lever <chuck.lever@oracle.com>]

On 7/4/25 7:26 AM, Daniel Gomez wrote:
> From: Daniel Gomez <da.gomez@samsung.com>
> 
> Introduce a params.mk file for user-defined Makefile parameters.
> Users can refer to params.mk.sample to see the available configuration
> options and create their own params.mk accordingly. When enabled via
> Kconfig, a params.mk will be automatically generated at the specified
> directory using the sample as a template. When enabled,
> Makefile will attempt to include params.mk if it exists.
> 
> Example: Enable Makefile verbosity and Ansible Verbosity Level 4.
> 
> File: params.mk
> 
> V = 1
> AV = 4
> 
> Signed-off-by: Daniel Gomez <da.gomez@samsung.com>
> ---
> We use GNU Make as a wrapper mainly for Kconfig and Ansible. In order
> to assist kdevops users to change certain options we expose certain
> features to the cli interface via Makefile parameters. A few examples
> are the verbosity of the ansible-playbook command (-v, -vv...) via AV
> parameter. Or the Linux tree git reference (Kconfig) to be used via
> LINUX_TREE_REF. This seems convenient and has been increasing over time.
> Enough parameters have been added than I personally tend to forget
> the exact variable/argument/parameter names. For that, documentation
> may be best but when using a large combination of these options, the
> cli becomes inconvenient by itself. To help with command cli parameters,
> this patch adds a file params.mk.sample that will generate a params.mk
> (if it doesn't exist already) with all parameters disabled. But the file
> will contain examples of how to use each of the options available via
> cli interface. The main Makefile will parse this first and will make it
> available as if the user would pass them through the command line making
> it easier to handle, specially when multiple options are used.
> 
> Thoughts?

Why is a new Kconfig option needed? Instead, can the param.mk file
either exist or not, and "make" will work either way?


I'm glad to see the comments documenting each variable in the sample
file. Some of these I did not know about before. Are they documented
somewhere else as well? Having them all in one place is great.

But, the use of "[ section name ]" suggests this file might be an
INI file, though it's actually just "VAR = value" with comments. A
block comment at the top that explains the format would be helpful,
and perhaps find a different way of demarcating the sections.

/etc/ssh/ssh_config, for example, has this:

# This is the ssh client system-wide configuration file.  See
# ssh_config(5) for more information.  This file provides defaults for
# users, and the values can be changed in per-user configuration files
# or on the command line.

# Configuration data is parsed as follows:
#  1. command line options
#  2. user-specific file
#  3. system-wide file
# Any configuration value is only changed the first time it is set.
# Thus, host-specific definitions should be at the beginning of the
# configuration file, and defaults at the end.

The head of /etc/dnsmasq.conf looks like this:

# Configuration file for dnsmasq.
#
# Format is one option per line, legal options are the same
# as the long options legal on the command line. See
# "/usr/sbin/dnsmasq --help" or "man 8 dnsmasq" for details.


> ---
>  .gitignore               |  2 ++
>  Makefile                 |  8 ++++++++
>  kconfigs/Kconfig.kdevops | 14 ++++++++++++++
>  params.mk.sample         | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 68 insertions(+)
> 
> diff --git a/.gitignore b/.gitignore
> index 706ef3f..470ac24 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -5,6 +5,8 @@
>  .kdevops\.depcheck
>  .provisioned_once*
>  
> +params.mk
> +
>  guestfs/
>  !playbooks/roles/guestfs/
>  
> diff --git a/Makefile b/Makefile
> index acbdd1a..0cbcc09 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -32,6 +32,11 @@ export ANSIBLE_CONFIG := $(ANSIBLE_CFG_FILE)
>  endif
>  ANSIBLE_INVENTORY_FILE := $(shell echo $(CONFIG_ANSIBLE_CFG_INVENTORY) | tr --delete '"')
>  
> +ifeq (y,$(CONFIG_ENABLE_KDEVOPS_PARAMS))
> +KDEVOPS_PARAMS := $(shell echo $(CONFIG_KDEVOPS_PARAMS) | tr --delete '"')
> +-include $(KDEVOPS_PARAMS)
> +endif
> +
>  KDEVOPS_INSTALL_TARGETS :=
>  
>  DEFAULT_DEPS :=
> @@ -204,6 +209,9 @@ $(ANSIBLE_CFG_FILE): .config
>  		$(KDEVOPS_PLAYBOOKS_DIR)/ansible_cfg.yml \
>  		--extra-vars=@./.extra_vars_auto.yaml
>  
> +$(KDEVOPS_PARAMS):
> +	$(Q)cp --verbose $(TOPDIR_PATH)/params.mk.sample $(KDEVOPS_PARAMS)
> +
>  PHONY += $(EXTRA_VAR_INPUTS) $(EXTRA_VAR_INPUTS_LAST)
>  
>  $(KDEVOPS_EXTRA_VARS): .config $(EXTRA_VAR_INPUTS) $(EXTRA_VAR_INPUTS_LAST)
> diff --git a/kconfigs/Kconfig.kdevops b/kconfigs/Kconfig.kdevops
> index 35e9758..4d58261 100644
> --- a/kconfigs/Kconfig.kdevops
> +++ b/kconfigs/Kconfig.kdevops
> @@ -199,3 +199,17 @@ endmenu
>  menu "Ansible Configuration"
>  source "kconfigs/Kconfig.ansible_cfg"
>  endmenu
> +
> +config ENABLE_KDEVOPS_PARAMS
> +	bool "Enable Kdevops User Makefile Parameters File"
> +	default y
> +
> +if ENABLE_KDEVOPS_PARAMS
> +
> +config KDEVOPS_PARAMS
> +	string "Enable Makefile Parameters File"
> +	default "$(TOPDIR_PATH)/params.mk"
> +	help
> +	  Create user params.mk
> +
> +endif # ENABLE_KDEVOPS_PARAMS
> diff --git a/params.mk.sample b/params.mk.sample
> new file mode 100644
> index 0000000..2afae8f
> --- /dev/null
> +++ b/params.mk.sample
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: copyleft-next-0.3.1
> +#
> +# [ Makefile Parameters ]
> +#
> +# Enable Makefile Verbosity
> +# V = 1
> +#
> +# Configure Ansible Verbosity Level (V = {1..6})
> +# AV = 1
> +#
> +# Run command on defined targets
> +# HOSTS="host1 host2"
> +#
> +# [ Ansible Configuration ]
> +#
> +# ANSIBLE_CFG_CALLBACK_PLUGIN = "dense"
> +# ANSIBLE_CFG_INTERPRETER_PYTHON = "/usr/bin/env python3"
> +# ANSIBLE_CFG_FORKS = "5"
> +# KDEVOPS_HOSTS = "$(TOPDIR_PATH)/hosts"
> +#
> +# [ Workflows ]
> +#
> +# KDEVOPS_HOSTS_PREFIX = "kdevops"
> +#
> +# [ Workflow: fstests ]
> +#
> +# Running fstests against only a set of tests
> +# TESTS = "generic/531 xfs/008 xfs/013"
> +#
> +# Running fstests from a specific starting point
> +# START_AFTER = "generic/451"
> +#
> +# SOAK_DURATION = ""
> +# FSTESTS_GROUP = ""
> +#
> +# [ Workflow: linux ]
> +#
> +# LINUX_TREE = ""
> +# LINUX_TREE_REF = ""
> +# B4_MESSAGE_ID = ""
> +#
> +# [ Workflows: CI ]
> +#
> +# CI_WORKFLOW = "blktests_block"
> 
> ---
> base-commit: 4d8e5043bf64d51fe391245d1526c1c65617e437
> change-id: 20250704-b4-params-1b98406cbbea
> prerequisite-change-id: 20250430-ansible_cfg_inventory-7955944ce8ff:v4
> prerequisite-patch-id: a6c8585cae96f5a44064b18d68113d6bb9e36584
> prerequisite-patch-id: e27d48a419b82a0fa1af06d78da315c9ef36c8b8
> prerequisite-patch-id: 87405487f3863f93b623f391d46333fb50b9c148
> prerequisite-patch-id: c628eed9f004f0d494a7fc028bc2c1398da511e5
> prerequisite-patch-id: 95108ed1160cfe3bcc87f141e0508efc2c08cf47
> prerequisite-patch-id: 5962dc6100eae5658513ef88c22175988763aa4b
> prerequisite-patch-id: 19a992e90eed8faa04f20d122d4d3d5a51f2ea77
> prerequisite-patch-id: db660adc9266da83e6dad6d4df59ecd7538b32bb
> prerequisite-patch-id: 5f05abf3975b5e8168ac56b8a3ce1c9b2eacf41d
> prerequisite-patch-id: 0eb292f5292f2748fcc794c91ec978c8816c4ffc
> prerequisite-patch-id: ecad3ea874604b00af65ed08c65368db0d95f1c0
> 
> Best regards,
> --  
> Daniel Gomez <da.gomez@samsung.com>
> 


-- 
Chuck Lever