Compat writemime, strings, os, startup file with some tests #434

Merged
merged 2 commits into from Jul 25, 2016

Projects

None yet

5 participants

@wookay
Contributor
wookay commented Jun 25, 2016 edited

deprecated writemime to show.
thanks.

@tkelman
Member
tkelman commented Jun 25, 2016

Would be good to update the minimum required version of Compat in REQUIRE to the first tag that implements these features.

@wookay wookay changed the title from deprecated writemime to show to WIP deprecated writemime to show Jun 26, 2016
@wookay
Contributor
wookay commented Jun 26, 2016

actually, it's not working for Julia 0.4 and 0.5
I'm trying to investigate both of IJulia.jl and Interact.jl.
then will commit again with proper REQUIRE.

thanks.

@tkelman tkelman and 1 other commented on an outdated diff Jun 30, 2016
@@ -0,0 +1,16 @@
+if VERSION >= v"0.5-"
+ using Base.Test
+else
+ using BaseTestNext
+ const Test = BaseTestNext
+end
+
+import IJulia: init, Comm
+
+profile = normpath(Pkg.dir("IJulia"), "test", "profile.json")
@tkelman
tkelman Jun 30, 2016 Member

use dirname(@__FILE__) rather than Pkg.dir

@wookay
wookay Jun 30, 2016 Contributor

applied quickly, thanks :)

@stevengj stevengj and 2 others commented on an outdated diff Jun 30, 2016
@@ -1,6 +1,6 @@
julia 0.3
-Nettle 0.2
-JSON 0.5
-ZMQ 0.3
-Compat 0.7.15
-Conda 0.1.5
+Compat 0.8.0
+Nettle 0.2.2
+JSON 0.5.2
+ZMQ 0.3.2
@stevengj
stevengj Jun 30, 2016 Member

Why did you need to increment all of these version numbers?

@wookay
wookay Jul 1, 2016 Contributor

for Compat 0.8.0

  • bytestring(::Ptr, ...) has been replaced with unsafe_string

for other packages, keeping the version numbers.

@tkelman
tkelman Jul 1, 2016 Member

0.5.2 isn't a good tag of JSON, it was accidentally breaking and later 0.5.x tags were actually earlier code

@stevengj
stevengj Jul 4, 2016 edited Member

