| Message ID | 20220225151321.44126-30-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Toolstack build system improvement, toward non-recursive makefiles | expand |
> On 25 Feb 2022, at 15:13, Anthony PERARD <anthony.perard@citrix.com> wrote: > > This patch fix ".ocamldep.make" rule by always spelling the variable > $(OCAML_TOPLEVEL). > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > > Notes: > v2: > - new patch > > tools/ocaml/libs/eventchn/Makefile | 8 ++++---- > tools/ocaml/libs/mmap/Makefile | 8 ++++---- > tools/ocaml/libs/xb/Makefile | 8 ++++---- > tools/ocaml/libs/xc/Makefile | 8 ++++---- > tools/ocaml/libs/xentoollog/Makefile | 8 ++++---- > tools/ocaml/libs/xl/Makefile | 8 ++++---- > tools/ocaml/libs/xs/Makefile | 8 ++++---- > tools/ocaml/Makefile.rules | 2 +- Acked-by: Christian Lindig <christian.lindig@citrix.com> I am fine with this but in general think that the OCaml part should be built using Dune (but invoked from Make), which is now the standard tool to build OCaml projects and is simple, fast, and accurate. Edwin maintains such a build for all development work on the OCaml side but it is not upstreamed. — C
On Fri, Feb 25, 2022 at 03:30:59PM +0000, Christian Lindig wrote: > > > > On 25 Feb 2022, at 15:13, Anthony PERARD <anthony.perard@citrix.com> wrote: > > > > This patch fix ".ocamldep.make" rule by always spelling the variable > > $(OCAML_TOPLEVEL). > > > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > --- > > > > Notes: > > v2: > > - new patch > > > > tools/ocaml/libs/eventchn/Makefile | 8 ++++---- > > tools/ocaml/libs/mmap/Makefile | 8 ++++---- > > tools/ocaml/libs/xb/Makefile | 8 ++++---- > > tools/ocaml/libs/xc/Makefile | 8 ++++---- > > tools/ocaml/libs/xentoollog/Makefile | 8 ++++---- > > tools/ocaml/libs/xl/Makefile | 8 ++++---- > > tools/ocaml/libs/xs/Makefile | 8 ++++---- > > tools/ocaml/Makefile.rules | 2 +- > > Acked-by: Christian Lindig <christian.lindig@citrix.com> > > I am fine with this but in general think that the OCaml part should be built using Dune (but invoked from Make), which is now the standard tool to build OCaml projects and is simple, fast, and accurate. Edwin maintains such a build for all development work on the OCaml side but it is not upstreamed. ocaml-dune doesn't seems to be available on debian oldstable. So I don't think we can use it for now. But thanks for pointing that out.
> On 25 Feb 2022, at 16:28, Anthony PERARD <anthony.perard@citrix.com> wrote: > > On Fri, Feb 25, 2022 at 03:30:59PM +0000, Christian Lindig wrote: >> >> >>> On 25 Feb 2022, at 15:13, Anthony PERARD <anthony.perard@citrix.com> wrote: >>> >>> This patch fix ".ocamldep.make" rule by always spelling the variable >>> $(OCAML_TOPLEVEL). >>> >>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >>> --- >>> >>> Notes: >>> v2: >>> - new patch >>> >>> tools/ocaml/libs/eventchn/Makefile | 8 ++++---- >>> tools/ocaml/libs/mmap/Makefile | 8 ++++---- >>> tools/ocaml/libs/xb/Makefile | 8 ++++---- >>> tools/ocaml/libs/xc/Makefile | 8 ++++---- >>> tools/ocaml/libs/xentoollog/Makefile | 8 ++++---- >>> tools/ocaml/libs/xl/Makefile | 8 ++++---- >>> tools/ocaml/libs/xs/Makefile | 8 ++++---- >>> tools/ocaml/Makefile.rules | 2 +- >> >> Acked-by: Christian Lindig <christian.lindig@citrix.com> >> >> I am fine with this but in general think that the OCaml part should be built using Dune (but invoked from Make), which is now the standard tool to build OCaml projects and is simple, fast, and accurate. Edwin maintains such a build for all development work on the OCaml side but it is not upstreamed. > > ocaml-dune doesn't seems to be available on debian oldstable. So I don't > think we can use it for now. > > But thanks for pointing that out. > I think we should try to add it as an optional build-system: when available use it, and at some point in the future remove the old one. It is pretty much impossible to do development on the codebase without it, any developer who wants to make the changes to the OCaml code will likely want it. (Of course those who only want to build and install oxenstored may not require dune, and may be fine with the Makefiles as they wouldn't require incremental builds or editor support). Best regards, --Edwin > -- > Anthony PERARD
diff --git a/tools/ocaml/libs/eventchn/Makefile b/tools/ocaml/libs/eventchn/Makefile index 154efd4a8e..7362a28d9e 100644 --- a/tools/ocaml/libs/eventchn/Makefile +++ b/tools/ocaml/libs/eventchn/Makefile @@ -1,6 +1,6 @@ -TOPLEVEL=$(CURDIR)/../.. -XEN_ROOT=$(TOPLEVEL)/../.. -include $(TOPLEVEL)/common.make +OCAML_TOPLEVEL=$(CURDIR)/../.. +XEN_ROOT=$(OCAML_TOPLEVEL)/../.. +include $(OCAML_TOPLEVEL)/common.make CFLAGS += $(CFLAGS_libxenevtchn) $(CFLAGS_xeninclude) @@ -31,5 +31,5 @@ install: $(LIBS) META uninstall: $(OCAMLFIND) remove -destdir $(OCAMLDESTDIR) xeneventchn -include $(TOPLEVEL)/Makefile.rules +include $(OCAML_TOPLEVEL)/Makefile.rules diff --git a/tools/ocaml/libs/mmap/Makefile b/tools/ocaml/libs/mmap/Makefile index df45819df5..a621537135 100644 --- a/tools/ocaml/libs/mmap/Makefile +++ b/tools/ocaml/libs/mmap/Makefile @@ -1,6 +1,6 @@ -TOPLEVEL=$(CURDIR)/../.. -XEN_ROOT=$(TOPLEVEL)/../.. -include $(TOPLEVEL)/common.make +OCAML_TOPLEVEL=$(CURDIR)/../.. +XEN_ROOT=$(OCAML_TOPLEVEL)/../.. +include $(OCAML_TOPLEVEL)/common.make OBJS = xenmmap INTF = $(foreach obj, $(OBJS),$(obj).cmi) @@ -26,5 +26,5 @@ install: $(LIBS) META uninstall: $(OCAMLFIND) remove -destdir $(OCAMLDESTDIR) xenmmap -include $(TOPLEVEL)/Makefile.rules +include $(OCAML_TOPLEVEL)/Makefile.rules diff --git a/tools/ocaml/libs/xb/Makefile b/tools/ocaml/libs/xb/Makefile index be4499147e..ff4428af6d 100644 --- a/tools/ocaml/libs/xb/Makefile +++ b/tools/ocaml/libs/xb/Makefile @@ -1,6 +1,6 @@ -TOPLEVEL=$(CURDIR)/../.. -XEN_ROOT=$(TOPLEVEL)/../.. -include $(TOPLEVEL)/common.make +OCAML_TOPLEVEL=$(CURDIR)/../.. +XEN_ROOT=$(OCAML_TOPLEVEL)/../.. +include $(OCAML_TOPLEVEL)/common.make CFLAGS += -I../mmap CFLAGS += $(CFLAGS_libxenctrl) # For xen_mb() @@ -49,4 +49,4 @@ install: $(LIBS) META uninstall: $(OCAMLFIND) remove -destdir $(OCAMLDESTDIR) xenbus -include $(TOPLEVEL)/Makefile.rules +include $(OCAML_TOPLEVEL)/Makefile.rules diff --git a/tools/ocaml/libs/xc/Makefile b/tools/ocaml/libs/xc/Makefile index b6da4fdbaf..67acc46bee 100644 --- a/tools/ocaml/libs/xc/Makefile +++ b/tools/ocaml/libs/xc/Makefile @@ -1,6 +1,6 @@ -TOPLEVEL=$(CURDIR)/../.. -XEN_ROOT=$(TOPLEVEL)/../.. -include $(TOPLEVEL)/common.make +OCAML_TOPLEVEL=$(CURDIR)/../.. +XEN_ROOT=$(OCAML_TOPLEVEL)/../.. +include $(OCAML_TOPLEVEL)/common.make CFLAGS += -I../mmap $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) CFLAGS += $(APPEND_CFLAGS) @@ -38,4 +38,4 @@ xenctrl_abi_check.h: abi-check xenctrl_stubs.c xenctrl.ml GENERATED_FILES += xenctrl_abi_check.h -include $(TOPLEVEL)/Makefile.rules +include $(OCAML_TOPLEVEL)/Makefile.rules diff --git a/tools/ocaml/libs/xentoollog/Makefile b/tools/ocaml/libs/xentoollog/Makefile index 593f9e9e9d..9ede2fd124 100644 --- a/tools/ocaml/libs/xentoollog/Makefile +++ b/tools/ocaml/libs/xentoollog/Makefile @@ -1,6 +1,6 @@ -TOPLEVEL=$(CURDIR)/../.. -XEN_ROOT=$(TOPLEVEL)/../.. -include $(TOPLEVEL)/common.make +OCAML_TOPLEVEL=$(CURDIR)/../.. +XEN_ROOT=$(OCAML_TOPLEVEL)/../.. +include $(OCAML_TOPLEVEL)/common.make # allow mixed declarations and code CFLAGS += -Wno-declaration-after-statement @@ -62,4 +62,4 @@ install: $(LIBS) META uninstall: ocamlfind remove -destdir $(OCAMLDESTDIR) xentoollog -include $(TOPLEVEL)/Makefile.rules +include $(OCAML_TOPLEVEL)/Makefile.rules diff --git a/tools/ocaml/libs/xl/Makefile b/tools/ocaml/libs/xl/Makefile index cbe1569cc5..7c1c4edced 100644 --- a/tools/ocaml/libs/xl/Makefile +++ b/tools/ocaml/libs/xl/Makefile @@ -1,6 +1,6 @@ -TOPLEVEL=$(CURDIR)/../.. -XEN_ROOT=$(TOPLEVEL)/../.. -include $(TOPLEVEL)/common.make +OCAML_TOPLEVEL=$(CURDIR)/../.. +XEN_ROOT=$(OCAML_TOPLEVEL)/../.. +include $(OCAML_TOPLEVEL)/common.make # ignore unused generated functions and allow mixed declarations and code CFLAGS += -Wno-unused -Wno-declaration-after-statement @@ -68,4 +68,4 @@ install: $(LIBS) META uninstall: $(OCAMLFIND) remove -destdir $(OCAMLDESTDIR) xenlight -include $(TOPLEVEL)/Makefile.rules +include $(OCAML_TOPLEVEL)/Makefile.rules diff --git a/tools/ocaml/libs/xs/Makefile b/tools/ocaml/libs/xs/Makefile index 572efb76c4..e934bbb550 100644 --- a/tools/ocaml/libs/xs/Makefile +++ b/tools/ocaml/libs/xs/Makefile @@ -1,6 +1,6 @@ -TOPLEVEL=$(CURDIR)/../.. -XEN_ROOT=$(TOPLEVEL)/../.. -include $(TOPLEVEL)/common.make +OCAML_TOPLEVEL=$(CURDIR)/../.. +XEN_ROOT=$(OCAML_TOPLEVEL)/../.. +include $(OCAML_TOPLEVEL)/common.make OCAMLINCLUDE += -I ../xb/ OCAMLOPTFLAGS += -for-pack Xenstore @@ -43,7 +43,7 @@ install: $(LIBS) META uninstall: $(OCAMLFIND) remove -destdir $(OCAMLDESTDIR) xenstore -include $(TOPLEVEL)/Makefile.rules +include $(OCAML_TOPLEVEL)/Makefile.rules genpath-target = $(call buildmakevars2module,paths.ml) $(eval $(genpath-target)) diff --git a/tools/ocaml/Makefile.rules b/tools/ocaml/Makefile.rules index abfbc64ce0..7e4db457a1 100644 --- a/tools/ocaml/Makefile.rules +++ b/tools/ocaml/Makefile.rules @@ -44,7 +44,7 @@ META: META.in ALL_OCAML_OBJ_SOURCES=$(addsuffix .ml, $(ALL_OCAML_OBJS)) -.ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile $(TOPLEVEL)/Makefile.rules +.ocamldep.make: $(ALL_OCAML_OBJ_SOURCES) Makefile $(OCAML_TOPLEVEL)/Makefile.rules $(call quiet-command, $(OCAMLDEP) $(ALL_OCAML_OBJ_SOURCES) *.mli $o,MLDEP,) clean: $(CLEAN_HOOKS)
They are two competiting spelling for the variable holding the path to "tools/ocaml", $(TOPLEVEL) and $(OCAML_TOPLEVEL). The "Makefile.rules" which is included in all ocaml Makefiles have one rule which make use of that variable which is then sometime unset. When building "ocaml/xenstored", make isn't capable of generating ".ocamldep.make" because $(TOPLEVEL) isn't defined in this case. This can fail with an error like this when paths.ml have been regenerated: Error: Files define.cmx and paths.cmx make inconsistent assumptions over interface Paths This patch fix ".ocamldep.make" rule by always spelling the variable $(OCAML_TOPLEVEL). Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- Notes: v2: - new patch tools/ocaml/libs/eventchn/Makefile | 8 ++++---- tools/ocaml/libs/mmap/Makefile | 8 ++++---- tools/ocaml/libs/xb/Makefile | 8 ++++---- tools/ocaml/libs/xc/Makefile | 8 ++++---- tools/ocaml/libs/xentoollog/Makefile | 8 ++++---- tools/ocaml/libs/xl/Makefile | 8 ++++---- tools/ocaml/libs/xs/Makefile | 8 ++++---- tools/ocaml/Makefile.rules | 2 +- 8 files changed, 29 insertions(+), 29 deletions(-)