Merge pull request #18012 from dhoegh/fix_17956

Fix #17956 and add tests
JuliaLang · Aug 14, 2016 · 0b7c8c2 · 0b7c8c2
2 parents 75e88af + 319cae5
commit 0b7c8c2
Show file tree

Hide file tree

Showing 6 changed files with 154 additions and 123 deletions.
diff --git a/base/deprecated.jl b/base/deprecated.jl
@@ -785,4 +785,9 @@ const _oldstyle_array_vcat_ = false
 
 @deprecate write(x) write(STDOUT::IO, x)
 
+function delete!(::EnvHash, k::AbstractString, def)
+    depwarn("`delete!(ENV, k, def)` should be replaced with `pop!(ENV, k, def)`. Be aware that `pop!` returns `k` or `def`, while `delete!` returns `ENV` or `def`.", :delete!)
+    haskey(ENV,k) ? delete!(ENV,k) : def
+end
+
 # End deprecations scheduled for 0.6
diff --git a/base/docs/helpdb/Base.jl b/base/docs/helpdb/Base.jl
@@ -2127,17 +2127,6 @@ Largest integer less than or equal to `x/y`.
 """
 fld
 
-"""
-    withenv(f::Function, kv::Pair...)
-
-Execute `f()` in an environment that is temporarily modified (not replaced as in `setenv`)
-by zero or more `"var"=>val` arguments `kv`. `withenv` is generally used via the
-`withenv(kv...) do ... end` syntax. A value of `nothing` can be used to temporarily unset an
-environment variable (if it is set). When `withenv` returns, the original environment has
-been restored.
-"""
-withenv
-
 """
     setdiff!(s, iterable)
 

diff --git a/base/env.jl b/base/env.jl
@@ -1,61 +1,59 @@
 # This file is a part of Julia. License is MIT: http://julialang.org/license
 
 if is_windows()
-const ERROR_ENVVAR_NOT_FOUND = UInt32(203)
+    const ERROR_ENVVAR_NOT_FOUND = UInt32(203)
 
-_getenvlen(var::Vector{UInt16}) = ccall(:GetEnvironmentVariableW,stdcall,UInt32,(Ptr{UInt16},Ptr{UInt16},UInt32),var,C_NULL,0)
-_hasenv(s::Vector{UInt16}) = _getenvlen(s) != 0 || Libc.GetLastError() != ERROR_ENVVAR_NOT_FOUND
-_hasenv(s::AbstractString) = _hasenv(cwstring(s))
+    _getenvlen(var::Vector{UInt16}) = ccall(:GetEnvironmentVariableW,stdcall,UInt32,(Ptr{UInt16},Ptr{UInt16},UInt32),var,C_NULL,0)
+    _hasenv(s::Vector{UInt16}) = _getenvlen(s) != 0 || Libc.GetLastError() != ERROR_ENVVAR_NOT_FOUND
+    _hasenv(s::AbstractString) = _hasenv(cwstring(s))
 
-function access_env(onError::Function, str::AbstractString)
-    var = cwstring(str)
-    len = _getenvlen(var)
-    if len == 0
-        return Libc.GetLastError() != ERROR_ENVVAR_NOT_FOUND ? "" : onError(str)
+    function access_env(onError::Function, str::AbstractString)
+        var = cwstring(str)
+        len = _getenvlen(var)
+        if len == 0
+            return Libc.GetLastError() != ERROR_ENVVAR_NOT_FOUND ? "" : onError(str)
+        end
+        val = zeros(UInt16,len)
+        ret = ccall(:GetEnvironmentVariableW,stdcall,UInt32,(Ptr{UInt16},Ptr{UInt16},UInt32),var,val,len)
+        if (ret == 0 && len != 1) || ret != len-1 || val[end] != 0
+            error(string("getenv: ", str, ' ', len, "-1 != ", ret, ": ", Libc.FormatMessage()))
+        end
+        pop!(val) # NUL
+        return transcode(String, val)
     end