You should just need to bump the Compat version number. The only reason to increment the versions of the other packages is if we actually require things in the newer versions (i.e. it doesn't work at all with earlier versions — deprecation warnings are okay).

@wookay
wookay Jul 5, 2016 Contributor

I have come to realize that the compatibility of the dependent packages.
I was not mind carefully enough to that. I'll take care.
I found these lower bounds within the commit.
Compat.jl : v0.7.20 for is_apple, is_windows
ZMQ.jl : v0.3.1 to fix FDWatcher error during test

@stevengj stevengj and 1 other commented on an outdated diff Jun 30, 2016
deps/build.jl
@@ -62,7 +62,7 @@ juliakspec = abspath(spec_name)
binary_name = @windows? "julia.exe":"julia"
kernelcmd_array = String[joinpath(JULIA_HOME,("$binary_name")), "-i"]
ijulia_dir = get(ENV, "IJULIA_DIR", Pkg.dir("IJulia")) # support non-Pkg IJulia installs
-append!(kernelcmd_array, ["-F", joinpath(ijulia_dir,"src","kernel.jl"), "{connection_file}"])
+append!(kernelcmd_array, ["--startup-file=yes", joinpath(ijulia_dir,"src","kernel.jl"), "{connection_file}"])
@stevengj
stevengj Jun 30, 2016 Member

What is the reason for this change?

@tkelman
tkelman Jun 30, 2016 Member

it's deprecated on 0.5

@stevengj stevengj commented on an outdated diff Jun 30, 2016
src/comm_manager.jl
@@ -54,10 +54,10 @@ function comm_info_request(sock, msg)
comms
end
- _comms = map(reply) do x
- x[1] => @compat Dict(:target_name, comm_target(x[2]))
+ _comms = map(collect(reply)) do x
@stevengj
stevengj Jun 30, 2016 Member

Why is collect needed here?

@stevengj stevengj commented on an outdated diff Jun 30, 2016
@@ -2,6 +2,18 @@ import Base.show
export Msg, msg_pub, msg_reply, send_status, send_ipython
+# Julia commit 77631ce7f615059c7021756ef539d36964af13dc
+if VERSION >= v"0.5-dev+4194"
+ import Base.unsafe_string
+else
+ const unsafe_string = Base.bytestring
+end
+
+if length(methods(unsafe_string, (ZMQ.Message,))) == 0
+ unsafe_string(zmsg::ZMQ.Message) = unsafe_string(pointer(zmsg), length(zmsg))
+end
@stevengj
stevengj Jun 30, 2016 Member

This should be patched in ZMQ.jl, not here. Until ZMQ is patched, we can live with the deprecation message.

@stevengj stevengj commented on an outdated diff Jun 30, 2016
@@ -176,7 +178,7 @@ end
function oslibuv_flush()
#refs: https://github.com/JuliaLang/IJulia.jl/issues/347#issuecomment-144505862
# https://github.com/JuliaLang/IJulia.jl/issues/347#issuecomment-144605024
- @windows_only ccall(:SwitchToThread, stdcall, Void, ())
+ Compat.is_windows() && ccall(:SwitchToThread, stdcall, Void, ())
@stevengj
stevengj Jun 30, 2016 Member

No need for Compat.

@stevengj stevengj commented on an outdated diff Jun 30, 2016
@@ -5,6 +5,8 @@
# logging in verbose mode goes to original stdio streams. Use macros
# so that we do not even evaluate the arguments in no-verbose modes
+import Compat.is_windows
@stevengj
stevengj Jun 30, 2016 Member

no need for this import since Compat already exports is_windows

@stevengj stevengj commented on an outdated diff Jun 30, 2016
@@ -0,0 +1,21 @@
+if VERSION >= v"0.5-"
+ using Base.Test
+else
+ using BaseTestNext
+ const Test = BaseTestNext
+end
+
@stevengj
stevengj Jun 30, 2016 Member

These using statements don't need to be here since they are in runtests.jl ... I don't think we need to worry about the case where people want to runt the different tests individually.

@stevengj stevengj commented on an outdated diff Jun 30, 2016
@@ -0,0 +1,17 @@
+if VERSION >= v"0.5-"
+ using Base.Test
+else
+ using BaseTestNext
+ const Test = BaseTestNext
+end
+
+import IJulia: init, Comm, comm_target
@stevengj
stevengj Jun 30, 2016 Member

No need to import IJulia.init if you are going to call it as a fully qualified name anyway.

@stevengj stevengj commented on an outdated diff Jul 4, 2016
@@ -1,6 +1,6 @@
julia 0.3
Nettle 0.2
-JSON 0.5
+JSON
@stevengj
stevengj Jul 4, 2016 Member

Don't we require JSON 0.5 or later? Why remove the version number?

@stevengj stevengj commented on an outdated diff Jul 4, 2016
ZMQ 0.3
-Compat 0.7.15
-Conda 0.1.5
+Compat 0.8.0
+Conda 0.1.8
@stevengj
stevengj Jul 4, 2016 Member

What do we require from Conda 0.1.8?

@stevengj stevengj commented on an outdated diff Jul 4, 2016
src/execute_request.jl
@@ -72,7 +73,7 @@ end
# return the content of a pyerr message for exception e
function error_content(e, bt=catch_backtrace(); backtrace_top::Symbol=:execute_request_0x535c5df2, msg::AbstractString="")
- tb = map(utf8, @compat(split(sprint(show_bt,
+ tb = map(String, @compat(split(sprint(show_bt,
@stevengj
stevengj Jul 4, 2016 Member

map(Compat.UTF8String, ... ?

@stevengj stevengj and 2 others commented on an outdated diff Jul 4, 2016
src/execute_request.jl
@@ -133,7 +134,7 @@ function helpcode(code::AbstractString)
@compat if !haskey(Docs.keywords, Symbol(code_))
return "Base.Docs.@repl $code_"
elseif VERSION < v"0.5.0-dev+3831"
- return "eval(:(Base.Docs.@repl \$(symbol(\"$code_\"))))"
+ return "eval(:(Base.Docs.@repl \$(Symbol(\"$code_\"))))"
@stevengj
stevengj Jul 4, 2016 Member

needs @compat Symbol for 0.3?

@yuyichao
yuyichao Jul 4, 2016 Member

Also note that this should be needed. The branch that uses Symbol is below

@yuyichao
yuyichao Jul 4, 2016 Member

It is written this way since there's already a branch and I was too lazy to figure out an order that eval, @repl and @compat are all happy about and that the @compat is evaluated in the right module.

@wookay
wookay Jul 5, 2016 edited Contributor

running in the jupyter notebook with @compat,
getting an error because that has not yet imported properly for eval(expr).
I have revert back to the original code. added a related test.

@stevengj stevengj commented on an outdated diff Jul 4, 2016
@@ -176,7 +176,7 @@ end
function oslibuv_flush()
#refs: https://github.com/JuliaLang/IJulia.jl/issues/347#issuecomment-144505862
# https://github.com/JuliaLang/IJulia.jl/issues/347#issuecomment-144605024
- @windows_only ccall(:SwitchToThread, stdcall, Void, ())
+ is_windows() && ccall(:SwitchToThread, stdcall, Void, ())
@stevengj
stevengj Jul 4, 2016 Member

@static is_windows() && ... to avoid the runtime branch

@stevengj stevengj commented on an outdated diff Jul 4, 2016
@@ -0,0 +1,2 @@
+julia 0.3
+Compat 0.8.0
@stevengj
stevengj Jul 4, 2016 Member

Why is this REQUIRE file necessary? Aren't these requirements already implied by the top-level REQUIRE? (I thought you only need a test/REQUIRE if your tests require additional packages not needed for the package itself.)

@stevengj
Member
stevengj commented Jul 5, 2016

Some of the test failures seem spurious...

@wookay
Contributor
wookay commented Jul 7, 2016

I'm trying to resolve these errors.

  • error from appveyor (0.3, 0.5)
===============================[ ERROR: Nettle ]================================
failed process: Process(`7z x -y 'C:\Users\appveyor\.julia\v0.3\WinRPM\cache\1\noarch%2Fmingw32-libgcc_s_sjlj1-6.1.0-4.2.noarch.cpio' '-oC:\Users\appveyor\.julia\v0.3\WinRPM\deps'`, ProcessExited(2)) [2]
while loading C:\Users\appveyor\.julia\v0.3\Nettle\deps\build.jl, in expression starting on line 39
  • error from travis (0.5 osx)
in _precompile_ at /Users/travis/.julia
jl_uv_writecb() ERROR: operation canceled ECANCELED
jl_uv_writecb() ERROR: operation canceled ECANCELED
...
@stevengj
Member
stevengj commented Jul 7, 2016

The AppVeyor failure seems like an unrelated glitch in their server; might go away if you just restart the check with a forced push.

@wookay
Contributor
wookay commented Jul 8, 2016

on OSX, Julia 0.5 with option inline=no has segmentation fault
during call start_heartbeat (IJulia/src/heartbeat.jl)

~/.julia/v0.5/IJulia/test $ julia --inline=no comm.jl
[1]    51285 segmentation fault
~/.julia/v0.5/IJulia/test $ julia --inline=yes comm.jl

Running Pkg.test("IJulia", coverage=true), the argument coverage makes --inline=no.

@yuyichao
Member

Ping. What's the status here?

@wookay
Contributor
wookay commented Jul 17, 2016

@yuyichao hello, there's an error during on test (on OSX, inline=no option for Julia 0.5-dev)
just saw your new pull request (isvalid method).

@wookay wookay changed the title from WIP deprecated writemime to show to Compat writemime, strings, os, startup file with some tests Jul 20, 2016
@wookay
Contributor
wookay commented Jul 20, 2016

rebase for squashing commits. thanks.

@stevengj stevengj and 1 other commented on an outdated diff Jul 20, 2016
deps/build.jl
@@ -62,7 +62,8 @@ juliakspec = abspath(spec_name)
binary_name = @windows? "julia.exe":"julia"
kernelcmd_array = String[joinpath(JULIA_HOME,("$binary_name")), "-i"]
ijulia_dir = get(ENV, "IJULIA_DIR", Pkg.dir("IJulia")) # support non-Pkg IJulia installs
-append!(kernelcmd_array, ["-F", joinpath(ijulia_dir,"src","kernel.jl"), "{connection_file}"])
+startupfile = VERSION >= v"0.4-" ? "--startup-file=yes" : "-F"
@stevengj
stevengj Jul 20, 2016 Member

0.4, not 0.4- (which denotes every prerelease of 0.4, from the instant it was forked from 0.3)

@wookay
wookay Jul 20, 2016 Contributor

Fixed, thanks!

@stevengj stevengj and 1 other commented on an outdated diff Jul 20, 2016
src/execute_request.jl
@@ -72,7 +73,7 @@ end
# return the content of a pyerr message for exception e
function error_content(e, bt=catch_backtrace(); backtrace_top::Symbol=:execute_request_0x535c5df2, msg::AbstractString="")
- tb = map(utf8, @compat(split(sprint(show_bt,
+ tb = map(Compat.UTF8String, @compat(split(sprint(show_bt,
@stevengj
stevengj Jul 20, 2016 Member

Will this work on 0.3?

@wookay
wookay Jul 20, 2016 Contributor

changed as

map(x->convert(Compat.UTF8String, x), ...
@andreasnoack
Member

@stevengj Is this ready?

@stevengj
Member

AppVeyor is failing with julia-latest?

@wookay
Contributor
wookay commented Jul 24, 2016

it's been failed about Conda linking packages.

Linking packages ...
Error: Error: post-link failed for: qt-4.8.7-vc9_9
@tkelman
Member
tkelman commented Jul 24, 2016

cc @Luthaf in case there's a broader issue here

@wookay
Contributor
wookay commented Jul 24, 2016 edited

Found a workaround for linking qt (on windows julia-0.5-pre)

      Pkg.clone(pwd(), \"IJulia\");
      using Conda;
      try Conda.add(\"qt\") end;
      postlink = normpath(Pkg.dir(\"Conda\"), \"deps/usr/pkgs/qt-4.8.7-vc9_9/Scripts/.qt-post-link.bat\");
      isfile(postlink) && rm(postlink);
      Pkg.build(\"IJulia\")"
@wookay
Contributor
wookay commented Jul 24, 2016

oh, ready to rebase. waiting reviews. thanks.

@stevengj stevengj and 1 other commented on an outdated diff Jul 24, 2016
deps/build.jl
@@ -59,10 +59,11 @@ debugdesc = ccall(:jl_is_debugbuild,Cint,())==1 ? "-debug" : ""
spec_name = "julia-$(VERSION.major).$(VERSION.minor)"*debugdesc
juliakspec = abspath(spec_name)
-binary_name = @windows? "julia.exe":"julia"
+binary_name = @static is_windows() ? "julia.exe" : "julia"
@stevengj
stevengj Jul 24, 2016 Member

No need for @static in the build script.

@wookay
wookay Jul 24, 2016 Contributor

Fixed. thanks.

@wookay wookay Compat writemime, strings, os, startup file with some tests
* Compat writemime to stringmime
* Compat with strings, dicts
* Compat is_apple, is_windows
* Fixed startup file option
* Added tests for travis, appveyor
fd9293e
@tkelman tkelman commented on the diff Jul 24, 2016
appveyor.yml
@@ -30,6 +32,13 @@ install:
- C:\projects\julia-binary.exe /S /D=C:\projects\julia
build_script:
- - C:\projects\julia\bin\julia -e "versioninfo();
- Pkg.clone(pwd(), \"IJulia\"); Pkg.build(\"IJulia\")"
-# - C:\projects\julia\bin\julia -e "Pkg.test(\"IJulia\")"
+ - C:\projects\julia\bin\julia -F -e "versioninfo(true);
+ Pkg.clone(pwd(), \"IJulia\");
+ using Conda;
+ try Conda.add(\"qt\") end;
+ postlink = normpath(Pkg.dir(\"Conda\"), \"deps/usr/pkgs/qt-4.8.7-vc9_9/Scripts/.qt-post-link.bat\");
@tkelman
tkelman Jul 24, 2016 Member

this is an odd workaround and seems likely to be fragile?

@wookay
wookay Jul 25, 2016 Contributor

right. it's just to pass the appveyor. I don't know exactly how to resolve that correctly.
I thought that this pull request has too much things,
I hope to open another commit/issue/pull request for this responsibility.

@tkelman tkelman commented on an outdated diff Jul 24, 2016
@@ -176,7 +176,9 @@ end
function oslibuv_flush()
#refs: https://github.com/JuliaLang/IJulia.jl/issues/347#issuecomment-144505862
# https://github.com/JuliaLang/IJulia.jl/issues/347#issuecomment-144605024
- @windows_only ccall(:SwitchToThread, stdcall, Void, ())
+ @compat @static if is_windows()
@tkelman
tkelman Jul 24, 2016 Member

@compat not needed here

@wookay wookay prune compat macro
faf2f5e
@stevengj
Member

Let's just merge this and robustify AppVeyor separately as needed.

@stevengj stevengj merged commit 74e6be1 into JuliaLang:master Jul 25, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment