Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix contrib/build_sysimg.jl #27629

Closed
wants to merge 4 commits into from
Closed

Fix contrib/build_sysimg.jl #27629

wants to merge 4 commits into from

Conversation

staticfloat
Copy link
Member

@staticfloat staticfloat commented Jun 18, 2018

The contrib/build_sysimg.jl script had bitrotted a bit with the 0.7 changes. This PR also incorporates @RalphAS's changes to address #27201 and #27451.

Marking this as WIP until testing is done on all major OS's:

  • Linux
  • OSX
  • FreeBSD
  • Windows

@ararslan could I ask you to test this out on FreeBSD? I create the "user image" file:

# /tmp/usrimg.jl
function foobar()
    println("It worked!")
end

Then I compile it into a custom sysimg:

$ ./julia --color=yes ./contrib/build_sysimg.jl /tmp/sysimg x86-64 /tmp/usrimg.jl

Finally, I test it:

$ ./julia -J /tmp/sysimg.so -e 'foobar()'
It worked!

@ViralBShah
Copy link
Member

ViralBShah commented Jun 18, 2018

I believe PackageCompiler.jl is the more general way to do this. But of course if we ship a script, we should make sure it works.

@ararslan
Copy link
Member

If this script is no longer the recommended way to accomplish this, can we simply delete the script and tell people to use PackageCompiler?

end
else
run(`$cc $FLAGS -o $sysimg_file $sysimg_path.o`)
if success(pipeline(cc_cmd; stdout=stdout, stderr=stderr))

This comment was marked as outdated.

@ararslan
Copy link
Member

Looks good on FreeBSD now 👍

@ararslan
Copy link
Member

Getting this on macOS:

[ Info: Linking sys.dylib with `cc -L/Users/alex/Projects/julia/usr/lib -shared -ljulia -o /Users/alex/stuff/sysimg.tmp -Wl,--whole-archive /Users/alex/stuff/sysimg.o -Wl,--no-whole-archive`
ld: unknown option: --whole-archive
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I think something like this should fix it:

--- a/contrib/build_sysimg.jl
+++ b/contrib/build_sysimg.jl
@@ -161,8 +161,16 @@ function link_sysimg(sysimg_path=nothing, cc=find_system_compiler(), debug=false
         push!(FLAGS, "-lssp")
     end

+    if Sys.isapple()
+        whole_archive = "-Xlinker -all_load"
+        no_whole_archive = ""
+    else
+        whole_archive = "-Wl,--whole-archive"
+        no_whole_archive = "-Wl,--no-whole-archive"
+    end
+
     sysimg_file = "$sysimg_path.$(Libdl.dlext)"
-    cc_cmd = `$cc $FLAGS -o $sysimg_path.tmp -Wl,--whole-archive $sysimg_path.o -Wl,--no-whole-archive`
+    cc_cmd = `$cc $FLAGS -o $sysimg_path.tmp $whole_archive $sysimg_path.o $no_whole_archive`
     @info("Linking sys.$(Libdl.dlext) with $cc_cmd")
     # Windows has difficulties overwriting a file in use so we first link to a temp file
     if success(pipeline(cc_cmd; stdout=stdout, stderr=stderr))

@staticfloat staticfloat changed the title [WIP] Fix contrib/build_sysimg.jl Fix contrib/build_sysimg.jl Jun 18, 2018
@tknopp
Copy link
Contributor

tknopp commented Jun 18, 2018

I always wondered why this needs to be a script and is not a regular function within julia. But I tend to agree with @ararslan that if PackageCompiler.jl does this and well tested, it might be better having the functionality only once.

@dgleich
Copy link
Contributor

dgleich commented Aug 10, 2018

Just a quick note on this. In the past I compiled custom sysimages to get things like fma operations to use native instructions when they were supported. First, this no longer seems to be needed. But second, the way I did this was: bin/julia share/julia/build_sysimg.jl --force which now seems to be a good way to ruin your julia install :)

I get the

ERROR: System image file failed consistency check: maybe opened the wrong version?
when trying to use the new one.

This is on linux with both julia 0.7.0 and julia 1.0.0 (after I modified the script to use @info and using Libdl)

@Gnimuc
Copy link
Contributor

Gnimuc commented Aug 18, 2018

I'm on macOS, also got this error "ERROR: System image file failed consistency check: maybe opened the wrong version?".

this PR works well(I forgot to checkout this PR before testing).

@SimonDanisch
Copy link
Contributor

If someone more familiar with correctly linking + outputting the shared libraries etc helps to keep PackageCompiler up to date, I'd very much welcome to just use PackageCompiler instead ;)

Otherwise it'd be nice to keep this script as a reference implementation!
It's always a bit of an adventure for me to figure out what exact flags && commands need to go into PackageCompiler after a Julia compiler change...

@staticfloat
Copy link
Member Author

I think we should just remove contrib/build_sysimg.jl and tell users to use PackageCompiler.jl instead. All in favor?

@staticfloat staticfloat closed this Dec 1, 2018
@ViralBShah
Copy link
Member

Please delete.

@staticfloat staticfloat deleted the sf/build_sysimg branch December 1, 2018 07:48
@mattcbro
Copy link

What? PackageCompiler.jl doesn't build post 1.0. I'm getting Libdl errors just trying to run default_sysimg_path() . I'm on Linux Mint 19. Are there any work arounds?

@staticfloat
Copy link
Member Author

I suggest you open an issue against PackageCompiler.jl if it isn't working for you; it should definitely work on 1.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants