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

Support a [sources] section in Project.toml for specifying paths and repo locations for dependencies #3783

Merged
merged 5 commits into from
Mar 5, 2024

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Feb 7, 2024

Extracted and tweaked based on #3263.

Currently requires JuliaLang/julia#53233

Some questions:

  • What happens if you have a source for a URL and then you add the package with another URL? Error or update the source?
  • Same as above but with a path.
  • What should free do when there is a source specified.

TODO:

  • Docs
  • More thorough tests
  • Verify parsing of source section in project file
  • remove source in free (update_package_free)
  • update source on add (update_package_add)
  • remove source in rm
  • Test when resolve with non source in manifest and source in project.

@MilesCranmer
Copy link
Member

MilesCranmer commented Feb 7, 2024

Personal views on these:

What happens if you have a source for a URL and then you add the package with another URL? Error or update the source?

Updating the source seems the most reasonable thing to do here. Mostly because it's the same thing done for Manifest.toml.

Same as above but with a path.

Also update the source.

What should free do when there is a source specified.

I think free should remove the source, only because currently the docstring says If pkg is tracking a path, e.g. after Pkg.develop, go back to tracking registered versions. Since a path is a type of source I think this behavior would be in line with the docstring.

@KristofferC
Copy link
Member Author

More questions:

  • What happens if you have a source of a package that is not in [deps]? Is that an error, or what should happen when you end up adding a package with that name?
  • Related to the above, should rm of a package remove its source entry?
  • What happens if you have a manifest with one path and then modify the project source to take another path? What should take precedence? Presumably the project source?

@MilesCranmer
Copy link
Member

  • What happens if you have a source of a package that is not in [deps]? Is that an error, or what should happen when you end up adding a package with that name?

I think it should error, unless it is in [weakdeps] or [extras]. That would let you set sources for packages you don't directly depend on. i.e., the same behavior as for [compat]:

# With MyFakePackage appearing in [compat]
ERROR: Compat `MyFakePackage` not listed in `deps`, `weakdeps` or `extras` section at ...
  • Related to the above, should rm of a package remove its source entry?

I think it should (similar to [compat])

  • What happens if you have a manifest with one path and then modify the project source to take another path? What should take precedence? Presumably the project source?

I think that the manifest would take precedence but there should be a warning message that the manifest file is out of date and should be updated. i.e., similar behavior to when you change the [compat] without updating the manifest:

┌ Warning: The project dependencies or compat requirements have changed since the manifest was last resolved.
│ It is recommended to `Pkg.resolve()` or consider `Pkg.update()` if necessary.
└ @ Pkg.API ~/.julia/juliaup/julia-1.10.0+0.aarch64.apple.darwin14/share/julia/stdlib/v1.10/Pkg/src/API.jl:1800

@KristofferC
Copy link
Member Author

I think that the manifest would take precedence but there should be a warning message that the manifest file is out of date and should be updated. i.e., similar behavior to when you change the [compat] without updating the manifest:

I underspecified a bit perhaps. What I means was, you have a manifest with some path to a package. Then you set a source to another path. Now you do a resolve. What should be in the manifest after that? Having the non-source path entry from the old manifest would be bad IMO in a similar way of having a package that breaks the compat specification.

@MilesCranmer
Copy link
Member

Oh, I see. Then yes I would agree with you the [sources] should take precedence and overwrite the Manifest

@giordano
Copy link
Contributor

giordano commented Feb 8, 2024

I haven't tried this PR yet, but a possible use-case I see for this feature is having test-only dependencies which aren't registered in General, sometimes they aren't really worth being registered on their own. This came up for example in JuliaHEP/Geant4.jl#8, where the current solution is to manually check out the repository.

@Krastanov
Copy link
Contributor

Krastanov commented Feb 8, 2024

Will this be useful for dev packages? E.g. I dev ./LocalPackage.jl which provides a source entry LocalPackage = "file:/.../LocalPackage.jl". Then I delete my Manifest.toml and finally I do instantiate. Will LocalPackage:

  • be added or deved?
  • still refer to file:/.../LocalPackage.jl?

@KristofferC
Copy link
Member Author

KristofferC commented Feb 8, 2024

feature is having test-only dependencies which aren't registered in General, sometimes they aren't really worth being registered on their own.

This should work now, see added test.

@mortenpi
Copy link
Contributor

mortenpi commented Feb 8, 2024

I am cautiously excited about this! I'm curious about two use cases though, and it's not immediately clear if this helps with these or not:

  1. The Documenter-type use case. I have e.g. a docs/Project.toml:

    [deps]
    MyPackage = "5217a498-cd5d-4ec6-b8c2-9b85a09b6e3e"
    Documenter = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
    
    [source]
    MyPackage = {path = ".."}

    Just doing julia --project=docs/ -e 'using Pkg; Pkg.instantiate()' should work, right? But will the Manifest now point to MyPackage as a dev-dependency (i.e. pointing to ..) or will it copy the source to into the depot (i.e. ~/.julia/packages)?

  2. Will it handle the case where the local dependency has itself a [sources] section that points to another local "sub-dependency"? I.e. suppose I have a top-level environment I want to instantiate / work in:

    [deps]
    LocalPackage = "5217a498-cd5d-4ec6-b8c2-9b85a09b6e3e"
    
    [source]
    LocalPackage = {path = "packages/LocalPackage"}

    And packages/LocalPackage/Project.toml itself uses another local package:

    [deps]
    LocalSubPackage = "e30172f5-a6a5-5a46-863b-614d45cd2de4"
    
    [source]
    LocalSubPackage = {path = "../LocalSubPackage/"}

    Will it resolve it correctly? I.e. will LocalSubPackage get added to the manifest automatically?

    The use case here is the monorepo case, where you have one or more top-level dependencies, and then a bunch of vendored local packages that have a complex dependency graph among themselves, but you would pretty much like to treat them as normal packages.

@KristofferC
Copy link
Member Author

The Documenter-type use case.

Yes, that should work fine and it will point to it as a "dev-dependency".

Will it handle the case where the local dependency has itself a [sources] section that points to another local "sub-dependency"?

At least for now, I didn't plan on making it "recursive". It's possible but it also has many failure modes where you encounter conflicting information and you may also have to keep downloading stuff as you "discover" more and more URL sources. As a first cut I don't think that will be enabled but it can be further discussed later of course.

@mortenpi
Copy link
Contributor

At least for now, I didn't plan on making it "recursive".

Would it by any chance work if I specify the subpackages in the top-level project though, even if they are not direct dependencies of the top-level project themselves? So instead of the example above, you'd have something like this:

[deps]
LocalPackage = "5217a498-cd5d-4ec6-b8c2-9b85a09b6e3e"

[source]
LocalPackage = {path = "packages/LocalPackage"}
LocalSubPackage = {path = "packages/LocalSubPackage/"}

And packages/LocalPackage/Project.toml itself uses another local package:

[deps]
LocalSubPackage = "e30172f5-a6a5-5a46-863b-614d45cd2de4"

Adding some CI checks and simple tooling to keep a global [source] list updated across a few top-level projects would be easy.

@KristofferC
Copy link
Member Author

Would it by any chance work if I specify the subpackages in the top-level project though, even if they are not direct dependencies of the top-level project themselves?

In theory, I think it could be made to work if that package is also added to [extras].

@KristofferC KristofferC force-pushed the kc/dsadsa branch 3 times, most recently from cd8a9d9 to 78f4587 Compare February 28, 2024 15:26
@KristofferC KristofferC requested a review from a team as a code owner February 28, 2024 15:26
@MasonProtter
Copy link
Contributor

MasonProtter commented Feb 29, 2024

Just tested this out, and the use-case I had in mind seems to work great! I'm very excited, thank you for the awesome work @KristofferC. This, together with the app stuff, and pkgimages are all coalescing into a very cool story for julia code management and sharing.


Regarding the questions you posed, I think I agree with @MilesCranmer's answers, they all seem reasonable. Generally, my mental model is that the Project.toml should describe what we desire, and the Manifest.toml should describe what specifically was done.

So being able to describe paths, urls, and branches in the Project, and have those take priority over what is written in the Manifest seems correct and ergonomic to me.


Regarding API, is there a way we can make Pkg.add(path=some_local_path) add the package at some_local_path to the [sources] section? Or maybe we should make a new function like Pkg.add_source?

@KristofferC
Copy link
Member Author

Regarding API, is there a way we can make Pkg.add(path=some_local_path) add the package at some_local_path to the [sources] section?

I think that makes sense to do automatically because a free or rm will also remove it from the source section.

@LilithHafner
Copy link
Member

The design decision here all look good to me. Thanks for this lovely feature!

@quinnj
Copy link
Member

quinnj commented Mar 1, 2024

Thanks for picking this up and pushing it forward @KristofferC; sorry I let it drop/droop so long ago. I agree it will be a great feature!

@tpgillam
Copy link

tpgillam commented Mar 1, 2024

To echo, thank you for working on this!

I'm very interested in whether this might simplify the 'monorepo workflow' discussed here: #3263 (comment)

Am I right in thinking that a top-level, single Manifest.toml will be required in this case?

IIUC, I think 'yes' because [sources] won't be followed recursively. i.e. suppose we have a directory containing multiple packages that depend on each other non-trivially. Ideally one would be able to go into a leaf dependency and say ] instantiate. But if A depends on B, and B depends on C, then I can't instantiate A because it won't find C?

Or is there a mechanism for all of A, B & C etc. to share a single [sources] section "included" into their Project.tomls from somewhere else?

@nhz2
Copy link
Contributor

nhz2 commented Mar 1, 2024

Thank you for working on this!

I am testing using this in a test/Project.toml and it seems to behave strangely there.
For example, for a package Foo
In Foo/Project.toml I have:

name = "Foo"
uuid = "7694c9c6-b3cb-4fb3-a3a7-392d5a2739ee"
authors = ["nhz2 <nhz2@cornell.edu>"]
version = "0.1.0"

In Foo/test/Project.toml I have:

[deps]
Foo = "7694c9c6-b3cb-4fb3-a3a7-392d5a2739ee"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[sources]
Foo = {path = ".."}

If I run the tests using Pkg.test it looks like the relative paths in Foo/test/Project.toml are being looked up relative to the active Project.toml instead of Foo/test/Project.toml

Here is the full error.
julia> Pkg.activate(;temp=true)
  Activating new project at `/tmp/jl_oAEynA`

julia> Pkg.develop(path="/home/nathan/Documents/myenv/Foo")
   Resolving package versions...
    Updating `/tmp/jl_oAEynA/Project.toml`
  [7694c9c6] + Foo v0.1.0 `~/Documents/myenv/Foo`
    Updating `/tmp/jl_oAEynA/Manifest.toml`
  [7694c9c6] + Foo v0.1.0 `~/Documents/myenv/Foo`

julia> Pkg.test("Foo")
     Testing Foo
ERROR: could not find project file for package `Foo [7694c9c6]` at `/tmp/`
Stacktrace:
  [1] pkgerror(msg::String)
    @ Pkg.Types ~/github/Pkg.jl/src/Types.jl:68
  [2] collect_project(pkg::Pkg.Types.PackageSpec, path::String)
    @ Pkg.Operations ~/github/Pkg.jl/src/Operations.jl:253
  [3] collect_fixed!(env::Pkg.Types.EnvCache, pkgs::Vector{Pkg.Types.PackageSpec}, names::Dict{Base.UUID, String})
    @ Pkg.Operations ~/github/Pkg.jl/src/Operations.jl:329
  [4] resolve_versions!(env::Pkg.Types.EnvCache, registries::Vector{…}, pkgs::Vector{…}, julia_version::VersionNumber, installed_only::Bool)
    @ Pkg.Operations ~/github/Pkg.jl/src/Operations.jl:392
  [5] up(ctx::Pkg.Types.Context, pkgs::Vector{…}, level::Pkg.Types.UpgradeLevel; skip_writing_project::Bool, preserve::Nothing)
    @ Pkg.Operations ~/github/Pkg.jl/src/Operations.jl:1610
  [6] up
    @ ~/github/Pkg.jl/src/Operations.jl:1589 [inlined]
  [7] up(ctx::Pkg.Types.Context, pkgs::Vector{…}; level::Pkg.Types.UpgradeLevel, mode::Pkg.Types.PackageMode, preserve::Nothing, update_registry::Bool, skip_writing_project::Bool, kwargs::@Kwargs{})
    @ Pkg.API ~/github/Pkg.jl/src/API.jl:381
  [8] up
    @ ~/github/Pkg.jl/src/API.jl:355 [inlined]
  [9] up
    @ ~/github/Pkg.jl/src/API.jl:163 [inlined]
 [10] #resolve#143
    @ ~/github/Pkg.jl/src/API.jl:387 [inlined]
 [11] resolve
    @ ~/github/Pkg.jl/src/API.jl:386 [inlined]
 [12] (::Pkg.Operations.var"#118#123"{String, Bool, Bool, Bool, Pkg.Operations.var"#131#136"{}, Pkg.Types.PackageSpec})()
    @ Pkg.Operations ~/github/Pkg.jl/src/Operations.jl:1885
 [13] with_temp_env(fn::Pkg.Operations.var"#118#123"{}, temp_env::String)
    @ Pkg.Operations ~/github/Pkg.jl/src/Operations.jl:1767
 [14] (::Pkg.Operations.var"#116#121"{})(tmp::String)
    @ Pkg.Operations ~/github/Pkg.jl/src/Operations.jl:1872
 [15] mktempdir(fn::Pkg.Operations.var"#116#121"{}, parent::String; prefix::String)
    @ Base.Filesystem ./file.jl:835
 [16] mktempdir(fn::Function, parent::String)
    @ Base.Filesystem ./file.jl:831
 [17] mktempdir
    @ ./file.jl:831 [inlined]
 [18] #sandbox#115
    @ ~/github/Pkg.jl/src/Operations.jl:1823 [inlined]
 [19] test(ctx::Pkg.Types.Context, pkgs::Vector{…}; coverage::Bool, julia_args::Cmd, test_args::Cmd, test_fn::Nothing, force_latest_compatible_version::Bool, allow_earlier_backwards_compatible_versions::Bool, allow_reresolve::Bool)
    @ Pkg.Operations ~/github/Pkg.jl/src/Operations.jl:2034
 [20] test
    @ ~/github/Pkg.jl/src/Operations.jl:1978 [inlined]
 [21] test(ctx::Pkg.Types.Context, pkgs::Vector{…}; coverage::Bool, test_fn::Nothing, julia_args::Cmd, test_args::Cmd, force_latest_compatible_version::Bool, allow_earlier_backwards_compatible_versions::Bool, allow_reresolve::Bool, kwargs::@Kwargs{})
    @ Pkg.API ~/github/Pkg.jl/src/API.jl:474
 [22] test(pkgs::Vector{Pkg.Types.PackageSpec}; io::Pkg.UnstableIO, kwargs::@Kwargs{})
    @ Pkg.API ~/github/Pkg.jl/src/API.jl:158
 [23] test(pkgs::Vector{Pkg.Types.PackageSpec})
    @ Pkg.API ~/github/Pkg.jl/src/API.jl:147
 [24] test
    @ ~/github/Pkg.jl/src/API.jl:146 [inlined]
 [25] test(pkg::String)
    @ Pkg.API ~/github/Pkg.jl/src/API.jl:145
 [26] top-level scope
    @ REPL[7]:1
Some type information was truncated. Use `show(err)` to see complete types.

@KristofferC
Copy link
Member Author

But in test/Project.toml you don't need to put the parent package at all I thought? That will be handled by Pkg.test, or?

@KristofferC KristofferC changed the title Support a [sources] section in Project.toml for specifying relative path and repo locations for dependencies Support a [sources] section in Project.toml for specifying paths and repo locations for dependencies Mar 5, 2024
@KristofferC
Copy link
Member Author

This error also happens if I add a local unregistered test utility package to the test/Project.toml sources.

I will fix this in an upcoming PR.

@KristofferC KristofferC merged commit 5c73d7f into master Mar 5, 2024
9 checks passed
@KristofferC KristofferC deleted the kc/dsadsa branch March 5, 2024 11:23
@goerz
Copy link

