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.

Sign up for GitHub

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

Exit calls in build scripts no longer build to abort #20070

Merged
merged 4 commits into from
Apr 16, 2017
Merged

Exit calls in build scripts no longer build to abort #20070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7bb938e
Remove need for temp_rel_pkg_dir
omus Apr 14, 2017
94f1f85
Exit calls in build scripts no longer abort build
omus Jan 14, 2017
7a3c2fe
Isolate each build.jl in a seperate Julia process
omus Jan 24, 2017
c3063d4
Switch to triple-backticks
omus Apr 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 39 additions & 44 deletions base/pkg/entry.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,26 +579,9 @@ function warnbanner(msg...; label="[ WARNING ]", prefix="")
warn(prefix="", "="^cols)
end

function build!(pkgs::Vector, buildstream::IO, seen::Set)
for pkg in pkgs
pkg == "julia" && continue
pkg in seen ? continue : push!(seen,pkg)
Read.isinstalled(pkg) || throw(PkgError("$pkg is not an installed package"))
build!(Read.requires_list(pkg),buildstream,seen)
path = abspath(pkg,"deps","build.jl")
isfile(path) || continue
println(buildstream, path) # send to build process for evalfile
flush(buildstream)
end
end

function build!(pkgs::Vector, errs::Dict, seen::Set=Set())
# To isolate the build from the running Julia process, we
# execute the build.jl files in a separate process that
# is sitting there waiting for paths to evaluate. Errors
# are serialized to errfile for later retrieval into errs[pkg]
errfile = tempname()
close(open(errfile, "w")) # create empty file
function build(pkg::AbstractString, build_file::AbstractString, errfile::AbstractString)
# To isolate the build from the running Julia process, we execute each build.jl file in
# a separate process. Errors are serialized to errfile for later reporting.
# TODO: serialize the same way the load cache does, not with strings
LOAD_PATH = filter(x -> x isa AbstractString, Base.LOAD_PATH)
code = """
Expand All @@ -609,42 +592,54 @@ function build!(pkgs::Vector, errs::Dict, seen::Set=Set())
empty!(Base.DL_LOAD_PATH)
append!(Base.DL_LOAD_PATH, $(repr(Base.DL_LOAD_PATH)))
open("$(escape_string(errfile))", "a") do f
for path in eachline(STDIN)
pkg = basename(dirname(dirname(path)))
try
info("Building \$pkg")
cd(dirname(path)) do
evalfile(path)
end
catch err
Base.Pkg.Entry.warnbanner(err, label="[ ERROR: \$pkg ]")
serialize(f, pkg)
serialize(f, err)
pkg, build_file = "$pkg", "$(escape_string(build_file))"
try
info("Building \$pkg")
cd(dirname(build_file)) do
evalfile(build_file)
end
catch err
Base.Pkg.Entry.warnbanner(err, label="[ ERROR: \$pkg ]")
serialize(f, pkg)
serialize(f, err)
end
end
"""
io, pobj = open(pipeline(detach(`$(Base.julia_cmd()) -O0
--compilecache=$(Bool(Base.JLOptions().use_compilecache) ? "yes" : "no")
--history-file=no
--color=$(Base.have_color ? "yes" : "no")
--eval $code`), stderr=STDERR), "w", STDOUT)
cmd = ```
$(Base.julia_cmd()) -O0
--compilecache=$(Bool(Base.JLOptions().use_compilecache) ? "yes" : "no")
--history-file=no
--color=$(Base.have_color ? "yes" : "no")
--eval $code
```

success(pipeline(cmd, stderr=STDERR))
Copy link
Member

Choose a reason for hiding this comment

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

why we do not resend stdout to STDOUT?

Copy link
Member

@iblislin iblislin Sep 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

MXNet.jl ran into this: dmlc/MXNet.jl#265 (comment)

We need to build libmxnet via make. The main build step requires more than 10 mins on travis-ci. But stdout is gone in Julia 0.6+, so the build got killed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Definitely an oversight. We should add a test for this.

end

