Fix embedding example, simplify Makefile, and test it #20535

ihnorton · 2017-02-09T05:46:18Z

get rid of makefile cruft
build in-place instead of putting in usr/bin
use julia-config for flags, as documented in manual

(related: #18421)

tkelman · 2017-02-09T05:53:10Z

examples/Makefile

@@ -1,41 +1,25 @@
-JULIAHOME = $(abspath ..)
-include $(JULIAHOME)/Make.inc
+JULIA = $(abspath ../usr/bin/julia)


this isn't right for an out-of-tree build

tkelman · 2017-02-09T05:53:28Z

examples/Makefile

-JULIAHOME = $(abspath ..)
-include $(JULIAHOME)/Make.inc
+JULIA = $(abspath ../usr/bin/julia)
+JL_SHARE = $(shell $(JULIA) -e 'print(joinpath(JULIA_HOME,Base.DATAROOTDIR,"julia"))')


may require spawn

tkelman · 2017-02-09T05:53:54Z

examples/Makefile

-ifneq ($(UNTRUSTED_SYSTEM_LIBM),0)
-JLDFLAGS += $(WHOLE_ARCHIVE) $(build_libdir)/libopenlibm.a $(NO_WHOLE_ARCHIVE)
-endif
+ifeq ($(OS),Windows_NT)


not correct for cross-compiling

tkelman · 2017-02-09T05:54:56Z

src/julia.h

@@ -1313,6 +1313,7 @@ JL_DLLEXPORT void julia_init(JL_IMAGE_SEARCH rel);
 JL_DLLEXPORT void jl_init(const char *julia_home_dir);
 JL_DLLEXPORT void jl_init_with_image(const char *julia_home_dir,
                                     const char *image_relative_path);
+JL_DLLEXPORT const char* jl_get_default_sysimg_path();


if exported, needs JL_DLLEXPORT on both prototype and implementation

ihnorton · 2017-02-10T06:00:51Z

Updated, thanks. I'm going for non-reliance on Make.inc, so that this can serve as a usable template. However, added checks for spawn and BUILDROOT independently (to handle cross-compile and out-of-tree). Hopefully sufficient, though I'm not set up to test those right now.

I'll note that this example isn't built at all right now with out-of-tree.

ihnorton · 2017-02-10T15:10:46Z

Win64 and OSX test failures seem unrelated.

tkelman · 2017-02-13T00:45:22Z

examples/Makefile

-
-ifeq ($(USE_SYSTEM_LIBM),0)
-ifneq ($(UNTRUSTED_SYSTEM_LIBM),0)
-JLDFLAGS += $(WHOLE_ARCHIVE) $(build_libdir)/libopenlibm.a $(NO_WHOLE_ARCHIVE)


does julia-config handle this and the llvm-config output?

libm yes, llvm-config no. Doesn't libjulia link llvm already?

yes but presumably the llvm-config --ldflags call was needed for something

That was added in 072a861 when the file was created. My guess is copy-paste or prior/local build issue. If it breaks the example we can put it back.

tkelman · 2017-02-13T00:46:38Z

test/examples.jl

+
+# test that the embedding example runs without error
+let
+    lines = readlines(pipeline(`$(joinpath(JULIA_HOME,Base.DATAROOTDIR,"doc","julia","examples","embedding","embedding"))`, stderr=DevNull))


wrap long line

tkelman · 2017-02-13T00:48:21Z

examples/embedding/Makefile

+	$(CC) $^ -o $@ $(CFLAGS) $(DEBUGFLAGS)
+
+clean:
+	rm -f *.o *.do


doesn't look like this will create any object files any more

tkelman · 2017-02-13T00:54:13Z

we shouldn't install binaries in the same place as documentation and source code. installing and redistributing the compiled embedding results isn't the most useful test of this, see my comments on #18688

ihnorton · 2017-02-13T04:56:15Z

installing and redistributing the compiled embedding results isn't the most useful test of this, see my comments on #18688

Ok:

I put the binary in libexec in the latest update, because I agree with Yichao (and your thumbs-up) in that thread that this could be a useful test of relocatability, which I will have a follow-up PR to make possible.
I made the test optional, as you requested in that thread, and added to appveyor/travis scripts.

tkelman · 2017-02-13T19:22:36Z

examples/embedding/Makefile

+# hooks for julia build system
+ifdef BUILDROOT
+  SRCDIR := $(abspath $(dir $(lastword $(MAKEFILE_LIST))))/
+  BINDIR := $(abspath $(BUILDROOT)/usr/libexec)/


build_libexecdir

tkelman · 2017-02-16T18:45:37Z

examples/embedding/Makefile

+  $(error "Please set JULIA to the path of the target julia binary!")
+endif
+ifndef BIN
+  $(error "Please set BIN to the path of the build directory!")


could maybe use a bit more clarification what is meant by this

tkelman · 2017-03-23T16:03:50Z

examples/embedding/Makefile

+$(BIN)/embedding$(EXE): embedding.c
+	$(CC) $^ -o $@ $(CFLAGS)
+
+$(BINDIR)/embedding-debug$(EXE): embedding.c


should this also be BIN ?

tkelman · 2017-03-23T16:06:39Z

examples/embedding/Makefile

+debug:   $(BIN)/embedding-debug$(EXE)
+
+$(BIN)/embedding$(EXE): embedding.c
+	$(CC) $^ -o $@ $(CFLAGS)


might want to also explicitly error if CC isn't set, or default to cc

make already does that

- get rid of makefile cruft - use julia-config for flags, as documented in manual - build into `libexec` also move to subdir for clarity.

tkelman · 2017-03-24T04:35:32Z

contrib/julia-config.jl

@@ -70,9 +70,9 @@ function cflags()
    arg2 = replace(includeDir(),"\\","\\\\")
    threading_def = threadingOn() ? "-DJULIA_ENABLE_THREADING=1 " : ""
    if is_unix()
-        return """$(threading_def)-fPIC -DJULIA_INIT_DIR=\\"$arg1\\" -I$arg2"""
+        return """-std=gnu99 $(threading_def)-fPIC -DJULIA_INIT_DIR=\\"$arg1\\" -I$arg2"""


does the runtime need this, or the code in the embedding example?

the runtime.

are we using gnu extensions in our headers, or which compiler versions don't default to c99 mode?

JeffBezanson · 2017-03-24T19:04:25Z

examples/embedding/embedding.c

@@ -14,7 +14,7 @@ double my_c_sqrt(double x)

 int main()
 {
-    jl_init(NULL);
+    jl_init(JULIA_INIT_DIR);


Does this still work with NULL as the argument?

Oh, even better, it looks like jl_init takes no arguments now?

JeffBezanson · 2017-03-24T19:06:46Z

src/jlapi.c

    julia_init(JL_IMAGE_JULIA_HOME);
    jl_exception_clear();
 }

-JL_DLLEXPORT void jl_init(const char *julia_home_dir)
+JL_DLLEXPORT void jl_init()


Should be jl_init(void).

JeffBezanson · 2017-03-24T19:07:27Z

src/julia.h

 JL_DLLEXPORT int jl_is_initialized(void);
 JL_DLLEXPORT void jl_atexit_hook(int status);
 JL_DLLEXPORT void JL_NORETURN jl_exit(int status);
+JL_DLLEXPORT const char* jl_pathname_for_handle(void *handle);
+JL_DLLEXPORT const char* jl_get_default_sysimg_path();


This is duplicated and needs (void).

JeffBezanson · 2017-03-28T18:48:10Z

Bump.

JeffBezanson · 2017-04-06T19:57:49Z

Moved to #21299.

ihnorton added the embedding Embedding Julia using the C API label Feb 9, 2017

tkelman reviewed Feb 9, 2017

View reviewed changes

ihnorton force-pushed the fix_embedding branch from a1be72d to 0638ec2 Compare February 10, 2017 05:50

ihnorton mentioned this pull request Feb 12, 2017

MethodError calling jl_show from embedding API #20580

Closed

ihnorton force-pushed the fix_embedding branch from 0638ec2 to 9378555 Compare February 13, 2017 00:10

tkelman reviewed Feb 13, 2017

View reviewed changes

ihnorton force-pushed the fix_embedding branch from 9378555 to 128f941 Compare February 13, 2017 04:48

ihnorton changed the title ~~Fix embedding example and simplify Makefile~~ Fix embedding example, simplify Makefile, and test it Feb 13, 2017

tkelman reviewed Feb 13, 2017

View reviewed changes

ihnorton force-pushed the fix_embedding branch 2 times, most recently from dcfd49a to 69ea5ea Compare February 16, 2017 18:14

tkelman reviewed Feb 16, 2017

View reviewed changes

ihnorton force-pushed the fix_embedding branch 4 times, most recently from 2ebb8db to f4312aa Compare February 26, 2017 04:31

ihnorton force-pushed the fix_embedding branch from f4312aa to 143ef51 Compare March 22, 2017 20:27

ihnorton mentioned this pull request Mar 22, 2017

fix embedding example and default sysimg path #21138

Closed

JeffBezanson added this to the 0.6.0 milestone Mar 22, 2017

vtjnash mentioned this pull request Mar 23, 2017

Immediately expand JULIAHOME in Makefile #21140

Merged

ihnorton force-pushed the fix_embedding branch from 143ef51 to 4e0b3af Compare March 23, 2017 15:02

ihnorton force-pushed the fix_embedding branch from 4e0b3af to af4a7ca Compare March 23, 2017 15:44

tkelman reviewed Mar 23, 2017

View reviewed changes

ihnorton force-pushed the fix_embedding branch from af4a7ca to 74a8bf0 Compare March 24, 2017 01:21

ihnorton and others added 7 commits March 24, 2017 00:18

Fix 20580, embedding example crash

9f91c5f

Add test/embedding.jl

4df564d

julia-config needs -std=gnu99

3271fe2

Fix embedding example and simplify Makefile

e8c1b39

- get rid of makefile cruft - use julia-config for flags, as documented in manual - build into `libexec` also move to subdir for clarity.

Run embedding test on appveyor and travis

5540447

Install libexec on Windows

1daed5d

Teach jl_init to find itself

ffb0644

ihnorton force-pushed the fix_embedding branch from 74a8bf0 to ffb0644 Compare March 24, 2017 04:19

tkelman reviewed Mar 24, 2017

View reviewed changes

JeffBezanson reviewed Mar 24, 2017

View reviewed changes

JeffBezanson requested changes Mar 24, 2017

View reviewed changes

JeffBezanson self-assigned this Apr 6, 2017

JeffBezanson mentioned this pull request Apr 6, 2017

fix embedding example, continued #21299

Merged

JeffBezanson closed this Apr 6, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix embedding example, simplify Makefile, and test it #20535

Fix embedding example, simplify Makefile, and test it #20535

ihnorton commented Feb 9, 2017

tkelman Feb 9, 2017

tkelman Feb 9, 2017

tkelman Feb 9, 2017

tkelman Feb 9, 2017

ihnorton commented Feb 10, 2017

ihnorton commented Feb 10, 2017

tkelman Feb 13, 2017

ihnorton Feb 13, 2017

tkelman Feb 13, 2017

ihnorton Feb 13, 2017

tkelman Feb 13, 2017

tkelman Feb 13, 2017

tkelman commented Feb 13, 2017

ihnorton commented Feb 13, 2017

tkelman Feb 13, 2017

tkelman Feb 16, 2017

tkelman Mar 23, 2017

tkelman Mar 23, 2017

vtjnash Mar 23, 2017

tkelman Mar 24, 2017

ihnorton Mar 24, 2017

tkelman Mar 24, 2017

JeffBezanson Mar 24, 2017

JeffBezanson Mar 24, 2017

JeffBezanson Mar 24, 2017

JeffBezanson Mar 24, 2017

JeffBezanson commented Mar 28, 2017

JeffBezanson commented Apr 6, 2017

Fix embedding example, simplify Makefile, and test it #20535

Fix embedding example, simplify Makefile, and test it #20535

Conversation

ihnorton commented Feb 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ihnorton commented Feb 10, 2017

ihnorton commented Feb 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman commented Feb 13, 2017

ihnorton commented Feb 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeffBezanson commented Mar 28, 2017

JeffBezanson commented Apr 6, 2017