| Message ID | 110f63b76a12e8a9fead09f47319a35229222953.1724314239.git.andrii.sultanov@cloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Stabilize Oxenstored's interface with | expand |
On 22/08/2024 10:06 am, Andrii Sultanov wrote: > This flag does not work as assumed and will not pass > options (such as -shared) to the C compiler: > https://github.com/ocaml/ocaml/issues/12284 > > Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com> > --- > tools/ocaml/common.make | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/ocaml/common.make b/tools/ocaml/common.make > index 0c8a597d5b..cc126b749f 100644 > --- a/tools/ocaml/common.make > +++ b/tools/ocaml/common.make > @@ -12,7 +12,7 @@ OCAMLFIND ?= ocamlfind > CFLAGS += -fPIC -I$(shell ocamlc -where) > > OCAMLOPTFLAG_G := $(shell $(OCAMLOPT) -h 2>&1 | sed -n 's/^ *\(-g\) .*/\1/p') > -OCAMLOPTFLAGS = $(OCAMLOPTFLAG_G) -ccopt "$(LDFLAGS)" -dtypes $(OCAMLINCLUDE) -cc $(CC) -w F -warn-error F > +OCAMLOPTFLAGS = $(OCAMLOPTFLAG_G) -ccopt "$(LDFLAGS)" -dtypes $(OCAMLINCLUDE) -w F -warn-error F > OCAMLCFLAGS += -g $(OCAMLINCLUDE) -w F -warn-error F > > VERSION := 4.1 This patch itself is fine, and I'll commit it in due course, but then I got looking at the surrounding context... `$(OCAMLOPT) -h` tells you on stderr to try `--help instead`, so OCAMLOPTFLAG_G is never going to contain -g. Also, why is -g conditional for OCAMLOPTFLAGS but unconditional for OCAMLCFLAGS? I think we can safely drop OCAMLOPTFLAG_G Next, there's VERSION and git grep says its only used in META files. xen.git/tools/ocaml$ git grep -w VERSION Makefile.rules:43: sed 's/@VERSION@/$(VERSION)/g' < $< $o common.make:18:VERSION := 4.1 libs/eventchn/META.in:1:version = "@VERSION@" libs/mmap/META.in:1:version = "@VERSION@" libs/xb/META.in:1:version = "@VERSION@" libs/xc/META.in:1:version = "@VERSION@" libs/xenstoredglue/META.in:1:version = "@VERSION@" libs/xenstoredglue/domain_getinfo_plugin_v1/META.in:1:version = "@VERSION@" libs/xs/META.in:1:version = "@VERSION@" 4.1 is very very stale and should say 4.19 these days (definitely for xc, and whatever else is using an unstable API), yet should definitely not be 4.19 for xenstoredglue. Are there any ABI/API implication from changing the META file? ~Andrew
> On 22 Aug 2024, at 13:25, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > Are there any ABI/API implication from changing the META file? The META file is for package/library management in an OCaml environment. I don’t see much relevance for it in the code that is part of the Xen tree - so see no problem changing the version. — C
On Thu, Aug 22, 2024 at 1:25 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 22/08/2024 10:06 am, Andrii Sultanov wrote: > > This flag does not work as assumed and will not pass > > options (such as -shared) to the C compiler: > > https://github.com/ocaml/ocaml/issues/12284 > > > > Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com> > > --- > > tools/ocaml/common.make | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/ocaml/common.make b/tools/ocaml/common.make > > index 0c8a597d5b..cc126b749f 100644 > > --- a/tools/ocaml/common.make > > +++ b/tools/ocaml/common.make > > @@ -12,7 +12,7 @@ OCAMLFIND ?= ocamlfind > > CFLAGS += -fPIC -I$(shell ocamlc -where) > > > > OCAMLOPTFLAG_G := $(shell $(OCAMLOPT) -h 2>&1 | sed -n 's/^ *\(-g\) .*/\1/p') > > -OCAMLOPTFLAGS = $(OCAMLOPTFLAG_G) -ccopt "$(LDFLAGS)" -dtypes $(OCAMLINCLUDE) -cc $(CC) -w F -warn-error F > > +OCAMLOPTFLAGS = $(OCAMLOPTFLAG_G) -ccopt "$(LDFLAGS)" -dtypes $(OCAMLINCLUDE) -w F -warn-error F > > OCAMLCFLAGS += -g $(OCAMLINCLUDE) -w F -warn-error F > > > > VERSION := 4.1 > > This patch itself is fine, and I'll commit it in due course, but then I > got looking at the surrounding context... > > `$(OCAMLOPT) -h` tells you on stderr to try `--help instead`, so > OCAMLOPTFLAG_G is never going to contain -g. > > Also, why is -g conditional for OCAMLOPTFLAGS but unconditional for > OCAMLCFLAGS? I think we can safely drop OCAMLOPTFLAG_G I'm not aware of a version of OCaml that didn't support -g, but maybe a very old version (that wouldn't pass the minimum version check) didn't have it. I agree that we can safely drop the conditional and always pass `-g`. > > > Next, there's VERSION and git grep says its only used in META files. > > xen.git/tools/ocaml$ git grep -w VERSION > Makefile.rules:43: sed 's/@VERSION@/$(VERSION)/g' < $< $o > common.make:18:VERSION := 4.1 > libs/eventchn/META.in:1:version = "@VERSION@" > libs/mmap/META.in:1:version = "@VERSION@" > libs/xb/META.in:1:version = "@VERSION@" > libs/xc/META.in:1:version = "@VERSION@" > libs/xenstoredglue/META.in:1:version = "@VERSION@" > libs/xenstoredglue/domain_getinfo_plugin_v1/META.in:1:version = "@VERSION@" > libs/xs/META.in:1:version = "@VERSION@" > > 4.1 is very very stale and should say 4.19 these days (definitely for > xc, and whatever else is using an unstable API), yet should definitely > not be 4.19 for xenstoredglue. > > Are there any ABI/API implication from changing the META file? It is purely informational (e.g. show up in the output of `ocamlfind list`), dependency resolution is done using `opam` files (which Xen doesn't have), not `META` files. You can link some code into an executable that lists the versions of all the libraries that it got linked with (using an OCamlfind module), and in that case it might be nice to have correct information there, but I don't think any of our code does that. > > ~Andrew
diff --git a/tools/ocaml/common.make b/tools/ocaml/common.make index 0c8a597d5b..cc126b749f 100644 --- a/tools/ocaml/common.make +++ b/tools/ocaml/common.make @@ -12,7 +12,7 @@ OCAMLFIND ?= ocamlfind CFLAGS += -fPIC -I$(shell ocamlc -where) OCAMLOPTFLAG_G := $(shell $(OCAMLOPT) -h 2>&1 | sed -n 's/^ *\(-g\) .*/\1/p') -OCAMLOPTFLAGS = $(OCAMLOPTFLAG_G) -ccopt "$(LDFLAGS)" -dtypes $(OCAMLINCLUDE) -cc $(CC) -w F -warn-error F +OCAMLOPTFLAGS = $(OCAMLOPTFLAG_G) -ccopt "$(LDFLAGS)" -dtypes $(OCAMLINCLUDE) -w F -warn-error F OCAMLCFLAGS += -g $(OCAMLINCLUDE) -w F -warn-error F VERSION := 4.1
This flag does not work as assumed and will not pass options (such as -shared) to the C compiler: https://github.com/ocaml/ocaml/issues/12284 Signed-off-by: Andrii Sultanov <andrii.sultanov@cloud.com> --- tools/ocaml/common.make | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)