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

What kind of io is allowed for Pkg.test? #3430

Open
ericphanson opened this issue Apr 2, 2023 · 6 comments
Open

What kind of io is allowed for Pkg.test? #3430

ericphanson opened this issue Apr 2, 2023 · 6 comments

Comments

@ericphanson
Copy link
Contributor

If you have a test environment that hasn't been precompiled, then as of #3281, running Pkg.test(; io) will call two commands like

run(pipeline(`...`, stdout=io))
run(pipeline(`...`, stdout=io))

with the same IO object. This doesn't work well for a lot of common IO objects like IOBuffer, Base.BufferStream, Base.PipeBuffer, etc, since they will have eof(stream) == true after the first command finishes, and some of them will error on the second command.

What happens for these IO types?

For IOBuffer and Base.PipeBuffer, this will show

┌ Warning: Process I/O error
│   exception =
│    ArgumentError: ensureroom failed, IOBuffer is not writeable
│    Stacktrace:
│     [1] ensureroom_slowpath(io::IOBuffer, nshort::UInt64)
│       @ Base ./iobuffer.jl:303
│     [2] ensureroom
│       @ ./iobuffer.jl:325 [inlined]
│     [3] unsafe_write(to::IOBuffer, p::Ptr{UInt8}, nb::UInt64)
│       @ Base ./iobuffer.jl:424
│     [4] unsafe_write
│       @ ./io.jl:685 [inlined]
│     [5] write
│       @ ./io.jl:708 [inlined]
│     [6] write(to::IOBuffer, from::Base.PipeEndpoint)
│       @ Base ./io.jl:755
│     [7] macro expansion
│       @ ./process.jl:300 [inlined]
│     [8] (::Base.var"#764#765"{IOBuffer, Bool, Base.PipeEndpoint, IOBuffer, Base.PipeEndpoint})()
│       @ Base ./task.jl:514
└ @ Base process.jl:302

For Base.BufferStream, it will "seem" to work, but you get strange behavior if you test it like this:

julia> io = Base.BufferStream()
BufferStream(bytes waiting=0, isopen=true)

julia> let
           t = @async begin
               for _ = 1:10
                   bytes = readavailable(io)
                   @show String(bytes)
                   @show eof(io)
               end
           end
           run(pipeline(`echo "hi"`, stdout=io))
           run(pipeline(`echo "bye"`, stdout=io))
       end
String(bytes) = "hi\n"
eof(io) = true
String(bytes) = ""
eof(io) = true
String(bytes) = ""
eof(io) = true
String(bytes) = "bye\n"
eof(io) = true
String(bytes) = ""
eof(io) = true
String(bytes) = ""
eof(io) = true
String(bytes) = ""
eof(io) = true
String(bytes) = ""
eof(io) = true
String(bytes) = ""
eof(io) = true
String(bytes) = ""
eof(io) = true
Process(`echo bye`, ProcessExited(0))

since it shows eof(io) == true but then still displays "bye\n" later.

However devnull and stdout work fine, which my guess is what was tested here. So is there a particular restriction on what kinds of IO are allowed? Or is the change in #3281 a regression?

The context for me is I wrote some code to try to parse Pkg.jl test results "live" during GitHub Actions CI runs to re-emit test failures as logs which can then show up as annotations in GitHub PRs. It's actually fairly handy when it works (annotations only show up in-line on the diff, so you have to have changed those tests), but this was broken by #3281: julia-actions/julia-runtest#76.

@KristofferC
Copy link
Member

Being able to run two Pkg commands with an IOBuffer io should be possible so I guess this is a regression then.

@ericphanson
Copy link
Contributor Author

This is an issue with one package command, Pkg.test, which now internally calls run twice if it needs to precompile.

@ericphanson
Copy link
Contributor Author

ericphanson commented Apr 3, 2023

For example:

(@v1.9) pkg> generate TestPkg
  Generating  project TestPkg:
    TestPkg/Project.toml
    TestPkg/src/TestPkg.jl

shell> cd TestPkg
/Users/eph/TestPkg

(@v1.9) pkg> activate .
  Activating project at `~/TestPkg`

julia> io = IOBuffer()
IOBuffer(data=UInt8[...], readable=true, writable=true, seekable=true, append=false, size=0, maxsize=Inf, ptr=1, mark=-1)

julia> using Pkg

shell> mkdir test

shell> touch test/runtests.jl

julia> io = IOBuffer()
IOBuffer(data=UInt8[...], readable=true, writable=true, seekable=true, append=false, size=0, maxsize=Inf, ptr=1, mark=-1)

julia> Pkg.test(; io)
Precompiling environment...
  1 dependency successfully precompiled in 0 seconds
ERROR: ArgumentError: ensureroom failed, IOBuffer is not writeable
Stacktrace:
  [1] ensureroom_slowpath(io::IOBuffer, nshort::UInt64)
    @ Base ./iobuffer.jl:303
  [2] ensureroom
    @ ./iobuffer.jl:325 [inlined]
  [3] unsafe_write(to::IOBuffer, p::Ptr{UInt8}, nb::UInt64)
    @ Base ./iobuffer.jl:424
  [4] write
    @ ./strings/io.jl:244 [inlined]
  [5] print
    @ ./strings/io.jl:246 [inlined]
  [6] with_output_color(f::Function, color::Symbol, io::IOBuffer, args::String; bold::Bool, underline::Bool, blink::Bool, reverse::Bool, hidden::Bool)
    @ Base ./util.jl:80
  [7] #printstyled#963
    @ ./util.jl:130 [inlined]
  [8] printpkgstyle(io::IOBuffer, cmd::Symbol, text::String, ignore_indent::Bool; color::Symbol)
    @ Pkg ~/.julia/juliaup/julia-1.9.0-beta4+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/Pkg/src/utils.jl:5
 ...

@KristofferC
Copy link
Member

I guess we have to make a new IOBuffer for the external process and write the output from that back into the original io?

@ericphanson
Copy link
Contributor Author

That sounds like it would work to me (though I can't find any docs to say if run is supposed to mark the IOBuffer as not-writable or not, or if there is a way to control that, or what it actually means etc). I guess it would need to be done in a streaming way since otherwise you need to wait for all the tests to finish to get the output, right?

@KristofferC
Copy link
Member

Yeah, the follow-up question is indeed why run kills the buffer...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants