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 allow packages to fall back to finding UUID in project if not in deps of manifest #27932

Merged
merged 2 commits into from
Jul 20, 2018

Conversation

KristofferC
Copy link
Member

A common developer situation is that you have a project with a bunch of dependencies. You are working on the dependencies and want to add a dependency to one of them.

As a concrete example, let's say I have:

(Env) pkg> st
    Status `Project.toml`
  [7876af07] Example v0.5.1+ [`~/.julia/dev/Example`]
  [682c06a0] JSON v0.18.0+ [`~/.julia/dev/JSON`]

Now, I want to use Example in JSON.

The current steps to properly do this before JSON can be loaded is:

  1. Add import Example to JSON.
  2. Make the JSON project your active project.
  3. Do Pkg.add("Example") to update Project.toml in JSON (this could potentially fail due to some weird constraints in the Manifest file in the JSON project)
  4. Make your previous main project your active project
  5. Do Pkg.resolve() to update the main project's Manifest.toml, which looks at JSON's Project.toml (populate deps = ["Example"] for JSON).
  6. Load JSON.

Arguably, the steps 2-5 is kinda annoying while you are in the heat of developing. The reason we need to lookup the deps section in the Manifest is because `JSON´ could potentially have used another package (another UUID) that also had the name Example. However, sometimes the Project.toml Example is the only package with that name in the Manifest, and it is likely that that is the package that one wanted to load into JSON anyway. So it feels a bit cruel to not allow it to just be loaded.

This PR instead makes the workflow:

  1. Add import Example to JSON.
  2. Load JSON.

with the result

julia> import JSON
[ Info: Precompiling module JSON
┌ Warning: Package JSON does not have Example in its dependencies:
│  - If you have JSON checked out for development and have
│    added Example as a dependency but haven't updated your primary
│    environment's manifest file, try `Pkg.resolve()`.
│  - Otherwise you may need to report an issue with JSON
│
│ Attempting fallback to project dependency with name Example.
└ @ Base loading.jl:817

julia>

So it is quite clear that something wrong is happening but you can still continue working.

There are of course other solutions to this issue:

  • Make steps 2-5 easier (pkg> add JSON:Example; resolve)
  • Somehow detect when this situation occurs and do it manually.

The current implementation is not perfect. It will look for fallback packages in environments below you in the stack which is perhaps not wanted. Also, maybe this should only be enabled for packages that are in "developer mode", i.e. tracking a path.

cc @Keno, @stevengj who has been "bitten" by this issue, cc @StefanKarpinski

@KristofferC KristofferC added packages Package management and loading triage This should be discussed on a triage call labels Jul 4, 2018
@stevengj
Copy link
Member

stevengj commented Jul 5, 2018

(Closes JuliaLang/Pkg.jl#457)

@StefanKarpinski StefanKarpinski added this to the 0.7 milestone Jul 5, 2018
@StefanKarpinski StefanKarpinski removed the triage This should be discussed on a triage call label Jul 5, 2018
@JeffBezanson
Copy link
Member

What's the status of this?

@StefanKarpinski
Copy link
Member

This is a non-breaking change so I don't think it's a blocker. Would be nice to have though.

@KristofferC
Copy link
Member Author

I think this will really spam warning messages so need to work on that a bit.

One thing where this would be good to have is where you set the context module to something else (like is common in Atom). Right now, you can't import almost any packages when you do that due to them not having the correct deps entries.

@KristofferC KristofferC force-pushed the kc/fallback_project_load branch from d3691af to db17b5b Compare July 18, 2018 14:15
@KristofferC
Copy link
Member Author

Should be ready for review.

julia> Base.require(UUIDs, :Revise)
┌ Warning: Package UUIDs does not have Revise in its dependencies:
│ - If you have UUIDs checked out for development and have
│   added Revise as a dependency but haven't updated your primary
│   environment's manifest file, try `Pkg.resolve()`.
│ - Otherwise you may need to report an issue with UUIDs
└ Loading Revise into UUIDs from project dependency, future warnings for UUIDs are suppressed.
Revise

julia> Base.require(LinearAlgebra, :Revise)
┌ Warning:
└ Loading Revise into LinearAlgebra from project dependency, future warnings for LinearAlgebra are suppressed.
Revise

julia> Base.require(UUIDs, :Revise)
Revise

Need to add a test but want to get an approval of the method first.

@StefanKarpinski StefanKarpinski force-pushed the kc/fallback_project_load branch from db17b5b to fabd2e4 Compare July 19, 2018 20:56
@StefanKarpinski
Copy link
Member

FreeBSD is ye ol' ccall failure: #28191.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants