fix embedding example, continued #21299

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

This is a rebased version of #20535, plus the changes I wanted. ~~However the example program crashes on startup for me; have not figured out the problem yet.~~

JeffBezanson · 2017-04-06T19:59:08Z

I'm getting this:

*** Error in `usr/libexec/embedding-debug': munmap_chunk(): invalid pointer: 0x00007f86b1559a28 ***

signal (6): Aborted
while loading no file, in expression starting on line 0
raise at /build/buildd/eglibc-2.19/signal/../nptl/sysdeps/unix/sysv/linux/raise.c:56
abort at /build/buildd/eglibc-2.19/stdlib/abort.c:89
__libc_message at /build/buildd/eglibc-2.19/libio/../sysdeps/posix/libc_fatal.c:175
malloc_printerr at /build/buildd/eglibc-2.19/malloc/malloc.c:4996
jl_init at /home/jeff/src/julia/usr/lib (unknown line)
main at /home/jeff/src/julia/examples/embedding/embedding.c:17
__libc_start_main at /build/buildd/eglibc-2.19/csu/libc-start.c:287
unknown function (ip: 0x400bb8)
Allocations: 881071 (Pool: 879743; Big: 1328); GC: 0
Aborted (core dumped)

Keno · 2017-04-06T20:12:05Z

src/jlapi.c

+
+    void *hdl = (void*)jl_load_dynamic_library_e(jl_is_debugbuild() ? "libjulia-debug" : "libjulia", JL_RTLD_DEFAULT);
+    if (hdl)
+        libjldir = dirname((char*)jl_pathname_for_handle(hdl));


dirname does not return a malloc'ed pointer.

Keno · 2017-04-06T20:12:43Z

src/jlapi.c

-    jl_init_with_image(julia_home_dir, NULL);
+    char *libjldir = NULL;
+
+    void *hdl = (void*)jl_load_dynamic_library_e(jl_is_debugbuild() ? "libjulia-debug" : "libjulia", JL_RTLD_DEFAULT);


Make sure that you're not loading libjulia twice here (though if jl_is_debugbuild() I guess it's ok).

If this is used for what I think it is, this kind of thing is usually handled using dladdr(&jl_init, ...) and equivalent.

Probably should be NULL rather than the library name, to get the this branch returns handle of libjulia that we want from here (thought I had changed this):

julia/src/dlload.c

Lines 131 to 145 in d5d7280

/*

this branch returns handle of libjulia

*/

if (modname == NULL) {

#ifdef _OS_WINDOWS_

if (!GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,

(LPCWSTR)(&jl_load_dynamic_library),

(HMODULE*)&handle)) {

jl_error("could not load base module");

}

#else

handle = dlopen(NULL, RTLD_NOW);

#endif

goto done;

}

ararslan · 2017-04-06T20:21:54Z

test/embedding.jl

@@ -0,0 +1,8 @@
+# test that the embedding example runs without error
+let
+    embedding_bin = joinpath(JULIA_HOME,"../libexec","embedding")


Should separate ".." from "libexec" in joinpath

JeffBezanson · 2017-04-06T22:20:40Z

Ok, as part of this I think I found a bug in jl_load_dynamic_library_ --- when called on NULL, it was supposed to return a handle for libjulia, but on unix systems called dlopen(NULL), which returns a handle for the executable. Somebody should check me on this.

codecov-io · 2017-04-06T23:27:52Z

Codecov Report

❗ No coverage uploaded for pull request base (master@c8026a9). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #21299   +/-   ##
=========================================
  Coverage          ?   50.24%           
=========================================
  Files             ?      270           
  Lines             ?    48978           
  Branches          ?        0           
=========================================
  Hits              ?    24610           
  Misses            ?    24368           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8026a9...c30e6f6. Read the comment docs.

JeffBezanson · 2017-04-07T15:20:01Z

Finally have this working on linux at least. One issue was that the example Makefile depends on Make.inc, which does not get installed. For now I'm building the example from the source directory, and having it put its output under the install prefix. However it would be nice to get it working with only the installed files. One approach might be to generate the example makefile, pasting in the needed paths and build parameters.

The next issue is that julia-config.jl is failing on OS X on Libdl.dlpath("libjulia"). Could somebody try it locally?

ihnorton · 2017-04-07T16:21:37Z

There's an off-by-one error when calling _dyld_image_name. This fixes it:

diff --git a/src/sys.c b/src/sys.c
index e391c469de..0e984cef70 100644
--- a/src/sys.c
+++ b/src/sys.c
@@ -668,7 +668,7 @@ JL_DLLEXPORT const char *jl_pathname_for_handle(void *handle)

 #ifdef __APPLE__
     // Iterate through all images currently in memory
-    for (int32_t i = _dyld_image_count(); i >= 0 ; i--) {
+    for (int32_t i = _dyld_image_count() - 1; i >= 0 ; i--) {
         // dlopen() each image, check handle
         const char *image_name = _dyld_get_image_name(i);
         void *probe_lib = jl_load_dynamic_library(image_name, JL_RTLD_DEFAULT);

JeffBezanson · 2017-04-07T16:29:48Z

Thank you!! Definitely shows the value of adding this to the tests!

JeffBezanson · 2017-04-07T16:55:44Z

embedding.exe still doesn't start on windows; looks like a dll issue. I tried putting the executable in C:\projects\julia\usr\bin, which I believe is where julia and its libraries are, but it still doesn't work. Any ideas?

JeffBezanson · 2017-04-07T21:47:40Z

Ok, @vtjnash pointed out that the error was a stack overflow, and we needed to add a stack size flag to julia-config.jl. Hopefully working now!

JeffBezanson · 2017-04-08T03:02:57Z

Yay!

tkelman · 2017-04-10T17:24:45Z

contrib/windows/msys_build.sh

@@ -206,3 +206,4 @@ cat Make.user
 make -j3 VERBOSE=1
 make build-stats
 #make debug
+make prefix=/c/projects/julia/usr VERBOSE=1 -C examples


this prefix is appveyor specific, can it be removed here?

When I remove this, it ends up looking for /c/projects/julia/examples/julia-251f61c05f/bin/julia.exe. How can I determine where julia was installed from the examples Makefile, or this script?

does this only run from the installed path?

I figured I should run the installed version, but it would work with either I think. This line is the only way it's been able to run any julia executable at all so far, because I have no idea where anything is in this configuration.

there's no make install executed on appveyor though...

All I want is for it to work. Here are a few things I've tried:

make prefix=/c/projects/julia/usr VERBOSE=1 -C examples in msys_build.sh --- works but hard-codes a path, not ideal.

make VERBOSE=1 -C examples in msys_build.sh --- fails attempting to find /c/projects/julia/examples/julia-251f61c05f/bin/julia.exe

C:\msys64\usr\bin\sh.exe --login -c 'make prefix=/c/projects/julia/usr VERBOSE=1 -C examples' in appveyor.yml --- fails saying it can't find examples.

What I need to do is (1) trigger a make in examples, (2) that process needs to be able to run julia julia-config.jl. Anything that accomplishes that is fine with me.

Ah okay it looks like prefix gets set relative to examples when examples/Makefile includes Make.inc. If we were to fix that, then make VERBOSE=1 install && make VERBOSE=1 -C examples in msys_build.sh would hopefully work.

Or we could make examples/Makefile refer to the Julia executable from build_bindir rather than bindir, but I don't know if we want that.

tkelman · 2017-04-10T17:26:41Z

contrib/julia-config.jl

@@ -23,6 +23,8 @@ function libDir()
    end
 end

+private_libDir() = joinpath(libDir(), "julia")


this isn't necessarily accurate if this was set to something different (like in distro packaging layouts)

Is dirname(imagePath()) correct here?

might have to worry about running this file while also having started julia with -J ?

Ok, then I think we should just go with what I have here and improve this script in the future as needed.

or save the makefile value as a relative-path constant

tkelman · 2017-04-10T17:38:13Z

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(void)


embedding docs need updating for this change

tkelman · 2017-04-10T17:41:41Z

test/embedding.jl

+    if is_windows()
+        embedding_bin = joinpath(JULIA_HOME,"embedding.exe")
+    else
+        embedding_bin = joinpath(JULIA_HOME,"..","libexec","embedding")


the path between JULIA_HOME and libexec may be set to something else

Is there any way to determine what it was set to?

Maybe easier to drop libexec and always put it in JULIA_HOME?

maybe, would have to read through the past discussions to remind myself why libexec was being used.

could save a relative path constant populated from the makefile like DATAROOTDIR if we keep it here

People objected (understandably) to globally installing a binary called "embedding":

#18688 (comment)

So I picked libexec, because it is designed for internal binaries (per the FHS, at least).

it may be preferable to do the same thing on windows too for the same reasons, just use withenv to temporarily append to path to include wherever libjulia is when running it in the test

I don't think we need to install this at all (this PR currently does not install it). I'll try adding a variable for libexecdir tho.

tkelman · 2017-04-12T21:09:17Z

.travis.yml

    - cd .. && mv julia julia2
    - /tmp/julia/bin/julia --precompiled=no -e 'true' &&
        /tmp/julia/bin/julia-debug --precompiled=no -e 'true'
    - /tmp/julia/bin/julia -e 'versioninfo()'
    - export JULIA_CPU_CORES=2 && export JULIA_TEST_MAXRSS_MB=600 &&
        cd /tmp/julia/share/julia/test &&
-        /tmp/julia/bin/julia --check-bounds=yes runtests.jl $TESTSTORUN &&
-        /tmp/julia/bin/julia --check-bounds=yes runtests.jl libgit2-online download pkg
+        /tmp/julia/bin/julia --check-bounds=yes runtests.jl embedding


to be reverted before merging, obviously

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

update manual for change to `jl_init`

always use libexec for embedding executable

tkelman · 2017-04-13T17:40:33Z

I'll finish up the last bit of this. Since libexecdir_rel is saving the path between the installed julia executable and libexecdir of the install tree, and test/embedding.jl refers to that relative path, I think we will need to install the executable. test/embedding.jl is simple enough that it actually should work to do Base.runtests("embedding") from a binary install, may as well make that possible. It won't exercise the compilation part, but that's okay.

In an out of tree build, or if you've set bindir, libexecdir, build_bindir, or build_libexecdir such that the install relative path isn't the same as the build_ relative path, then the setting of prefix here would mean make -C examples won't work, so I'll find a way to avoid needing to set it.

tkelman · 2017-04-14T12:37:44Z

Make.inc

@@ -177,7 +177,7 @@ endif
 USE_GPL_LIBS ?= 1

 # Directories where said libraries get installed to
-prefix ?= $(abspath julia-$(JULIA_COMMIT))
+prefix ?= $(BUILDROOT)/julia-$(JULIA_COMMIT)


@vtjnash can you think of any reason not to make this change? Part of the problem here was when examples/Makefile included Make.inc, the prefix was getting set to examples/julia-$(JULIA_COMMIT) which of course nothing else built into

dont set prefix in examples/Makefile install libexecdir on windows too run make install on appveyor use installed julia exe to run tests on appveyor

tkelman · 2017-04-14T18:05:56Z

I see now why building the embedding executable into build_libexecdir didn't work on Travis. Since julia-config outputs absolute rpaths, you have to pick: either build the embedding executable using build-tree Julia with build-tree rpaths, or build it using install-tree Julia with install-tree rpaths. Travis runs the tests from an install tree, with the build artifacts temporarily moved out of the way to identify this kind of issue. But make test-embedding would usually run from a build-tree Julia, so getting both to work would be a bit more complicated, probably involve patchelf'ing the embedding executable during make install.

I guess for now if you want make test-embedding to work locally in a source build then you'll have to do make -C examples prefix=usr like this PR was previously doing. It's kind of lying to the makefiles, and would need changing if you're doing an out-of-tree build. Maybe we revisit how this works if we ever decide we want to build this thing by default as a dependency of make install and have it shipped in the binary installers. Currently this doesn't do that unless we were to add a make -C examples line to the buildbots.
edit: and because the rpaths from julia-config are absolute, the embedding executable wouldn't run in a binary install as is anyway.

JeffBezanson · 2017-04-15T16:17:40Z

Yay!

tkelman · 2017-04-15T16:23:26Z

Maybe julia-config could have a flag for optionally outputting relative rpaths, in case there are use cases where it's more likely to want to move the whole julia tree including the embedding executable all together but keeping the same relative layout; vs right now where you could move the embedding executable wherever you want but you can't move libjulia to a different absolute path without breaking the embedding executable.

vtjnash · 2017-04-18T01:14:42Z

contrib/julia-config.jl

    else
-        return """$(threading_def)-DJULIA_INIT_DIR=\\"$arg1\\" -I$arg2"""
+        return """-std=gnu99 $(threading_def)-DJULIA_INIT_DIR=\\"$arg1\\" -I$arg2"""


I looked into why we can't use c99 here. it looks like the difference between these options is in the compiler flags that get set (https://www.gnu.org/software/libc/manual/html_node/Feature-Test-Macros.html). In particular, to use c99, we would need to set _POSIX_C_SOURCE >= 2001xxL (for a couple of pthread structs in the libuv header). When we get rid of the uv.h installation requirement (#8880), we'll only need to define _POSIX_SOURCE somewhere (for sigjmp_buf visibility).

JeffBezanson changed the title ~~fix embedding example, continued~~ WIP: fix embedding example, continued Apr 6, 2017

JeffBezanson mentioned this pull request Apr 6, 2017

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

Closed

JeffBezanson added this to the 0.6.0 milestone Apr 6, 2017

Keno reviewed Apr 6, 2017

View reviewed changes

ararslan added the embedding Embedding Julia using the C API label Apr 6, 2017

ararslan reviewed Apr 6, 2017

View reviewed changes

JeffBezanson force-pushed the ihnorton-fix_embedding branch from d199c9a to 3624866 Compare April 6, 2017 20:30

JeffBezanson changed the title ~~WIP: fix embedding example, continued~~ fix embedding example, continued Apr 6, 2017

JeffBezanson closed this Apr 6, 2017

JeffBezanson reopened this Apr 6, 2017

JeffBezanson force-pushed the ihnorton-fix_embedding branch from 3624866 to 131e8fb Compare April 6, 2017 22:19

JeffBezanson force-pushed the ihnorton-fix_embedding branch from 3b56638 to 0af7cea Compare April 7, 2017 15:11

JeffBezanson force-pushed the ihnorton-fix_embedding branch from 7fa0c00 to 997d41c Compare April 7, 2017 21:46

tkelman reviewed Apr 10, 2017

View reviewed changes

JeffBezanson mentioned this pull request Apr 10, 2017

Build, test, and install embedding example. #18688

Closed

tkelman reviewed Apr 12, 2017

View reviewed changes

JeffBezanson force-pushed the ihnorton-fix_embedding branch from dd13b16 to 707da31 Compare April 13, 2017 00:45

ihnorton and others added 5 commits April 12, 2017 23:17

Fix embedding example and simplify Makefile

8dd3123

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

Teach jl_init to find itself

0c123c1

update manual for change to `jl_init`

remove redundant jl_show entry point

1f05ba9

Run embedding test on appveyor and travis

87601aa

always use libexec for embedding executable

improve location of private_libdir in julia-config.jl

b4d01d9

JeffBezanson force-pushed the ihnorton-fix_embedding branch 2 times, most recently from 0a2c576 to b4d01d9 Compare April 13, 2017 03:56

tkelman reviewed Apr 14, 2017

View reviewed changes

tkelman force-pushed the ihnorton-fix_embedding branch 4 times, most recently from 843c574 to 9989c5e Compare April 14, 2017 14:05

Fix building of embedding example for out of tree build

c0d01c9

dont set prefix in examples/Makefile install libexecdir on windows too run make install on appveyor use installed julia exe to run tests on appveyor

tkelman force-pushed the ihnorton-fix_embedding branch from 9989c5e to c0d01c9 Compare April 14, 2017 14:13

tkelman merged commit f824e4b into master Apr 15, 2017

tkelman deleted the ihnorton-fix_embedding branch April 15, 2017 10:34

This was referenced Apr 15, 2017

MethodError calling jl_show from embedding API #20580

Closed

embedding issues #21137

Closed

This was referenced Apr 15, 2017

Install and test embedding examples #8757

Closed

Make embedding dirt-simple #18421

Closed

Embedding docs out of date #6327

Closed

vtjnash reviewed Apr 18, 2017

View reviewed changes

vtjnash mentioned this pull request Apr 21, 2017

try to bring some sanity to the embedding makefile #21427

Merged

ihnorton mentioned this pull request Jun 1, 2017

JLOptions.julia_bin incorrect when embedding #18437

Closed

ihnorton mentioned this pull request Jul 24, 2017

Add function to get libjulia path at runtime #5045

Closed

vtjnash mentioned this pull request May 3, 2023

Restore modname == NULL behavior for jl_load_dynamic_library #49611

Merged

	/*
	this branch returns handle of libjulia
	*/
	if (modname == NULL) {
	#ifdef _OS_WINDOWS_
	if (!GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS \| GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
	(LPCWSTR)(&jl_load_dynamic_library),
	(HMODULE*)&handle)) {
	jl_error("could not load base module");
	}
	#else
	handle = dlopen(NULL, RTLD_NOW);
	#endif
	goto done;
	}

fix embedding example, continued #21299

fix embedding example, continued #21299

Conversation

JeffBezanson commented Apr 6, 2017 • edited Loading

JeffBezanson commented Apr 6, 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

JeffBezanson commented Apr 6, 2017

codecov-io commented Apr 6, 2017 • edited Loading

Codecov Report

JeffBezanson commented Apr 7, 2017

ihnorton commented Apr 7, 2017

JeffBezanson commented Apr 7, 2017

JeffBezanson commented Apr 7, 2017

JeffBezanson commented Apr 7, 2017

JeffBezanson commented Apr 8, 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

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

tkelman commented Apr 13, 2017

Choose a reason for hiding this comment

tkelman commented Apr 14, 2017 • edited Loading

JeffBezanson commented Apr 15, 2017

tkelman commented Apr 15, 2017

Choose a reason for hiding this comment

JeffBezanson commented Apr 6, 2017 •

edited

Loading

codecov-io commented Apr 6, 2017 •

edited

Loading

tkelman commented Apr 14, 2017 •

edited

Loading