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

[Jürgen Groß <jgross@xxxxxxxx>]

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.

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


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature