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

[RFC] don't load Pkg automatically #28206

Merged
merged 1 commit into from
Jul 23, 2018
Merged

[RFC] don't load Pkg automatically #28206

merged 1 commit into from
Jul 23, 2018

Conversation

fredrikekre
Copy link
Member

@fredrikekre fredrikekre commented Jul 20, 2018

In fbc30cd automatic loading of Pkg was added in interactive use. I propose to revert this for the following reasons:

  • For interactive use most people should use the Pkg REPL, which will work regardless of this.
  • It is a bit weird to load things automatically based on the mode; it makes interactive and non-interactive scripts behave differently.
  • You can't load another package called Pkg.

don't load Pkg automatically in interactive use
@fredrikekre
Copy link
Member Author

Also, people that really want Pkg available in the REPL without having to using Pkg can put that in startup.jl.

@vtjnash
Copy link
Member

vtjnash commented Jul 20, 2018

For interactive use most people should use the Pkg REPL, which will work regardless of this.

We shouldn’t need to embed a second programming language just to install packages

It is a bit weird to load things automatically based on the mode; it makes interactive and non-interactive scripts behave differently.

In the future, I think we expect to do much more of this. Pkg and InteractiveTools are simply the demo cases right now.

@KristofferC
Copy link
Member

KristofferC commented Jul 20, 2018

Please don't make irrelevant gripes about the lack of API documentation of Pkg in others PR. There is a Julia API and it is being documented right now. This PR has nothing to do with that.

@fredrikekre
Copy link
Member Author

For interactive use most people should use the Pkg REPL, which will work regardless of this.

We shouldn’t need to embed a second programming language just to install packages

That seems completely unrelated to this PR. If you want to use Pkg just using Pkg just like you do with any other library. If that annoys you, put it in startup.jl.

In the future, I think we expect to do much more of this. Pkg and InteractiveTools are simply the demo cases right now.

Perhaps, but it should not be implemented like this, it has to be configurable. Otherwise the language makes it impossible to load other packages with the same name.

@vtjnash
Copy link
Member

vtjnash commented Jul 20, 2018

For interactive use most people should use the Pkg REPL, which will work regardless of this.

This isn't necessarily true or desirable. We're planning to remove Pkg from the system image as soon as we can without significantly impacting performance. The using Pkg call will thus be necessary for the REPL mode to be available. I thought we had an issue about this to link to (#5155 perhaps), but maybe it's been all in Slack discussion channels?

@KristofferC
Copy link
Member

This isn't necessarily true or desirable. We're planning to remove Pkg from the system image as soon as we can without significantly impacting performance. The using Pkg call will thus be necessary for the REPL mode to be available.

So even more reason to do this then?

@StefanKarpinski
Copy link
Member

The using Pkg call will thus be necessary for the REPL mode to be available.

Why? Activating the REPL mode can automatically load Pkg. All the more reason to prefer the REPL mode.

@KristofferC
Copy link
Member

Will merge this in a while unless someone objects since otherwise it is impossible to load another package called Pkg (for example when developing it).

@StefanKarpinski StefanKarpinski merged commit 10002fc into master Jul 23, 2018
@StefanKarpinski StefanKarpinski deleted the fe/noPkg branch July 23, 2018 19:53
@vtjnash
Copy link
Member

vtjnash commented Jul 24, 2018

otherwise it is impossible to load another package called Pkg

The new code has made this annoyingly long to type, but let's not get carried away with calling it impossible. Specifically, it should be: const Pkg = (Base._require(Base.PkgId("Pkg")); Base.root_module(Base.PkgId("Pkg")))

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