-    val = zeros(UInt16,len)
-    ret = ccall(:GetEnvironmentVariableW,stdcall,UInt32,(Ptr{UInt16},Ptr{UInt16},UInt32),var,val,len)
-    if (ret == 0 && len != 1) || ret != len-1 || val[end] != 0
-        error(string("getenv: ", str, ' ', len, "-1 != ", ret, ": ", Libc.FormatMessage()))
+
+    function _setenv(svar::AbstractString, sval::AbstractString, overwrite::Bool=true)
+        var = cwstring(svar)
+        val = cwstring(sval)
+        if overwrite || !_hasenv(var)
+            ret = ccall(:SetEnvironmentVariableW,stdcall,Int32,(Ptr{UInt16},Ptr{UInt16}),var,val)
+            systemerror(:setenv, ret == 0)
+        end
     end
-    pop!(val) # NUL
-    return transcode(String, val)
-end
 
-function _setenv(svar::AbstractString, sval::AbstractString, overwrite::Bool=true)
-    var = cwstring(svar)
-    val = cwstring(sval)
-    if overwrite || !_hasenv(var)
-        ret = ccall(:SetEnvironmentVariableW,stdcall,Int32,(Ptr{UInt16},Ptr{UInt16}),var,val)
+    function _unsetenv(svar::AbstractString)
+        var = cwstring(svar)
+        ret = ccall(:SetEnvironmentVariableW,stdcall,Int32,(Ptr{UInt16},Ptr{UInt16}),var,C_NULL)
         systemerror(:setenv, ret == 0)
     end
-end
-
-function _unsetenv(svar::AbstractString)
-    var = cwstring(svar)
-    ret = ccall(:SetEnvironmentVariableW,stdcall,Int32,(Ptr{UInt16},Ptr{UInt16}),var,C_NULL)
-    systemerror(:setenv, ret == 0)
-end
-
 else # !windows
-_getenv(var::AbstractString) = ccall(:getenv, Cstring, (Cstring,), var)
-_hasenv(s::AbstractString) = _getenv(s) != C_NULL
-
-function access_env(onError::Function, var::AbstractString)
-    val = _getenv(var)
-    val == C_NULL ? onError(var) : unsafe_string(val)
-end
+    _getenv(var::AbstractString) = ccall(:getenv, Cstring, (Cstring,), var)
+    _hasenv(s::AbstractString) = _getenv(s) != C_NULL
 
-function _setenv(var::AbstractString, val::AbstractString, overwrite::Bool=true)
-    ret = ccall(:setenv, Int32, (Cstring,Cstring,Int32), var, val, overwrite)
-    systemerror(:setenv, ret != 0)
-end
+    function access_env(onError::Function, var::AbstractString)
+        val = _getenv(var)
+        val == C_NULL ? onError(var) : unsafe_string(val)
+    end
 
-function _unsetenv(var::AbstractString)
-    ret = ccall(:unsetenv, Int32, (Cstring,), var)
-    systemerror(:unsetenv, ret != 0)
-end
+    function _setenv(var::AbstractString, val::AbstractString, overwrite::Bool=true)
+        ret = ccall(:setenv, Int32, (Cstring,Cstring,Int32), var, val, overwrite)
+        systemerror(:setenv, ret != 0)
+    end
 
+    function _unsetenv(var::AbstractString)
+        ret = ccall(:unsetenv, Int32, (Cstring,), var)
+        systemerror(:unsetenv, ret != 0)
+    end
 end # os test
 
 ## ENV: hash interface ##
@@ -78,50 +76,47 @@ in(k::AbstractString, ::KeyIterator{EnvHash}) = _hasenv(k)
 pop!(::EnvHash, k::AbstractString) = (v = ENV[k]; _unsetenv(k); v)
 pop!(::EnvHash, k::AbstractString, def) = haskey(ENV,k) ? pop!(ENV,k) : def
 delete!(::EnvHash, k::AbstractString) = (_unsetenv(k); ENV)
-delete!(::EnvHash, k::AbstractString, def) = haskey(ENV,k) ? delete!(ENV,k) : def
 setindex!(::EnvHash, v, k::AbstractString) = _setenv(k,string(v))
 push!(::EnvHash, k::AbstractString, v) = setindex!(ENV, v, k)
 
 if is_windows()
-start(hash::EnvHash) = (pos = ccall(:GetEnvironmentStringsW,stdcall,Ptr{UInt16},()); (pos,pos))
-function done(hash::EnvHash, block::Tuple{Ptr{UInt16},Ptr{UInt16}})
-    if unsafe_load(block[1]) == 0
-        ccall(:FreeEnvironmentStringsW, stdcall, Int32, (Ptr{UInt16},), block[2])
-        return true
+    start(hash::EnvHash) = (pos = ccall(:GetEnvironmentStringsW,stdcall,Ptr{UInt16},()); (pos,pos))
+    function done(hash::EnvHash, block::Tuple{Ptr{UInt16},Ptr{UInt16}})
+        if unsafe_load(block[1]) == 0
+            ccall(:FreeEnvironmentStringsW, stdcall, Int32, (Ptr{UInt16},), block[2])
+            return true
+        end
+        return false
     end
-    return false
-end
-function next(hash::EnvHash, block::Tuple{Ptr{UInt16},Ptr{UInt16}})
-    pos = block[1]
-    blk = block[2]
-    len = ccall(:wcslen, UInt, (Ptr{UInt16},), pos)
-    buf = Array{UInt16}(len)
-    unsafe_copy!(pointer(buf), pos, len)
-    env = transcode(String, buf)
-    m = match(r"^(=?[^=]+)=(.*)$"s, env)
-    if m === nothing
-        error("malformed environment entry: $env")
+    function next(hash::EnvHash, block::Tuple{Ptr{UInt16},Ptr{UInt16}})
+        pos = block[1]
+        blk = block[2]
+        len = ccall(:wcslen, UInt, (Ptr{UInt16},), pos) + 1
+        buf = Array{UInt16}(len)
+        unsafe_copy!(pointer(buf), pos, len)
+        env = transcode(String, buf)
+        m = match(r"^(=?[^=]+)=(.*)$"s, env)
+        if m === nothing
+            error("malformed environment entry: $env")
+        end
+        return (Pair{String,String}(m.captures[1], m.captures[2]), (pos+len*2, blk))
     end
-    return (Pair{String,String}(m.captures[1], m.captures[2]), (pos+len*2, blk))
-end
-
 else # !windows
-start(::EnvHash) = 0
-done(::EnvHash, i) = (ccall(:jl_environ, Any, (Int32,), i) === nothing)
+    start(::EnvHash) = 0
+    done(::EnvHash, i) = (ccall(:jl_environ, Any, (Int32,), i) === nothing)
 
-function next(::EnvHash, i)
-    env = ccall(:jl_environ, Any, (Int32,), i)
-    if env === nothing
-        throw(BoundsError())
-    end
-    env = env::String
-    m = match(r"^(.*?)=(.*)$"s, env)
-    if m === nothing
-        error("malformed environment entry: $env")
+    function next(::EnvHash, i)
+        env = ccall(:jl_environ, Any, (Int32,), i)
+        if env === nothing
+            throw(BoundsError())
+        end
+        env = env::String
+        m = match(r"^(.*?)=(.*)$"s, env)
+        if m === nothing
+            error("malformed environment entry: $env")
+        end
+        return (Pair{String,String}(m.captures[1], m.captures[2]), i+1)
     end
-    return (Pair{String,String}(m.captures[1], m.captures[2]), i+1)
-end
-
 end # os-test
 
 #TODO: Make these more efficent
@@ -139,7 +134,15 @@ function show(io::IO, ::EnvHash)
     end
 end
 
-# temporarily set and then restore an environment value
+"""
+    withenv(f::Function, kv::Pair...)
+
+Execute `f()` in an environment that is temporarily modified (not replaced as in `setenv`)
+by zero or more `"var"=>val` arguments `kv`. `withenv` is generally used via the
+`withenv(kv...) do ... end` syntax. A value of `nothing` can be used to temporarily unset an
+environment variable (if it is set). When `withenv` returns, the original environment has
+been restored.
+"""
 function withenv{T<:AbstractString}(f::Function, keyvals::Pair{T}...)
     old = Dict{T,Any}()
     for (key,val) in keyvals

