[m-rev.] For review: library predicates for Java using JNI

[Fergus Henderson]

On 02-Feb-2004, James Goddard <goddardjames at yahoo.com> wrote:
> For review by Fergus:
> 
> This is the relative diff for the code which incorporates JNI into the
> Java implementation.  Is this what you had in mind for the Makefile?
...
> +++ java/runtime/Native.java	2 Feb 2004 06:04:40 -0000
> @@ -18,13 +18,13 @@
>  	** SHARED_OBJ is the name of the shared object which contains all the
>  	** compiled native code.
>  	*/
> -	private	static final java.lang.String	SHARED_OBJ	= "Native.so";
> +	private	static final java.lang.String	SHARED_OBJ = "Native.pic_o";

That should stay as ".so".  The extension ".pic_o" is for object files
(a.k.a. "relocatables") that can be linked together into a ".so" file.

E.g. 
	$ file library.pic_o libmer_std.so
	library.pic_o: ELF 32-bit LSB relocatable, Intel 80386, ...
	libmer_std.so: ELF 32-bit LSB shared object, Intel 80386, ...

> diff -u java/runtime/Mmakefile java/runtime/Mmakefile
> --- java/runtime/Mmakefile	29 Jan 2004 00:21:15 -0000
> +++ java/runtime/Mmakefile	2 Feb 2004 05:42:20 -0000
> @@ -8,17 +8,27 @@
>  
> +MERCURY_DIR	= ../..
> +RUNTIME_DIR	= $(MERCURY_DIR)/runtime
>  
> +include $(MERCURY_DIR)/Mmake.common

Good...

> -MAIN_TARGET=Native.so
> +MAIN_TARGET	= Native.pic_o

Not a good idea.  See above.

> +CC		= $(MGNUC) $(ALL_GRADEFLAGS) $(ALL_MGNUCFLAGS)
> +CFLAGS		= -I$(RUNTIME_DIR)
...
>  Native.o:	Native.c
> -		gcc -c Native.c
> +		$(CC) $(CFLAGS) -c Native.c

This is not right.  ALL_MGNUCFLAGS already includes $(CFLAGS),
so this will include -I$(RUNTIME_DIR) twice.

Also, $(CC) should not be defined in terms of $(MGNUC) because
$(MGNUC) might be defined in terms of $(CC).  Just use
"$(MGNUC) $(ALL_GRADEFLAGS) $(ALL_MGNUCFLAGS)" rather than
"$(CC) $(CFLAGS)", and then you don't need to set $(CC).

> -Native.so:	Native.o ../../runtime/mercury_timing.o
> -		gcc -shared Native.o ../../runtime/mercury_timing.o \
> -				-o Native.so
> +OBJ		= $(RUNTIME_DIR)/mercury_timing.o Native.o
...
> +Native.pic_o:	$(OBJ)
> +		$(LINK_SHARED_OBJ) $(CFLAGS) $(CFLAGS_FOR_PIC) $(OBJ) \
> +		-o Native.pic_o

That won't work, because you are linking `.o' files (compiled without -fpic)
into a shared object file (which is misleading named `.pic_o' when it ought
to be named `.so').  Also, $(LINK_SHARED_OBJECT) might not be a C compiler,
so it may not understand $(CFLAGS).  Also, options (such as `-o') should
come before other command-line arguments (such as `$(OBJ)').

So I think you need something like this instead:

PIC_OBJS =	$(RUNTIME_DIR)/mercury_timing.$(EXT_FOR_PIC_OBJECTS) \
		Native.$(EXT_FOR_PIC_OBJECTS)

Native.$(EXT_FOR_SHARED_LIB): $(PIC_OBJS)
		$(LINK_SHARED_OBJ) -o Native.pic_o $(PIC_OBJS)
>  clean:
> -		rm -f Native.o Native.so
> +		rm -f Native.o Native.pic_o

You should remove both the .pic_o and the .so files.
The .pic_o files should be removed when you do `mmake clean',
but the .so files should only be removed when you do `mmake realclean'.

> --- configure.in	20 Jan 2004 23:02:16 -0000	1.384
> +++ configure.in	2 Feb 2004 05:10:01 -0000
> @@ -3701,6 +3701,7 @@
>  scripts/mkfifo_using_mknod bindist/bindist.INSTALL bindist/bindist.Makefile
>  scripts/mercury_config scripts/Mercury.config scripts/Mercury.config.bootstrap
>  tools/lmc tools/dotime runtime/mercury_dotnet.cs java/runtime/Constants.java
> +library/Mmakefile
>  ,
>  [
>  # Only do this when compiling the source, not when reconfiguring
> only in patch2:
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ library/Mmakefile.in	2 Feb 2004 05:49:15 -0000

The two changes above -- adding a library/Mmakefile.in file and
adding library/Mmakefile to the list of configure-generated files --
will result in the current library/Mmakefile being clobbered when
configure is run.

> +FULLARCH=@FULLARCH@
> 
> 
> +jars:	classes
> +	jar cf $(STD_LIB_NAME).jar mercury/*.class
> +	jar cf $(STD_LIB_NAME).runtime.jar mercury/runtime/*.class
> +	-+cd mercury/runtime && mmake $(NATIVE_SO)
> +	-cp mercury/runtime/$(NATIVE_SO) .

Please add a comment here explaining why the exit status of those last
two commands is ignored.

> +# This shared object is needed to run some of the standard library methods.
> +NATIVE_SO = Native.pic_o

s/.pic_o/$(EXT_FOR_SHARED_OBJECT)/

> 
> 
> +# Copy the jars and NATIVE_SO to INSTALL_JAVA_LIBRARY_DIR.
> +
> +.PHONY: install_library
> +install_library: jars
> +	mkdir -p $(INSTALL_JAVA_LIBRARY_DIR)
> +	cp $(JARS) $(INSTALL_JAVA_LIBRARY_DIR)
> +	cp $(NATIVE_SO) $(INSTALL_JAVA_LIBRARY_DIR)/$(FULLARCH)

The exit status for the last command should be ignored,
for the same reason as above.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------