goerz commented Mar 5, 2024

Wow, so excited this is merged! Does this automatically show up in the "nightly" Julia release, or do I need to do something to play around with this (since Pkg is developed sort-of-independently)?

@KristofferC
Copy link
Member Author

When Pkg is bumped on nightly it will be in the nightly release. Probably easiest to wait for that.

@KristofferC
Copy link
Member Author

KristofferC commented Mar 5, 2024

This is the PR to add thins automatically to [sources] if someone wants to look through the CI stuff #3826. Might be easy. Also needs tests.

@musoke
Copy link

musoke commented Apr 19, 2024

Wow, so excited this is merged! Does this automatically show up in the "nightly" Julia release, or do I need to do something to play around with this (since Pkg is developed sort-of-independently)?

Current nightly has these changes - this Project.toml acted as expected.

[deps]
Example = "7876af07-990d-54b4-ab0e-23690620f79a"

[sources]
Example = {url = "https://github.com/JuliaLang/Example.jl", rev = "custom_branch"}
julia> versioninfo()
Julia Version 1.12.0-DEV.369
Commit 2f9096218bf (2024-04-19 12:25 UTC)
Build Info:
  Official https://julialang.org/ release

KristofferC added a commit that referenced this pull request May 9, 2024
…d repo locations for dependencies (#3783)

* Support a `[sources]` section in Project.toml for specifying relative path and repo locations for dependencies

---------

Co-authored-by: Jacob Quinn <quinn.jacobd@gmail.com>
@sylvaticus
Copy link

Generally, my mental model is that the Project.toml should describe what we desire, and the Manifest.toml should describe what specifically was done.
So being able to describe paths, urls, and branches in the Project, and have those take priority over what is written in the Manifest seems correct and ergonomic to me.

I have the feeling that the second statement a bit contradicts the first one. Project.toml describes what [in general] we want, and Manifest.toml should describe what [in the specific installation of the package] was done. So Manifest should take precedence.
This leads to an issue with a couple of unregistered packages I am developing in parallel (julia-1.11.0-beta1). I want the main package project to specify that the dependent package source is on github. However I want then be able to specify that in my particular installation the source is in .julia/dev:

Main package (GenFSM) Project.toml:

[deps]
GenFSM_resources = "731fd045-8cb1-4036-8c04-a2a477f3265a"

[sources]
GenFSM_resources = {url = "git@github.com:forestmod/GenFSM_resources.jl.git", rev = "main"}

But when I try to specify to use a local version of the dependant package, instead of modify Manifest.toml and use that path in my particular installation, it returns an error:

(GenFSM) pkg> dev GenFSM_resources
ERROR: `path` and `url` are conflicting specifications

@KristofferC
Copy link
Member Author

What do you want dev GenFSM_resources to do? Do you want it to rewrite the entry in your [sources] or just start ignoring that source entry completely? When is the source entry used?

@sylvaticus
Copy link

Granted, I could be completely wrong, but I don't think that dev GenFSM_resources, ran in the context of the GenFSM environment, should change the [source] of GenFSM_resources (in GenFSM Project.toml), but rather add the local path of GenFSM_resources in GenFSM Manifest.toml.
Then the [source] of GenFSM_resources would be used when there is no Manifest, for example to instantiate the environment of the GenFSM package...

@KristofferC
Copy link
Member Author

So say you add:

[sources]
Example = {url = "[https:](https://github.com/JuliaLang/Example.jl)"}

and then do a Pkg.add("Example"). Should the source be respected?

@sylvaticus
Copy link

I think the difference between add and dev is that the first one is supposed to change the package itself (let's name it Foo). Foo to work needs Example, so you add Example for all the users of the package and you change the Foo Project.toml.
If you instead do dev Example, it's for yourself, the guy in front of the machine, so Example Project.toml shouldn't be touched, only its manifest (and this should prevail when actual package loading happens to lookup Example).
To answer your question, provided that there is a way to do Pkg.add("Example",url="[https:](https://github.com/JuliaLang/Example.jl), then doing add without this information for me should remove it from [source].

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.