diff --git a/test/choosetests.jl b/test/choosetests.jl
@@ -26,7 +26,7 @@ function choosetests(choices = [])
         "priorityqueue", "file", "read", "mmap", "version", "resolve",
         "pollfd", "mpfr", "broadcast", "complex", "socket",
         "floatapprox", "datafmt", "reflection", "regex", "float16",
-        "combinatorics", "sysinfo", "rounding", "ranges", "mod2pi",
+        "combinatorics", "sysinfo", "env", "rounding", "ranges", "mod2pi",
         "euler", "show", "lineedit", "replcompletions", "repl",
         "replutil", "sets", "test", "goto", "llvmcall", "grisu",
         "nullable", "meta", "stacktraces", "profile", "libgit2", "docs",

diff --git a/test/env.jl b/test/env.jl
@@ -0,0 +1,65 @@
+# This file is a part of Julia. License is MIT: http://julialang.org/license
+
+@test !("f=a=k=e=n=a=m=e" ∈ keys(ENV))
+
+# issue #10994
+@test_throws ArgumentError ENV["bad\0name"] = "ok"
+@test_throws ArgumentError ENV["okname"] = "bad\0val"
+@test_throws ArgumentError Sys.set_process_title("bad\0title")
+
+withenv("bad"=>"dog") do
+    @test_throws ArgumentError ENV["bad\0cat"]
+end
+
+# issue #11170
+withenv("TEST"=>"nonempty") do
+    @test ENV["TEST"] == "nonempty"
+end
+withenv("TEST"=>"") do
+    @test ENV["TEST"] == ""
+end
+
+let c = collect(ENV)
+    @test isa(c, Vector)
+    @test length(ENV) == length(c)
+    @test isempty(ENV) || first(ENV) in c
+end
+
+# test for non-existent keys
+key = randstring(25)
+@test !haskey(ENV,key)
+@test_throws KeyError ENV[key]
+@test get(ENV,key,"default") == "default"
+
+# Test for #17956
+@test length(ENV) > 1
+k1, k2 = "__test__", "__test1__"
+withenv(k1=>k1, k2=>k2) do
+    b_k1, b_k2 = false, false
+    for (k, v) in ENV
+        if k==k1
+            b_k1=true
+        elseif k==k2
+            b_k2=true
+        end
+    end
+    @test b_k1 && b_k2
+    io = IOBuffer()
+    show(io, ENV)
+    s = takebuf_string(io)
+    @test contains(s, "$k1=$k1")
+    @test contains(s, "$k2=$k2")
+
+    @test pop!(ENV, k1) == k1
+    @test !haskey(ENV, k1)
+    ENV[k1] = k1
+    @test pop!(ENV, k1) == k1
+    @test pop!(ENV, k1, "not_there") == "not_there"
+
+    ENV[k1] = k1
+    @test delete!(ENV, k1) == ENV
+    @test !haskey(ENV, k1)
+end
+
+# Test for #10853
+@test withenv(Dict{Any,Any}()...) do; true; end
diff --git a/test/sysinfo.jl b/test/sysinfo.jl
@@ -6,34 +6,3 @@
 sprint(Base.Sys.cpu_summary)
 @test Base.Sys.uptime() > 0
 Base.Sys.loadavg()
-
-@test !("f=a=k=e=n=a=m=e" ∈ keys(ENV))
-
-# issue #10994
-@test_throws ArgumentError ENV["bad\0name"] = "ok"
-@test_throws ArgumentError ENV["okname"] = "bad\0val"
-@test_throws ArgumentError Sys.set_process_title("bad\0title")
-
-withenv("bad"=>"dog") do
-    @test_throws ArgumentError ENV["bad\0cat"]
-end
-
-# issue #11170
-withenv("TEST"=>"nonempty") do
-    @test ENV["TEST"] == "nonempty"
-end
-withenv("TEST"=>"") do
-    @test ENV["TEST"] == ""
-end
-
-let c = collect(ENV)
-    @test isa(c, Vector)
-    @test length(ENV) == length(c)
-    @test isempty(ENV) || first(ENV) in c
-end
-
-# test for non-existent keys
-key = randstring(25)
-@test !haskey(ENV,key)
-@test_throws KeyError ENV[key]
-@test get(ENV,key,"default") == "default"