Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cross-compilation support #4

Closed
wants to merge 1 commit into from
Closed

Add cross-compilation support #4

wants to merge 1 commit into from

Conversation

whitequark
Copy link

This commit replaces the hardcoded .a/.so extensions with
the $(EXT_LIB)/$(EXT_DLL) variables, and runs the compilation
commands through ocamlfind so that OCAMLFIND_TOOLCHAIN could be
used by a cross-compilation repository.

Copy link
Owner

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about ocamlfind and cross-compilation, but globally this patch looks good. Some comments in the code review.

Makefile Outdated

OBJS=zlib.cmo zip.cmo gzip.cmo
C_OBJS=zlibstubs.o

NATDYNLINK := $(shell if [ -f `ocamlc -where`/dynlink.cmxa ]; then echo YES; else echo NO; fi)
include $(shell ocamlfind ocamlc -where)/Makefile.config
NATDYNLINK := $(shell if [ -f `ocamlfind ocamlc -where`/dynlink.cmxa ]; then echo YES; else echo NO; fi)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are to include OCaml's Makefile.config, there is probably a simpler way to determine whether .cmxs files are supported.

Makefile Outdated
@@ -68,25 +69,25 @@ libcamlzip.a: $(C_OBJS)

clean:
rm -f *.cm*
rm -f *.o *.a *.so
rm -f *.o *$(EXT_LIB) *$(EXT_DLL)

install:
mkdir -p $(INSTALLDIR)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make ocamlfind mandatory, this legacy, ocamlfind-free "install" procedure should be removed.

Makefile Outdated
ldconf=`$(OCAMLC) -where`/ld.conf; \
installdir=$(INSTALLDIR); \
if test `grep -s -c $$installdir'$$' $$ldconf || :` = 0; \
then echo $$installdir >> $$ldconf; fi \
fi

installopt:
cp zip.cmxa $(CMXS) zip.a zip.cmx gzip.cmx zlib.cmx $(INSTALLDIR)
cp zip.cmxa $(CMXS) zip$(EXT_LIB) zip.cmx gzip.cmx zlib.cmx $(INSTALLDIR)

install-findlib:
cp META-zip META && \
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this one should also be called "install".

This commit replaces the hardcoded .a/.so extensions with
the $(EXT_LIB)/$(EXT_DLL) variables, and runs the compilation
commands through ocamlfind so that OCAMLFIND_TOOLCHAIN could be
used by a cross-compilation repository.
@whitequark
Copy link
Author

All done, also fixed a stray ocamldep invocation.

@whitequark
Copy link
Author

Ping

@xavierleroy
Copy link
Owner

Apologies for the delay. This was merged manually (to resolve a tiny Makefile conflict) in [master 0c28962].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants