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

Updates for Julia 0.7 #216

Merged
merged 10 commits into from
Feb 3, 2018
Merged

Updates for Julia 0.7 #216

merged 10 commits into from
Feb 3, 2018

Conversation

andreasnoack
Copy link
Contributor

I realized that this work overlapped #197 but at that point, I concluded that it was easier to finish my own branch. This branch is based on FemtoCleaners suggestions and then some changes by hand to remove deprecation warnings. Along the way, I also hit #215 and added calls to gc() as a temporary fix to make tests pass. To avoid pollution of the namespace, I've changed the test of the samples to run each of them in a separate process in parallel with at most four Julia process running concurrently. We might want to add an option to set this number.

@coveralls
Copy link

coveralls commented Feb 2, 2018

Coverage Status

Coverage decreased (-0.7%) to 90.807% when pulling bc7b374 on andreasnoack:master into 10913a1 on JuliaGraphics:master.

@andreasnoack
Copy link
Contributor Author

Okay, I think this one works now. The only issue is ColorTypes which should be fixed by JuliaGraphics/ColorTypes.jl#103 and I think we can merge that one later today.

@lobingera
Copy link
Contributor

i'm not at my development box right now, but i guess i merge this rather soon (femto didn't work for me for some time, so i started manually editing ...) and bring in my 'ideas' as new PR in the next step.

REQUIRE Outdated
@@ -1,4 +1,4 @@
julia 0.5
julia 0.6
Compat 0.39.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs increasing for the newer features you're using

@lobingera
Copy link
Contributor

Running this locally fails in all samples, like:

sample: sample_alpha_paint.jl: Error During Test at /home/lobi/.julia/v0.7/Cairo/test/runtests.jl:80
  Got an exception of type UndefVarError outside of a @test
  UndefVarError: include not defined
  Stacktrace:
   [1] top-level scope
   [2] eval at ./boot.jl:295 [inlined]
   [3] eval(::Module, ::Expr) at ./sysimg.jl:71
   [4] macro expansion at /home/lobi/.julia/v0.7/Cairo/test/runtests.jl:82 [inlined]
   [5] macro expansion at /home/lobi/julia07/usr/share/julia/site/v0.7/Test/src/Test.jl:1080 [inlined]
   [6] macro expansion at /home/lobi/.julia/v0.7/Cairo/test/runtests.jl:80 [inlined]
   [7] macro expansion at /home/lobi/julia07/usr/share/julia/site/v0.7/Test/src/Test.jl:1008 [inlined]
   [8] top-level scope at /home/lobi/.julia/v0.7/Cairo/test/runtests.jl:72
   [9] include at ./boot.jl:292 [inlined]
   [10] include_relative(::Module, ::String) at ./loading.jl:1012
   [11] include(::Module, ::String) at ./sysimg.jl:26
   [12] exec_options(::Base.JLOptions) at ./client.jl:332
   [13] _start() at ./client.jl:447

@andreasnoack
Copy link
Contributor Author

Yes. I made a fix for 0.6 that didn't work on master. A new release of ColorTypes is now out so I've pushed a version here that should pass Travis tests on both 0.6 and 0.7/master.

@andreasnoack
Copy link
Contributor Author

So I'd say this one is ready to go. The nightly tests fail on Mac and Windows but that is because of Homebrew and WinRPM.

@lobingera lobingera merged commit c599c15 into JuliaGraphics:master Feb 3, 2018
@lobingera
Copy link
Contributor

i just ran this locally and i agree. There are still things, i'd do differently (esp. using Ptr{Cvoid} rather than Ptr{Nothing} but that's no showstopper.

@andreasnoack
Copy link
Contributor Author

Thanks.

There are still things, i'd do differently (esp. using Ptr{Cvoid} rather than Ptr{Nothing} but that's no showstopper.

No problem. Feel free to change it.

@andreasnoack
Copy link
Contributor Author

Would be great with a release, though, such that we can continue getting packages working on nightly.

@lobingera
Copy link
Contributor

lobingera commented Feb 3, 2018

yes, this shouldn't be a problem, but there should be more tests succeeding

@andreasnoack
Copy link
Contributor Author

but there should be more tests succeeding

I'm not sure I follow. Which tests? Nightlies for Windows and Mac?

@lobingera
Copy link
Contributor

appveyor (win7) fails on nightly,
travis fails on OSX nightly

@andreasnoack
Copy link
Contributor Author

It is a bit uncommon to make a release conditional on tests passing on nightly. The failures are (most likely) because of issues with WinRPM and Homebrew and not anything in this package. Postponing a release until the two packages are fixed will just delay Julia 0.7 migration of downstream packages (which is the reason I've spent time on fixing Cairo.jl).

@lobingera
Copy link
Contributor

lobingera commented Feb 3, 2018

I see what you mean. However, we test 3 architectures and imho 2 should succeed. Otherwise a longer list of 'why does Cairo not work on 0.7?' shows up here. btw: Homebrew fails currently on 0.6 and 0.7dev. You managed to run locally?

@andreasnoack
Copy link
Contributor Author

btw: Homebrew fails currently on 0.6 and 0.7dev. You managed to run locally?

Are you talking about Homebrew.jl's own tests? Cairo.jl passes on 0.6. I guess the subset of Homebrew.jl needed for the Cairo.jl build is working.

@lobingera
Copy link
Contributor

No i meant, you ran your own PR locally on OSX and 0.7dev?

using Compat
import Compat.Libdl
import Compat.Sys
import Compat.Pkg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't exist

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i cannot follow, as it does (here locally in 0.51.0).

               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: https://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.7.0-DEV.3677 (2018-02-01 16:36 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit ecb8b56* (2 days old master)
|__/                   |  x86_64-linux-gnu

julia> using Compat

help?>  Compat.Libdl
  Interface to libdl. Provides dynamic linking support.


julia> import Compat.Libdl

julia> 

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Tony is referring to Compat.Pkg. That was introduced in Compat v0.52.0, which postdates this PR (tagged today).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange, still 0.51.0 here and

               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: https://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.7.0-DEV.3677 (2018-02-01 16:36 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit ecb8b56* (2 days old master)
|__/                   |  x86_64-linux-gnu

julia> using Compat

julia> import Compat.Pkg
WARNING: importing deprecated binding Base.Pkg into Compat.

julia> 

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same warning in 0.50.0

@andreasnoack
Copy link
Contributor Author

No i meant, you ran your own PR locally on OSX and 0.7dev?

Yes and no. Both Pkg.build("Cairo") and Pkg.test("Cairo") work locally from my usual package directory but then I tried to clone Cairo master from a clean package directory. There the Pkg.build step completes without errors but deps.jl doesn't have the paths to the dylibs. Don't know why except that Homebrew.jl has a long track record of being unreliable.

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.

5 participants