Skip to content

Conversation

@waTeim
Copy link
Contributor

@waTeim waTeim commented Nov 24, 2014

This is a modification to the top level Make.inc and Makefile in src to allow for additional link parameters to passed to the shared library assembly step.

Additionally it is utilized to pass -Bsymbolic-functions when compiling on Linux. Fixes #8864

@ViralBShah ViralBShah added building Build system, or building Julia or its dependencies system:linux Affects only Linux labels Nov 24, 2014
@tkelman
Copy link
Contributor

tkelman commented Nov 24, 2014

I'm a little confused why JLDFLAGS only applies to the executable and not libjulia. That seems a bit strange to me.

@waTeim
Copy link
Contributor Author

waTeim commented Nov 24, 2014

Flexibility maybe? Potentially the executable needs a flag that doesn't make sense for the library and visa/versa. Candidates for this might be -strip, -L for some directory of libraries the the executable needs but the library doesn't

Historical? For a while LDFLAGS was good enough for everything, but then the julia executable needed something extra, so instead of a review and refactor, JLDFLAGS was invented for just julia.

@tkelman
Copy link
Contributor

tkelman commented Nov 24, 2014

I'm pretty sure the history on why we're avoiding using the standard flags names for most things is so distribution packagers can have an easier time using build systems that overwrite those flags.

@waTeim
Copy link
Contributor Author

waTeim commented Nov 25, 2014

Yea that makes sense why LDFLAGS is left alone.

It looks like there are flags added to Makefile for various reasons. So maybe there was no overarching reason, just was convenience.

For example

ifneq ($(USEMSVC), 1)
CXXLD = $(CXX) -shared
else
CXXLD = $(LD) -dll -export:jl_setjmp -export:jl_longjmp
endif

override CFLAGS += $(JCFLAGS)
override CXXFLAGS += $(JCXXFLAGS)
override CPPFLAGS += $(JCPPFLAGS)

@tkelman
Copy link
Contributor

tkelman commented Nov 25, 2014

I added CXXLD, it's because MSVC can't link a shared library through the cl compiler driver if you call it only on object files. Not so sure on the overriding there.

@waTeim
Copy link
Contributor Author

waTeim commented Nov 25, 2014

Well that makes me feel a little better about what I'm proposing. There are flags that are usable/make sense only in the context of assembling the shared library and only in the context of making the executable
.

@tkelman
Copy link
Contributor

tkelman commented Nov 25, 2014

Alrighty, I guess this is good enough for now then. If you rebase to fix the conflict then I'll merge.

@waTeim waTeim force-pushed the waT/symbf branch 2 times, most recently from 70c299d to 34105d3 Compare November 25, 2014 19:09
@waTeim
Copy link
Contributor Author

waTeim commented Nov 25, 2014

Whew that took a while because in between the pull request and now, a change to Makefile happened, I merged in that change, which was essentially adding BUILDDIR to flisp, etc:
e.g.

flisp/libflisp-debug.a support/libsupport-debug.a $(LIBUV)

became

$(BUILDDIR)/flisp/libflisp-debug.a $(BUILDDIR)/support/libsupport-debug.a $(LIBUV)

@tkelman
Copy link
Contributor

tkelman commented Nov 25, 2014

@staticfloat this look okay to you?

@staticfloat
Copy link
Member

LGTM.

@tkelman
Copy link
Contributor

tkelman commented Nov 25, 2014

Whew that took a while because in between the pull request and now, a change to Makefile happened

Yep that's what I meant by "rebase to fix the conflict." Good practice, happens often on more complicated PR's. Enough people looking at the same set of code, bound to step on each other's toes once in a while.

Don't think this can break anything, and since it helps with your embedding, we'll put it on the list to backport in a few days' time.

tkelman added a commit that referenced this pull request Nov 25, 2014
Introduce a new variable in Make.inc that applies only to libjulia.so
@tkelman tkelman merged commit b780b80 into JuliaLang:master Nov 25, 2014
@waTeim
Copy link
Contributor Author

waTeim commented Nov 25, 2014

Rebasing, yea, I see the reason for it now even though at the time, I'm like "ugh".

Backporting: That's awesome!

@tkelman
Copy link
Contributor

tkelman commented Nov 25, 2014

Why the ... would this cause failures on the parallel test on Travis? @amitmurthy @ViralBShah any ideas? I think I've been seeing this intermittently for a few days so I don't think it's the fault of this PR.

https://travis-ci.org/JuliaLang/julia/jobs/42121382

@waTeim
Copy link
Contributor Author

waTeim commented Nov 25, 2014

Especially since just 1 hour ago it ran to completion on both OS/X and Linux. Can you save this result the to side and simply restart?

@tkelman
Copy link
Contributor

tkelman commented Nov 25, 2014

I don't think there's a good way of saving old build logs if you restart a build. Anyway the next build passed, so it's definitely intermittent and not your fault. https://travis-ci.org/JuliaLang/julia/builds/42131222

It was just an unfortunate coincidence that 2 merges I did in a row both had this failure on the merge commit into master, but passed fine on the PR test. I'll open a new issue if it keeps appearing.

@waTeim
Copy link
Contributor Author

waTeim commented Nov 25, 2014

Ironically, is it possibly related somehow to #8551, because a couple days (since Saturday), and, you know, tasks....

@amitmurthy
Copy link
Contributor

The parallel test failed because worker 9 terminated unexpectedly. Sometimes the cause is memory corruption dues to previous tests run by the terminating worker.

In this case worker 9 executed

        From worker 9:       * linalg/pinv
        From worker 9:       * strings
        From worker 9:       * collections
        From worker 9:       * reducedim
        From worker 9:       * dsp
        From worker 9:       * bitarray

It will be good to see which are the common tests being executed by terminating workers across failed Travis runs.

@tkelman
Copy link
Contributor

tkelman commented Nov 26, 2014

@waTeim
Copy link
Contributor Author

waTeim commented Nov 26, 2014

This one failed at a slightly different location

Previous

Exception on 6: Worker 9 terminated.
ERROR: test error in expression: fetch(@spawnat id_other myid()) == id_other
ProcessExitedException()
 in wait at ./task.jl:284
 in wait at ./task.jl:194
 in wait_full at ./multi.jl:576
 in remotecall_fetch at multi.jl:676
 in call_on_owner at ./multi.jl:723
 in fetch at multi.jl:731
 in anonymous at test.jl:83
 in do_test at test.jl:47
 in runtests at /tmp/julia/share/julia/test/testdefs.jl:5
 in anonymous at multi.jl:828
 in run_work_thunk at multi.jl:593
 in anonymous at task.jl:828 
while loading parallel.jl, in expression starting on line 11
ERROR: ProcessExitedException()
 in anonymous at task.jl:1452
while loading /tmp/julia/share/julia/test/runtests.jl, in expression starting on line 42
The command "cd /tmp/julia/share/julia/test && /tmp/julia/bin/julia-debug --check-bounds=yes runtests.jl all && /tmp/julia/bin/julia-debug --check-bounds=yes runtests.jl pkg" exited with 1.

this time.

Worker 9 terminated.
exception on 8: ERROR: ProcessExitedException()
in worker_from_id at ./multi.jl:331
 in call_on_owner at ./multi.jl:723
 in wait at ./multi.jl:728
 in SharedArray at sharedarray.jl:71
 in runtests at /tmp/julia/share/julia/test/testdefs.jl:5
 in anonymous at multi.jl:828
 in run_work_thunk at multi.jl:593
 in anonymous at task.jl:828
while loading parallel.jl, in expression starting on line 35
ERROR: ProcessExitedException()
 in anonymous at task.jl:1452
while loading /tmp/julia/share/julia/test/runtests.jl, in expression starting on line 42

@amitmurthy
Copy link
Contributor

In this worker 9 previously executed

        From worker 9:       * linalg/pinv
        From worker 9:       * strings
        From worker 9:       * collections
        From worker 9:       * reducedim
        From worker 9:       * fft
        From worker 9:       * bitarray

Any recent changes to bitarray?

tkelman pushed a commit that referenced this pull request Dec 2, 2014
(cherry picked from commit 34105d3)
backport of #9130
Conflicts:
	Make.inc
	src/Makefile
@tkelman
Copy link
Contributor

tkelman commented Dec 2, 2014

backported in eb95148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

building Build system, or building Julia or its dependencies system:linux Affects only Linux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants