> On May 26, 2020, at 2:53 PM, Ian Jackson <[email protected]> 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
