Re: [PATCH 3/5] libxl: Generate golang bindings in libxl Makefile

[George Dunlap <George.Dunlap@citrix.com>]

> On May 26, 2020, at 2:53 PM, Ian Jackson <ian.jackson@citrix.com> wrote:
> 
> George Dunlap writes ("[PATCH 3/5] libxl: Generate golang bindings in libxl Makefile"):
>> +.PHONY: idl-external
>> +idl-external:
>> +	$(MAKE) -C $(XEN_ROOT)/tools/golang/xenlight idl-gen
> 
> Unfortunately this kind of thing is forbidden.  At least, without a
> rigorous proof that this isn't a concurrency hazard.
> 
> The problem is that with parallel make, the concurrency correctness
> principles are as follows:
> (1) different targets use nonoverlapping temporary and output files
>    (makefile authors' responsibiliy)
> (2) one invocation of make won't make the same target twice at the
>    same time (fundamental principle of operation for make)
> (3) the same makefile (or different makefiles with overlapping
>    targets) may not be entered multiple times in parallel
>    (build system authors' responsibility; preclucdes most use of
>    make -C to sibling directories rather than to children)
> 
> A correctness proof to make an exception would involve demonstrating
> that the tools/golang directories never touch this file when invoked
> as part of a recursive build.  NB, consider the clean targets too.

tools/golang/xenlight/Makefile:*.gen.go target will be triggered by xenlight/Makefile:idl-gen and xenlight/Makefile:build.

xenlight/Makefile:build is called from tools/golang/Makfile:subdirs-all, which is called from tools/Makefile:subdirs-all.

xenlight/Makefile:idl-gen is called from tools/libxl/Makefile:idl-external, which is called from tools/libxl/Makefile:all, which is called from tools/Makefile:subdirs-all.

tools/Makefile:subdirs-all is implemented as a non-parallel for loop executing over SUBDIRS-y; tools/golang comes after tools/libxl in that list, and so tools/golang:all will never be called until after tools/libxl:all has completed.  This invariant — that tools/golang/Makefile:all must not be called until tools/libxl/Makefile:all has completed must be kept regardless of this patch, since xenlight/Makefile:build depends on other output from tools/libxl/Makefile:all.

So as long as nothing else calls tools/libxl:all or tools/libxl:idl-external, this should be safe.  We could add a comments near xenlight/Makefile:idl-gen saying it must only be called from libxl/Makefile:idl-external; and to libxl/Makefile:idl-external saying it must not be called recursively from another Makefile.

> Alternatively, move the generated golang files to tools/libxl maybe,
> and perhaps leave symlinks behind.

Would that result in the files being accessible to the golang build tools at https://xenbits.xenproject.org/git-http/xen.git/tools/golang/xenlight ?  If not, it defeats the purpose of having the files checked into the tree.

> Or convert the whole (of tools/, maybe) to nonrecursive make using eg
> subdirmk :-).  https://diziet.dreamwidth.org/5763.html

This isn’t really a practical suggestion: I don’t have time to refactor the entire libxl Makefile tree, and certainly don’t have time by Friday.

 -George