yallop / ocaml-ctypes

Library for binding to C libraries using pure OCaml
MIT License
363 stars 95 forks source link

Update Makefile #722

Closed atupone closed 1 year ago

atupone commented 1 year ago

The suggestion here is to sequence the install.

When using shuffle option on make or when doing parallell make during install the two install can result in.

ocamlfind: Cannot find META in package dir

I hope this fixes it

yallop commented 1 year ago

Thank you for the report and proposed fix, @atupone.

I'd prefer not to call make recursively. Can we sequence by adding an additional dependency along the following lines instead?

$(PROJECTS:%=install-%): META-install
atupone commented 1 year ago

The install target is just a PHONY target, so I think you can just put all the actions you want to do:


install:
             $(OCAMLFIND) install ctypes META CHANGES.md
            $(if $(filter yes,$($(PROJECT).install)),\
        $(OCAMLFIND) install -add ctypes -optional $^ \
                   $(LIB_TARGETS) $(LIB_TARGET_EXTRAS) \
                   $(INSTALL_MLIS) $(INSTALL_CMIS) \
                   $(INSTALL_CMTS) $(INSTALL_CMTIS) \
                   $(INSTALL_HEADERS) \
                   $(if $(filter yes,$($(PROJECT).install_native_objects)),$(NATIVE_OBJECTS)))

```and maybe  others
yallop commented 1 year ago

I'd prefer to keep as much information as possible in the dependencies, not in the actions.

atupone commented 1 year ago

Can we sequence by adding an additional dependency along the following lines instead?

$(PROJECTS:%=install-%): META-install

I think it works

yallop commented 1 year ago

Thank you for checking. If you update this branch with that change, I'll merge.

atupone commented 1 year ago

commit tested here and squashed