Skip to content

Commit

Permalink
addenv(), by default, should inherit from ENV (#39100)
Browse files Browse the repository at this point in the history
The spirit of `addenv()` is to be non-destructive, however because a
`Cmd` object with an `env` member that is set to `nothing` (the default)
inherts from the current environment when it is run.  This means that
the following, rather surprising behavior holds true on current master:

```
julia> ENV["FOO"] = "foo"
       run(`/bin/bash -c "echo \$FOO \$BAR"`)
       run(addenv(`/bin/bash -c "echo \$FOO \$BAR"`, "BAR" => "bar"))
foo
bar
```

This PR adds an `inherit` flag to `addenv()` to allow keeping this
behavior if it is actually desired (this might make sense if you are
constructing `.env` blocks over a series of `addenv()` calls and you
don't want to have to special-case your first operation as a `setenv()`
rather than an `addenv()`.
  • Loading branch information
staticfloat authored Jan 7, 2021
1 parent c70a5bc commit fa14d0b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 8 deletions.
21 changes: 13 additions & 8 deletions base/cmd.jl
Original file line number Diff line number Diff line change
Expand Up @@ -246,14 +246,19 @@ setenv(cmd::Cmd, env::Pair{<:AbstractString}...; dir="") =
setenv(cmd::Cmd; dir="") = Cmd(cmd; dir=dir)

"""
addenv(command::Cmd, env...)
addenv(command::Cmd, env...; inherit::Bool = true)
Merge new environment mappings into the given `Cmd` object, returning a new `Cmd` object.
Duplicate keys are replaced.
Duplicate keys are replaced. If `command` does not contain any environment values set already,
it inherits the current environment at time of `addenv()` call if `inherit` is `true`.
"""
function addenv(cmd::Cmd, env::Dict)
function addenv(cmd::Cmd, env::Dict; inherit::Bool = true)
new_env = Dict{String,String}()
if cmd.env !== nothing
if cmd.env === nothing
if inherit
merge!(new_env, ENV)
end
else
for (k, v) in split.(cmd.env, "=")
new_env[string(k)::String] = string(v)::String
end
Expand All @@ -264,12 +269,12 @@ function addenv(cmd::Cmd, env::Dict)
return setenv(cmd, new_env)
end

function addenv(cmd::Cmd, pairs::Pair{<:AbstractString}...)
return addenv(cmd, Dict(k => v for (k, v) in pairs))
function addenv(cmd::Cmd, pairs::Pair{<:AbstractString}...; inherit::Bool = true)
return addenv(cmd, Dict(k => v for (k, v) in pairs); inherit)
end

function addenv(cmd::Cmd, env::Vector{<:AbstractString})
return addenv(cmd, Dict(k => v for (k, v) in split.(env, "=")))
function addenv(cmd::Cmd, env::Vector{<:AbstractString}; inherit::Bool = true)
return addenv(cmd, Dict(k => v for (k, v) in split.(env, "=")); inherit)
end

(&)(left::AbstractCmd, right::AbstractCmd) = AndCmds(left, right)
Expand Down
22 changes: 22 additions & 0 deletions test/spawn.jl
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,28 @@ end
@test strip(String(read(cmd))) == "bar bar"
cmd = addenv(cmd, ["FOO=baz"])
@test strip(String(read(cmd))) == "baz bar"

# Test that `addenv()` works properly with `inherit`
withenv("FOO" => "foo") do
cmd = Cmd(`$shcmd -c "echo \$FOO \$BAR"`)
@test strip(String(read(cmd))) == "foo"

cmd2 = addenv(cmd, "BAR" => "bar"; inherit=false)
@test strip(String(read(cmd2))) == "bar"

cmd2 = addenv(cmd, "BAR" => "bar"; inherit=true)
@test strip(String(read(cmd2))) == "foo bar"

# Changing the environment doesn't effect the command,
# because it was baked in at `addenv()` time
withenv("FOO" => "baz") do
@test strip(String(read(cmd2))) == "foo bar"
end

# Even with inheritance, `addenv()` dominates:
cmd2 = addenv(cmd, "FOO" => "foo2", "BAR" => "bar"; inherit=true)
@test strip(String(read(cmd2))) == "foo2 bar"
end
end


Expand Down

0 comments on commit fa14d0b

Please sign in to comment.