JuliaPy / Conda.jl

Conda managing Julia binary dependencies
Other
173 stars 57 forks source link

Change from isdefined function to @isdefined macro #94

Closed pbouffard closed 6 years ago

pbouffard commented 6 years ago

In Julia v0.7.0 the function form is deprecated and results in what looks like a parse error:

julia> Pkg.build("Conda")
INFO: Building Conda
=================================================================[ ERROR: Conda ]==================================================================

LoadError: type CodeInfo has no field def
in expression starting at /Users/patrick/.julia/v0.7/Conda/deps/build.jl:7

===================================================================================================================================================

=================================================================[ BUILD ERRORS ]==================================================================

WARNING: Conda had build errors.

 - packages with build errors remain installed in /Users/patrick/.julia/v0.7
 - build the package(s) and all dependencies with `Pkg.build("Conda")`
 - build a single package by running its `deps/build.jl` script

===================================================================================================================================================
pbouffard commented 6 years ago

I'm a bit puzzled why this doesn't seem to have impacted others (at least, I can't find evidence of that) since I'd assume that this package would be used by most who are on master of Julia. Aside from that a couple of questions arise:

  1. Shouldn't a deprecation warning also have popped up? Or was that not possible because the compiler was confused and didn't even recognize isdefined as a symbol?
  2. I see that builds with Julia 0.5.0 and 0.6.0 fail because the macro form doesn't exist yet in that version. Does that need to be fixed for this to merge?

My first PR, be gentle ;)

omus commented 6 years ago

I see that builds with Julia 0.5.0 and 0.6.0 fail because the macro form doesn't exist yet in that version. Does that need to be fixed for this to merge?

Typically with Julia the latest syntax is introduced using the Compat.jl package. The @isdefined(x) macro unfortunately cannot be supported in earlier version of Julia. Instead you can use the form isdefined(@__MODULE__, :x) which should work in Julia 0.7 and earlier versions.

I'm a bit puzzled why this doesn't seem to have impacted others (at least, I can't find evidence of that) since I'd assume that this package would be used by most who are on master of Julia.

I'm also unclear on why this issue hasn't appeared for others (including myself). I'm really not sure why you were getting an error here.

pbouffard commented 6 years ago

Hmm, I think the issue I'm seeing probably has nothing to do with the Conda package. It seems that my build for some reason doesn't like the function form of isdefined, but only in certain contexts:

MacBook-Pro-3:julia patrick$ ./julia
               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: https://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.7.0-DEV.2517 (2017-11-16 02:46 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit 8c3f8b1eeb (3 days old master)
|__/                   |  x86_64-apple-darwin16.7.0

julia> if !isdefined(:ROOTENV); println("not defined"); end
ERROR: type CodeInfo has no field def
Stacktrace:
 [1] firstcaller(::Array{Union{Ptr{Void}, Base.InterpreterIP},1}, ::Tuple{Symbol}) at ./deprecated.jl:100
 [2] firstcaller(::Array{Union{Ptr{Void}, Base.InterpreterIP},1}, ::Symbol) at ./deprecated.jl:84
 [3] depwarn(::String, ::Symbol) at ./deprecated.jl:69
 [4] top-level scope

julia> isdefined(:ROOTENV)
WARNING: `isdefined(:symbol)` is deprecated, use `@isdefined symbol` instead
Stacktrace:
 [1] depwarn(::String, ::Symbol) at ./deprecated.jl:68
 [2] top-level scope
 [3] eval(::Module, ::Expr) at ./repl/REPL.jl:3
 [4] eval_user_input(::Any, ::Base.REPL.REPLBackend) at ./repl/REPL.jl:69
 [5] macro expansion at ./repl/REPL.jl:100 [inlined]
 [6] (::getfield(Base.REPL, Symbol("##1#2")){Base.REPL.REPLBackend})() at ./event.jl:95
in expression starting at no file:0
false

julia> 
yuyichao commented 6 years ago

Seems to be a base julia bug and the depwarn code may need to be updated for the toplevel frame.

pbouffard commented 6 years ago

I'm going to pull the latest master of the Julia repo and see if I can still reproduce. If so I'll open a ticket over there.

pbouffard commented 6 years ago

See https://github.com/JuliaLang/julia/issues/24658