> On 30 Sep 2022, at 15:59, Christian Lindig <christian.lin...@citrix.com> > wrote: > > > >> On 27 Sep 2022, at 17:13, Edwin Torok <edvin.to...@citrix.com> wrote: >> >> >> See below for a patch for that. I've included this patch in the correct >> place (before the patch that breaks it) in the git repository at: >> https://github.com/edwintorok/xen/compare/private/edvint/public0 >> > > > Acked-by: Christian Lindig <christian.lin...@citrix.com> > > I believe these changes are fine. We are now allocating the event channel > dynamically and this requires using a finaliser to free that memory.
Thanks, > > >> -ifneq ($(MAKECMDGOALS),clean) >> +ifeq (,$(findstring clean,$(MAKECMDGOALS))) >> .ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile >> $(OCAML_TOPLEVEL)/Makefile.rules >> $(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli >> $o,MLDEP,) >> endif > > Is this the right logic? Moving from ifneq to ifeq here. > > I am not so familiar with the Makfile rules. The gist seems to be: we depend > on auto-generated Make dependencies that the Makefile in general depends on. > But in a “make clean” (or other “*clean” it is wasteful to generate these > only to later remove them. However, these kind of subtleties are obvious > enough while we are working on this but over time accumulate to Makefiles > that are hard to change. So I wonder if this kind of optimisation, while > correct, is worth it, but fine going along with it. > Makefile functions can be a bit confusing to read. "ifneq ($(MAKECMDGOALS), clean)" means $(MAKECMDGOALS) != "clean" "ifeq (,$(findstring clean,$(MAKECMDGOALS)))" means that "clean" in $(MAKECMDGOALS) == "" (the empty string), or i.o.w. "clean" not in $(MAKECMDGOALS), which is a bit more generic than the previous one, since we have all sorts of rules in the Makefile (especially around subdirs) where 'clean' is a substring. This is quite subtle and I had to reread this line many times too to check it is correct. The real solution here would be to have a single non-recursive Makefile (and there is some discussion/patches heading in that direction in xen-devel particularly from Anthony), and then evaluating the "clean" rules would be a lot less expensive, it'd only have to be done once. But there might be a while until we get there, and meanwhile these clean rules slow down the OCaml build too much (just running the "clean" takes a lot longer than building the entire OCaml libraries and oxenstored sequentially). Although I only need to use 'clean' when using the upstream Makefiles (where almost every incremental change requires a 'clean' inbetween because the Makefiles express the dependencies incorrectly), or when switching from upstream Makefile to 'dune' (a one-off event usually). Since I use 'Dune' for my daily work anyway (and the makefile is used in our internal build system) perhaps this Makefile patch is not needed at all, I can change 'Makefile.dune' to not call 'make clean' at all, and I'll know to remember to run it if things fail anyway (it'll be pretty obvious when Dune says you've got a build artifact in the wrong place). Best regards, --Edwin
