Update ccalltest #13077

staticfloat · 2015-09-11T20:36:49Z

Move ccalltest.c to src, install into private_libdir, ensure symbols are public.

Addresses #12779

nalimilan · 2015-09-11T20:45:53Z

Ah, it's definitely good to have you around!

vtjnash · 2015-09-11T21:34:51Z

src/Makefile

probably will need to make corresponding removals in test/Makefile

Oh, weird, I somehow didn't get those changes included. Thanks!

should also be cleaned

i think the ccalltest binary has reached the limit of its usefulness. at this point, it's pretty much just a test of whether you have a compiler (and a pretty poor test of that too). I recommend just removing this Makefile rule and the corresponding function main from ccalltest.c.

I was wondering what it was used for, but didn't want to muck with things
beyond my knowledge. Will do.
-E

On Sat, Sep 12, 2015 at 5:24 PM, Jameson Nash notifications@github.com
wrote:

In src/Makefile
#13077 (comment):

@@ -82,6 +82,14 @@ $(BUILDDIR)/%.o: %.cpp $(HEADERS) $(shell which $(LLVM_CONFIG)) | $(BUILDDIR)
$(BUILDDIR)/%.dbg.obj: %.cpp $(HEADERS) $(shell which $(LLVM_CONFIG)) | $(BUILDDIR)
@$(call PRINT_CC, $(CXX) $(call exec,$(LLVM_CONFIG) --cxxflags) $(CPPFLAGS) $(CXXFLAGS) $(DEBUGFLAGS) -c $< -o $@)

+libccalltest: $(build_shlibdir)/libccalltest.$(SHLIB_EXT)
+$(build_shlibdir)/libccalltest.$(SHLIB_EXT): ccalltest.c

@$(call PRINT_CC, $(CC) $(CFLAGS) $(DEBUGFLAGS) -O3 $< $(fPIC) -shared -o $@ $(LDFLAGS) -DCC="$(CC)")
+$(BUILDDIR)/ccalltest: ccalltest.c $(BUILDDIR)/libccalltest.$(SHLIB_EXT)

@$(call PRINT_CC, $(CC) $(CFLAGS) $(DEBUGFLAGS) -O3 $< -o $@ $(LDFLAGS) -DCC="$(CC)")

i think the ccalltest binary has reached the limit of its usefulness. at
this point, it's pretty much just a test of whether you have a compiler
(and a pretty poor test of that too). I recommend just removing this
Makefile rule and the corresponding function main from ccalltest.c.

—
Reply to this email directly or view it on GitHub
https://github.com/JuliaLang/julia/pull/13077/files#r39342989.

staticfloat · 2015-09-12T18:06:34Z

Ah, I was originally putting it in a place that didn't require cleaning (in $(BUILDDIR)) but yes, now it needs to be cleaned again. This is now ready to merge.

tkelman · 2015-09-12T18:08:24Z

mind squashing a bit?

staticfloat · 2015-09-12T18:10:19Z

Squished and squashed.

vtjnash · 2015-09-13T00:18:56Z

test/Makefile

i'm not exactly sure what gmake will do with this line (all:), probably best just to delete it

Ah, I missed that all gets expanded out of $(TESTS).

vtjnash · 2015-09-13T00:28:15Z

src/ccalltest.c

for other reviewers: this change shouldn't be necessary, since dllexport is the default for Mac, Linux, and Mingw64 compilers. However, it could perhaps be somewhat useful if dealing with MSVC in the future.

msvc can't really build this file as it stands right now since it doesn't support C-style complex

i think it should only really matter if msvc tried to link against the resulting file (even if mingw compiled it), which is even less likely however. and none of that matters unless you are also going to create the lib.a file

dll visibility matters for ccall - the default with mingw is only to export everything if there are no export attributes present. If you have a single dllexport that is explicitly specified, then only that function gets exported (unless you link with -Wl,--export-all-symbols). So it either needs to be nowhere or on everything you want to use.

Actually, the reason I added these DLLEXPORT things is because on Linux, with GCC, the symbols weren't accessible. nm showed them as 't' instead of 'T' which apparently means "local", but I'm honestly a little fuzzy as to what exactly that means. It's possible that what Tony just said is the culprit; something else was getting exported which caused everything else to be non-visible, but at least things are more explicit this way.

CFLAGS has a special meaning inside of src/Makefile due to an override at the top

Good point; I've removed CFLAGS and replaced it with CPPFLAGS now.

it should still have CFLAGS as well, to accommodate where-ever the user's environment is injecting the flags

Oh, I thought that since we're setting CFLAGS to JCFLAGS, we didn't want to pass them. I'll reinstate CFLAGS alongside CPPFLAGS then.

it contains a lot of unnecessary cruft, but it might contain user flags

tkelman · 2015-09-13T16:11:14Z

This okay to merge?

…are public

…ned as well

staticfloat · 2015-09-13T22:19:11Z

I think it is now ready to merge, if @vtjnash agrees then we're ready to go.

Update ccalltest

…are public (cherry picked from commit c440739) ref #13077

tkelman · 2015-09-16T04:08:10Z

backported in #13107

nalimilan mentioned this pull request Sep 11, 2015

Don't install test/libccalltest.so #12779

Closed

vtjnash reviewed Sep 11, 2015
View reviewed changes

tkelman added building Build system, or building Julia or its dependencies backport pending 0.4 labels Sep 12, 2015

tkelman mentioned this pull request Sep 12, 2015

make O=build out-of-tree #12463

Merged

staticfloat force-pushed the sf/ccalltest branch 2 times, most recently from b7e6297 to a2d4016 Compare September 12, 2015 18:09

vtjnash reviewed Sep 13, 2015
View reviewed changes

staticfloat force-pushed the sf/ccalltest branch from 359cd46 to 9afed54 Compare September 13, 2015 00:26

vtjnash reviewed Sep 13, 2015
View reviewed changes

staticfloat force-pushed the sf/ccalltest branch from 9afed54 to 853bfbd Compare September 13, 2015 06:09

staticfloat added 3 commits September 13, 2015 15:17

Move ccalltest.c to src, install into private_libdir, ensure symbols …

c440739

…are public

On OSX, libraries have .dSYM folders next to them that should be clea…

ffe0d53

…ned as well

Cull ccalltest executable as it has outlived its usefulness

9cc67dc

staticfloat force-pushed the sf/ccalltest branch from 853bfbd to 9cc67dc Compare September 13, 2015 22:18

vtjnash added a commit that referenced this pull request Sep 14, 2015

Merge pull request #13077 from JuliaLang/sf/ccalltest

a2c9f69

Update ccalltest

vtjnash merged commit a2c9f69 into master Sep 14, 2015

vtjnash deleted the sf/ccalltest branch September 14, 2015 02:34

staticfloat added a commit that referenced this pull request Sep 14, 2015

Move ccalltest.c to src, install into private_libdir, ensure symbols …

5369837

…are public (cherry picked from commit c440739) ref #13077

tkelman removed the backport pending 0.4 label Sep 16, 2015

Uh oh!

Update ccalltest #13077

Update ccalltest #13077

Uh oh!

Conversation

staticfloat commented Sep 11, 2015

Uh oh!

nalimilan commented Sep 11, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

staticfloat commented Sep 12, 2015

Uh oh!

tkelman commented Sep 12, 2015

Uh oh!

staticfloat commented Sep 12, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tkelman commented Sep 13, 2015

Uh oh!

staticfloat commented Sep 13, 2015

Uh oh!

tkelman commented Sep 16, 2015

Uh oh!

Uh oh!