function build!(pkgs::Vector, seen::Set, errfile::AbstractString)
for pkg in pkgs
pkg == "julia" && continue
pkg in seen ? continue : push!(seen,pkg)
Read.isinstalled(pkg) || throw(PkgError("$pkg is not an installed package"))
build!(Read.requires_list(pkg), seen, errfile)
path = abspath(pkg,"deps","build.jl")
isfile(path) || continue
build(pkg, path, errfile) || error("Build process failed.")
end
end

function build!(pkgs::Vector, errs::Dict, seen::Set=Set())
errfile = tempname()
touch(errfile) # create empty file
try
build!(pkgs, io, seen)
close(io)
wait(pobj)
success(pobj) || error("Build process failed.")
build!(pkgs, seen, errfile)
open(errfile, "r") do f
while !eof(f)
pkg = deserialize(f)
err = deserialize(f)
errs[pkg] = err
end
end
catch err
close(io)
isa(err, PkgError) ? wait(pobj) : kill(pobj)
rethrow(err)
finally
isfile(errfile) && Base.rm(errfile)
end
Expand Down
68 changes: 43 additions & 25 deletions test/pkg.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,23 @@ function capture_stdout(f::Function)
end


function temp_pkg_dir(fn::Function, remove_tmp_dir::Bool=true)
function temp_pkg_dir(fn::Function, tmp_dir=joinpath(tempdir(), randstring()),
remove_tmp_dir::Bool=true; initialize::Bool=true)

# Used in tests below to set up and tear down a sandboxed package directory
const tmpdir = joinpath(tempdir(),randstring())
withenv("JULIA_PKGDIR" => tmpdir) do
withenv("JULIA_PKGDIR" => tmp_dir) do
@test !isdir(Pkg.dir())
try
Pkg.init()
@test isdir(Pkg.dir())
Pkg.resolve()
if initialize
Pkg.init()
@test isdir(Pkg.dir())
Pkg.resolve()
else
mkpath(Pkg.dir())
end
fn()
finally
remove_tmp_dir && rm(tmpdir, recursive=true)
remove_tmp_dir && rm(tmp_dir, recursive=true)
end
end
end
Expand Down Expand Up @@ -569,28 +574,41 @@ let io = IOBuffer()
@test !contains(String(take!(io)), "backtrace()")
end


function temp_rel_pkg_dir(fn::Function, remove_tmp_dir::Bool=true)
# Used in tests below to set up and tear down a sandboxed package directory
@testset "Relative path operations" begin
cd(tempdir()) do
const tmpdir = randstring()
withenv("JULIA_PKGDIR" => tmpdir) do
@test !isdir(Pkg.dir())
try
Pkg.init()
@test isdir(Pkg.dir())
Pkg.resolve()
fn()
finally
remove_tmp_dir && rm(tmpdir, recursive=true)
end
temp_pkg_dir(randstring()) do
Pkg.add("Example")
@test [keys(Pkg.installed())...] == ["Example"]
end
end
end

@testset "Relative path operations" begin
temp_rel_pkg_dir() do
Pkg.add("Example")
@test [keys(Pkg.installed())...] == ["Example"]
temp_pkg_dir(initialize=false) do
function write_build(pkg, content)
build_filename = Pkg.dir(pkg, "deps", "build.jl")
mkpath(dirname(build_filename))
write(build_filename, content)
end

write_build("Normal", "")
write_build("Error", "error(\"An error has occurred while building a package\")")
write_build("Exit", "exit()")

cd(Pkg.dir()) do
errors = Dict()

empty!(errors)
@test_warn ("INFO: Building Error",
"INFO: Building Normal") Pkg.Entry.build!(["Error", "Normal"], errors)

empty!(errors)
@test_warn ("INFO: Building Exit",
"INFO: Building Normal") Pkg.Entry.build!(["Exit", "Normal"], errors)

empty!(errors)
@test_warn ("INFO: Building Exit",
"INFO: Building Normal",
"INFO: Building Exit",
"INFO: Building Normal") Pkg.Entry.build!(["Exit", "Normal", "Exit", "Normal"], errors)
end
end