Re: [PATCH v3 1/4] build: add make macro for making file from file.in

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.11.2025 13:45, Jürgen Groß wrote:
> On 24.11.25 13:18, Jan Beulich wrote:
>> On 24.11.2025 12:27, Juergen Gross wrote:
>>> On 24.11.25 12:15, Jan Beulich wrote:
>>>> On 24.11.2025 12:05, Jürgen Groß wrote:
>>>>> On 24.11.25 11:41, Jan Beulich wrote:
>>>>>> On 21.11.2025 14:23, Juergen Gross wrote:
>>>>>>> --- a/Config.mk
>>>>>>> +++ b/Config.mk
>>>>>>> @@ -159,6 +159,19 @@ define move-if-changed
>>>>>>>         if ! cmp -s $(1) $(2); then mv -f $(1) $(2); else rm -f $(1); fi
>>>>>>>     endef
>>>>>>>     
>>>>>>> +PATH_FILES := Paths
>>>>>>> +INC_FILES = $(foreach f, $(PATH_FILES), $(XEN_ROOT)/config/$(f).mk)
>>>>>>> +
>>>>>>> +include $(INC_FILES)
>>>>>>> +
>>>>>>> +BUILD_MAKE_VARS = $(foreach f, $(PATH_FILES), $(shell awk '$$2 == ":=" 
>>>>>>> { print $$1; }' $(XEN_ROOT)/config/$(f).mk.in))
>>>>>>
>>>>>> Feels like my prior comments weren't really addressed. I continue to 
>>>>>> think that
>>>>>> none of the above is part of what the subject says.
>>>>>
>>>>> I really don't understand your concern here.
>>>>>
>>>>> For replacing the @markers@ make needs to know what should be replaced.
>>>>> So it needs to scan the files containing the markers and gather them.
>>>>> This is what is done above.
>>>>>
>>>>> In the final macro below the replacements are done then. How would you
>>>>> handle that?
>>>>
>>>> By passing (another) argument to the macro, for example. As indicated
>>>> earlier, different sub-trees may have different places where these
>>>> definitions live, and they would want to be able to pass that in
>>>> (ideally without needing to put this in a common part of the tree).
>>>
>>> I don't get what you want to pass in additionally.
>>>
>>> I've already changed the macro and the Makefiles to be able to add another
>>> marker file to the PATH_FILES variable. What else do you need?
>>
>> Well, that's simply an odd way of passing a parameter. Plus, the extra file
> 
> We do that all the times, e.g. by "OBJ-y += ..."

That's sufficiently different though: Accumulating the set of objects to
produce is kind of naturally done that way.

>> won't affect INC_FILES, or more precisely its use in the include directive
>> in patch 1: At least aiui, $(INC_FILES) is expanded at the point when the
>> directive is processed. Hence why you need to open-code another include
>> there.
> 
> The INC_FILES variable is mostly needed for specifying the dependence of
> the generated files on the files mentioned in PATH_FILES.
> 
> It might be better to just have "-include $(XEN_ROOT/config/Paths.mk" in
> Config.mk, matching the setting of PATH_FILES there.

Looking at this the 3rd or 4th time now, I still don't quite get why the
include is needed in the first place. You don't mean to use (right here)
any of the settings the file produces, do you? Really, as also mentioned
by Andrew, you really can't, because in a pure hypervisor build the file
wouldn't have been made, as no configure would have run.

If I'm not mistaken, you really need those values only at the time you
execute the rule. And I'm worried of these definitions to collide with
something else. Hence one desire would be to limit the scope of these
variables to just the new rule. Maybe something like

# Replace @xxx@ markers in $(1).in with $(xxx) variable contents, write to $(1)
define apply-build-vars
$(1): $$(shell grep -h := $$(wildcard $$(INC_FILES)) /dev/null)
$(1): $(1).in $$(INC_FILES)
        sed $$(foreach v, $$(BUILD_MAKE_VARS), -e 's#@$$(v)@#$$($$(v))#g') <$$< 
>$$@
endef

could work? (This likely depends on INC_FILES to only list files which
are configure generated, i.e. wouldn't be updated by a make rule.)

Jan