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

Use the current std(out|err) for printing, fixes #2542. #2543

Merged
merged 1 commit into from
May 2, 2021

Conversation

fredrikekre
Copy link
Member

No description provided.

@fredrikekre fredrikekre added the backport 1.6 Change should be backported to release-1.6 label Apr 30, 2021
@IanButterworth
Copy link
Sponsor Member

@fredrikekre hope you don't mind the push

@IanButterworth
Copy link
Sponsor Member

It seems this won't be the full fix for #2542
IJulia needs to set DEFAULT_IO[] correctly or to nothing
Screen Shot 2021-05-02 at 1 27 17 PM

@KristofferC
Copy link
Sponsor Member

I think it is on us to fix it :p

@IanButterworth
Copy link
Sponsor Member

Yeah, that sounds good. But how can we detect if a custom io is set?
https://github.com/JuliaLang/IJulia.jl/blob/cc2a9bf61a2515596b177339f9a3514de8c38573/src/init.jl#L101-L108

@IanButterworth
Copy link
Sponsor Member

I just assumed DEFAULT_IO existed for packages like IJulia to override if stdio was redirected? If not, shouldn't everything just point to stdout/stderr?

@KristofferC
Copy link
Sponsor Member

KristofferC commented May 2, 2021

I think it is just an internal thing to redirect the Pkg test output globally.

When does DEFAULT_IO get set in that IJulia session?

@IanButterworth
Copy link
Sponsor Member

When does DEFAULT_IO get set in tha I Julia session?

I don't think it does. I couldn't find a reference in the package

@fredrikekre
Copy link
Member Author

It is just for silencing Pkg output in tests I think.

@KristofferC
Copy link
Sponsor Member

But why is it not nothing in the IJulia session then?

@fredrikekre
Copy link
Member Author

fredrikekre commented May 2, 2021

But why is it not nothing in the IJulia session then?

It is.
Screenshot from 2021-05-02 20-49-43

@IanButterworth
Copy link
Sponsor Member

IanButterworth commented May 2, 2021

It isn't for me (MacOS, 1.6.1, IJulia v1.23.2)
Screen Shot 2021-05-02 at 2 57 37 PM

Regular 1.6.1

julia> import Pkg

julia> Pkg.DEFAULT_IO[]
Base.TTY(RawFD(16) open, 0 bytes waiting)

@KristofferC
Copy link
Sponsor Member

Some garbage left from the build process? Perhaps it should be set to nothing in __init__ then?

@IanButterworth
Copy link
Sponsor Member

It's already set to DEFAULT_IO[] = stderr in Pkg's __init__ and IJulia doesn't touch it from what I can tell, so I don't understand how it's nothing on @fredrikekre's system

@fredrikekre
Copy link
Member Author

Have you rebuilt the sysimage or changed the IJulia source code? Note that IJulia won't load your Pkg with changed UUID

@KristofferC
Copy link
Sponsor Member

It's already set to DEFAULT_IO[] = stderr in Pkg's init

Okay, but we could change it in this PR to set it to nothing maybe?

@IanButterworth
Copy link
Sponsor Member

That's it. Sorry for the noise

@fredrikekre fredrikekre merged commit ad3d639 into master May 2, 2021
@fredrikekre fredrikekre deleted the fe/io-fix branch May 2, 2021 20:31
@KristofferC
Copy link
Sponsor Member

https://github.com/JuliaLang/Pkg.jl/pull/2543/files#diff-1347ee05134e00dcfa1afc9dd8b6fdad9be2afeceee98662a2b76b6d77eddf53R36

Yeah, but was thinking if something sets it during the Julia build process, then we will have the wrong value. But that probably won't happen.

@carstenbauer
Copy link
Member

Will this be fixed in a Julia 1.6.2 or only 1.7? In the latter case it might be worth fixing this temporarily in IJulia.jl, IMHO.

@KristofferC
Copy link
Sponsor Member

It has the backport label so will be in 1.6.2 which hopefully should be out in not too long time.

KristofferC pushed a commit that referenced this pull request Jun 10, 2021
KristofferC pushed a commit that referenced this pull request Jun 10, 2021
@fredrikekre fredrikekre removed the backport 1.6 Change should be backported to release-1.6 label Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants