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

WIP: Add the "@" entry to the end of Base.DEPOT_PATH, and add the Base.depot_path() function #35207

Closed
wants to merge 1 commit into from

Conversation

DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Mar 22, 2020

Alternative to: #35195
Alternative to: JuliaLang/Pkg.jl#1727

Needs PkgEval run.
Needs tests.
Needs compat annotation.
Needs news.


This pull request makes the following changes:

  1. Adds the "@" entry to the end of the Base.DEPOT_PATH vector.
  2. Adds the Base.depot_path() function. Base.depot_path() expands "@" to abspath(Base.active_project(), "julia_depot").

TODO:

  • Go through all of Base and replace all uses of Base.DEPOT_PATH with calls to Base.depot_path().

cc: @davidanthoff
cc: @StefanKarpinski

@DilumAluthge DilumAluthge changed the title WIP: Add support for "@" entries in DEPOT_PATH WIP: Add support for the "@" entry in DEPOT_PATH Mar 22, 2020
@DilumAluthge DilumAluthge changed the title WIP: Add support for the "@" entry in DEPOT_PATH WIP: Add the "@" entry to the end of Base.DEPOT_PATH, and add the Base.depot_path() function Mar 22, 2020
@StefanKarpinski
Copy link
Member

I can see the appeal of this since it allows the user to choose whether to look in a project for vendored dependencies (both packages and artifacts). However, I have a couple of issues with it:

  1. I think we should look in the project for packages and artifacts first. After all, if the project has bothered to ship vendored resources, shouldn't we use the project's version of those resources?

  2. I don't think we should be treating the project as a depot for all purposes. If $project/registries exists, should we look in registries that are included there? Seems like a bad idea. Similarly, if the project contains $project/servers should we let the authentication info included there apply to talking to authenticated servers and provide telemetry info? If there is an $project/environments directory, should we consider those to be named environments when this project happens to be active? These all seem like a bad idea.

An alternative would be to make the default DEPOT_PATH have @ as its first entry (and replace Pkg.depots1() with Pkg.user_depot() which skips the @ and is a better name anyway). We could then have a toggle for whether we should expand the @ or drop it and expand it when looking for packages and artifacts and drop it when looking for anything else. That would allow people to opt-out of loading vendored packages and artifacts.

However, I think that unconditionally using vendored packages and artifacts when loading resources for a project would be fine and would be simpler. After all, what's the benefit of opting out? It's not like there's a security issue here: if the project wants to load code that it ships with, it can do that much more easily than by shipping a vendored artifact or package.

@DilumAluthge
Copy link
Member Author

I agree 100%.

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.

2 participants