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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.11.2025 15:58, Jürgen Groß wrote:
> On 24.11.25 15:30, Jan Beulich wrote:
>> 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.)
> 
> And who is setting BUILD_MAKE_VARS? You didn't think the snippet doing that
> should be there.

I didn't mean to replace how that's done (albeit, as indicated earlier, I
think it wants doing differently). Hence I've only shown the sketch of the
updated macro wrt the specific aspect above. (Or am I misunderstanding
your reply?)

> And TBH: any setting from Paths.mk colliding with something else would
> _really_ be worrying.

No, why? Those settings should all be fine to use in e.g. the hypervisor
build system, for whatever purpose.

Jan