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(-)

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)
-- 
Anthony PERARD

> 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.

-- 
Anthony PERARD

> 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